On Wed, 2006-11-29 at 15:30 +1100, Keith Owens wrote: > David Miller (on Tue, 28 Nov 2006 20:04:53 -0800 (PST)) wrote: > >From: Keith Owens <kaos@ocs.com.au> > >Date: Wed, 29 Nov 2006 14:56:20 +1100 > > > >> Secondly, I believe that this is a separate problem from bug 22278. > >> hpet_readl() is correctly using volatile internally, but its result is > >> being assigned to a pair of normal integers (not declared as volatile). > >> In the context of wait_hpet_tick, all the variables are unqualified so > >> gcc is allowed to optimize the comparison away. > >> > >> The same problem may exist in other parts of arch/i386/kernel/time_hpet.c, > >> where the return value from hpet_readl() is assigned to a normal > >> variable. Nothing in the C standard says that those unqualified > >> variables should be magically treated as volatile, just because the > >> original code that extracted the value used volatile. IOW, time_hpet.c > >> needs to declare any variables that hold the result of hpet_readl() as > >> being volatile variables. > > > >I disagree with this. > > > >readl() returns values from an opaque source, and it is declared > >as such to show this to GCC. It's like a function that GCC > >cannot see the implementation of, which it cannot determine > >anything about wrt. return values. > > > >The volatile'ness does not simply disappear the moment you > >assign the result to some local variable which is not volatile. > > > >Half of our drivers would break if this were true. > > This is definitely a gcc bug, 4.1.0 is doing something weird. Compile > with CONFIG_CC_OPTIMIZE_FOR_SIZE=n and the bug appears, > CONFIG_CC_OPTIMIZE_FOR_SIZE=y has no problem. > > Compile with CONFIG_CC_OPTIMIZE_FOR_SIZE=n and _either_ of the patches > below and the problem disappears. >
My theory: gcc is inlining readl into hpet_readl (readl is an inline function, so it should be doing this no matter what), and inlining hpet_readl into wait_hpet_tick (otherwise, it can't possibly make any assumptions about the return values of hpet_readl -- this looks to be a SUSE-specific over-aggressive optimization), and somewhere along the way the volatile qualifier is getting lost. -- Nicholas Miell <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
