On Wed, May 19, 2010 at 11:03:27PM +1000, Nick Piggin wrote: > On Mon, May 17, 2010 at 08:01:54AM -0700, Linus Torvalds wrote: > > > > > > On Mon, 17 May 2010, Anton Blanchard wrote: > > > > > > It turns out this bad code is a result of us defining atomic_t as a > > > volatile int. > > > > Heh. Ok, as you point out in the commit message, I obviously agree with > > this patch. "volatile" on data is evil, with the possible exception of > > "jiffies" type things. > > > > So applied. > > I wonder, Linus, is there a good reason to use volatile for these at > all? > > I asked you about it quite a while back, and IIRC you said it might > be OK to remove volatile from bitops, provided that callers were audited > (ie. that nobody used bitops on volatile variables). > > For atomic_read it shouldn't matter unless gcc is *really* bad at it. > Ah, for atomic_read, the required semantic is surely ACCESS_ONCE, so > that's where the volatile is needed? (maybe it would be clearer to > explicitly use ACCESS_ONCE?) Explicit use of ACCESS_ONCE() where needed makes a lot of sense to me, and allows better code to be generated for initialization and cleanup code where no other task has access to the atomic_t. > The case I was thinking about for bitops was for multiple non-atomic > bitops, which would be nice to combine. In reality a lot of performance > critical code (like page allocator) bites the bullet and does the > open-coded bitwise ops. But it would be nice if that just worked for > __set_bit / __clear_bit too. FWIW, a similar debate in the C-language standards committee seems to be headed in the direction of allowing combining of adjacent atomic operations. Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
