On Wed, 6 Dec 2006, Linus Torvalds wrote:
>
> Sadly, gcc doesn't do it in this case. Still, I'd rather keep the source
> clean, and hope that the compiler improves eventually, than to make the
> code uglier.
Actually, it's our own damn fault.
We've long had our arguments "const volatile" to test_bit(), which
basically means that gcc can't do the optimization. Damn.
And they need to be "volatile" not because we _want_ the thing to be
volaile, but because these things are occationally used on volatile
objects (so if the function doesn't take a volatile pointer, it would
warn).
That's why so many of these helper functions use "const volatile"
pointers, which on the face of it would seem strange if you don't realize
that it's more about a C type issue than about the _actual_ type being
either "const" or "volatile".
Sad. I guess we could remove the "const volatile" from the _cast_, but the
thing is, if the underlying object we're actually looking at really _is_
volatile, we shouldn't do that. GAAH.
Really sad. I doubt any of the callers actually want the "volatile" access
at all. Things like <linux/page-flags.h> definitely _don't_ want it.
I suspect we should just face up to the fact that
(a) "volatile" on kernel data is basically always a bug, and you should
use locking. "volatile" doesn't help anything at all with memory
ordering and friends, so it's insane to think it "solves" anything on
its own.
(b) on "iomem" pointers it does make sense, but those need special
accessor functions _anyway_, so things like test_bit() wouldn't work
on them.
(c) if you spin on a value changing, you should use "cpu_relax()" or
"barrier()" anyway, which will force gcc to re-load any values from
memory over the loop.
and remove the "volatile" from all the bitop accessor functions.
Or at least from "test_bit()". It's not like it's synchronous _anyway_
(there's no memory barriers etc).
Hmm? Comments? linux-arch added to Cc, just in case people care (this
particular thing is in <asm-*/bitops.h>, so it _is_ architecture-
specific, but we should probably all agree on it first)
Linus
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
