Messages in this thread | | | Date | Sat, 21 Oct 2006 03:38:03 +0400 | From | Alexey Dobriyan <> | Subject | Re: [PATCH 3/3] Enforce "unsigned long flags;" when spinlocking |
| |
On Fri, Oct 20, 2006 at 11:46:40AM -0700, Andrew Morton wrote: > On Fri, 20 Oct 2006 17:15:44 +0400 > Alexey Dobriyan <adobriyan@gmail.com> wrote: > > Make it break or warn if you pass to spin_lock_irqsave() and friends > > something different from "unsigned long flags;". Suprisingly large amount of > > these was caught by recent commit c53421b18f205c5f97c604ae55c6a921f034b0f6 . > > > > Idea is largely from FRV typechecking. > > > > Note #1: checking with sparse is still needed, because a driver can save and > > pass around flags or something. So far patch is very intrusive. > > Note #2: techically, we should break only if sizeof(flags) < sizeof(unsigned long), > > but hey, there is opportunity to escalate. Thus != > > Note #3: yes, would break every single buggy out-of-tree module. > > > > This is a pretty ugly-looking patch. > > > > > + BUILD_BUG_ON(sizeof(flags) != sizeof(unsigned long)); \ > > + typecheck(unsigned long, flags); \ > > ... > > + BUILD_BUG_ON(sizeof(flags) != sizeof(unsigned long)); \ > > + typecheck(unsigned long, flags); \ > > ... > > + BUILD_BUG_ON(sizeof(flags) != sizeof(unsigned long)); \ > > + typecheck(unsigned long, flags); \ > > ... > > + BUILD_BUG_ON(sizeof(flags) != sizeof(unsigned long)); \ > > + typecheck(unsigned long, flags); \ > > ... > > + BUILD_BUG_ON(sizeof(flags) != sizeof(unsigned long)); \ > > + typecheck(unsigned long, flags); \ > > ... > > starting to see a pattern here?
OK, a pattern.
> If we're going to do this then a helper macro build_check_irq_flags() would > help clean things up. It will also allow us to centralise the > warning-vs-error policy decision.
I will find a common header. kernel.h probably.
> I'm not sure that we need both, do we?
I very much ask to do both.
I want build system, compiler, headers etc to do everything against compiling buggy code. Similar to dropping rogue packets, you drop them at the first firewall, even if your central box runs OpenBSD with all ports closed.
In most cases it is impossible, in some cases it's cheap and simple.
So far the tower of usefullness is
a) compiler refuses to compile b) linker refuses to link c) compiler spits warning d) sparse spits (endian) warning
a) is paragon of escalation. b) is close: developer patches code, test-compile _just_one_ file and happily send patch. Maintatiner gets patch, does allmodconfig before sending to Linus, swears, fixes. c) is also very visible, except GCC 4 doing strong as default compiler in distros, deprecation and must check threats; folks also drink coffee while Linux compiles. IIRC, there was seriously looking bug because of missed header => missed prototype => missed warning => compiler pushes junk to stack d) is OK, except number of people runnning sparse is orders of magnitude smaller that number of people compiling, consequently noticed by sparse bugs stay longer in tree. Add non-x86 and endian warnings not being default to the picture.
Enough words. The closer to a) you are, the better. There is cheap and simple way to be there.
> If it spits a warning then it'll get fixed soon enough.
:^) You apply patch to -mm and start waiting on how soon someone will fix gregkh-driver-nozomi.patch so it wouldn't do
u32 flags; but unsigned long flags;
everywhere.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |