Re: [PATCH 2/2]: atomic_t: Remove volatile from atomic_t definition

[Nick Piggin <npiggin@suse.de>]

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?)

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.