[PATCH 24/24] document volatile atomic_read() behavior

[PATCH 24/24] document volatile atomic_read() behavior

[Chris Snook]

From: Chris Snook <csnook@redhat.com>

Update atomic_ops.txt to reflect the newly consistent behavior of
atomic_read(), and to note that volatile (in declarations) is now
considered harmful.

Signed-off-by: Chris Snook <csnook@redhat.com>

--- linux-2.6.23-rc2-orig/Documentation/atomic_ops.txt	2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/Documentation/atomic_ops.txt	2007-08-09 08:24:32.000000000 -0400
@@ -12,7 +12,7 @@
 C integer type will fail.  Something like the following should
 suffice:
 
-	typedef struct { volatile int counter; } atomic_t;
+	typedef struct { int counter; } atomic_t;
 
 	The first operations to implement for atomic_t's are the
 initializers and plain reads.
@@ -38,9 +38,17 @@
 
 Next, we have:
 
-	#define atomic_read(v)	((v)->counter)
+	#define atomic_read(v)	(*(volatile int *)&(v)->counter)
 
-which simply reads the current value of the counter.
+which reads the counter as though it were volatile.  This prevents the
+compiler from optimizing away repeated atomic_read() invocations without
+requiring a more expensive barrier().  Historically this has been
+accomplished by declaring the counter itself to be volatile, but the
+ambiguity of the C standard on the semantics of volatile make this practice
+vulnerable to overly creative interpretation by compilers.  Explicit
+casting in atomic_read() ensures consistent behavior across architectures
+and compilers.  Even with this convenience in atomic_read(), busy-waiters
+should call cpu_relax().
 
 Now, we move onto the actual atomic operation interfaces.
 

Re: [PATCH 24/24] document volatile atomic_read() behavior

[Segher Boessenkool]

> Historically this has been
> +accomplished by declaring the counter itself to be volatile, but the
> +ambiguity of the C standard on the semantics of volatile make this 
> practice
> +vulnerable to overly creative interpretation by compilers.

It's even worse when accessing through a volatile casted pointer;
see for example the recent(*) GCC bugs in that area.

(*) Well, not _all_ that recent.  No one should be using the 3.x
series anymore, right?

> Explicit
> +casting in atomic_read() ensures consistent behavior across 
> architectures
> +and compilers.

Even modulo compiler bugs, what makes you believe that?


Segher

Re: [PATCH 24/24] document volatile atomic_read() behavior

[Chris Snook]

Segher Boessenkool wrote:
>> Historically this has been
>> +accomplished by declaring the counter itself to be volatile, but the
>> +ambiguity of the C standard on the semantics of volatile make this 
>> practice
>> +vulnerable to overly creative interpretation by compilers.
> 
> It's even worse when accessing through a volatile casted pointer;
> see for example the recent(*) GCC bugs in that area.
> 
> (*) Well, not _all_ that recent.  No one should be using the 3.x
> series anymore, right?
> 
>> Explicit
>> +casting in atomic_read() ensures consistent behavior across 
>> architectures
>> +and compilers.
> 
> Even modulo compiler bugs, what makes you believe that?

When you declare a variable volatile, you don't actually tell the compiler where 
you want to override its default optimization behavior, giving it some freedom 
to guess your intentions incorrectly.  When you put the cast on the data access 
itself, there is no question about precisely where in the code you want to 
override the compiler's default optimization behavior.  If the compiler doesn't 
do what you want with a volatile declaration, it might have a plausible excuse 
in the ambiguity of the C standard.  If the compiler doesn't do what you want in 
a cast specific to a single dereference, it's just plain broken.  We try to be 
compatible with plausibly correct compilers, but if they're completely broken, 
we're screwed no matter what.

	-- Chris

Re: [PATCH 24/24] document volatile atomic_read() behavior

[Segher Boessenkool]

>>> Explicit
>>> +casting in atomic_read() ensures consistent behavior across 
>>> architectures
>>> +and compilers.
>> Even modulo compiler bugs, what makes you believe that?
>
> When you declare a variable volatile, you don't actually tell the 
> compiler where you want to override its default optimization behavior, 
> giving it some freedom to guess your intentions incorrectly.  When you 
> put the cast on the data access itself, there is no question about 
> precisely where in the code you want to override the compiler's 
> default optimization behavior.

...except for the small point that this isn't how volatile works.

Rule of thumb: even people who know the semantics of volatile
shouldn't use it.

> If the compiler doesn't do what you want with a volatile declaration, 
> it might have a plausible excuse in the ambiguity of the C standard.  
> If the compiler doesn't do what you want in a cast specific to a 
> single dereference, it's just plain broken.

The other way around.  "volatile" has pretty weak semantics, and
certainly not the semantics you think it has, or that you wish it
had; but *(volatile XX *) doesn't have *any* semantics.  However
much you cast that pointer it still doesn't point to a volatile
object.

GCC will generally take the path of least surprise and perform a
volatile access anyway, but this has only be decided recently (a
year ago or so), and there very likely still are some bugs in
that area.

> We try to be compatible with plausibly correct compilers, but if 
> they're completely broken, we're screwed no matter what.

If you don't know what to expect, you're screwed for sure.


Anyway, what's the supposed advantage of *(volatile *) vs. using
a real volatile object?  That you can access that same object in
a non-volatile way?


Segher

Re: [PATCH 24/24] document volatile atomic_read() behavior

[Chris Snook]

Segher Boessenkool wrote:
>>>> Explicit
>>>> +casting in atomic_read() ensures consistent behavior across 
>>>> architectures
>>>> +and compilers.
>>> Even modulo compiler bugs, what makes you believe that?
>>
>> When you declare a variable volatile, you don't actually tell the 
>> compiler where you want to override its default optimization behavior, 
>> giving it some freedom to guess your intentions incorrectly.  When you 
>> put the cast on the data access itself, there is no question about 
>> precisely where in the code you want to override the compiler's 
>> default optimization behavior.
> 
> ...except for the small point that this isn't how volatile works.
> 
> Rule of thumb: even people who know the semantics of volatile
> shouldn't use it.
> 
>> If the compiler doesn't do what you want with a volatile declaration, 
>> it might have a plausible excuse in the ambiguity of the C standard.  
>> If the compiler doesn't do what you want in a cast specific to a 
>> single dereference, it's just plain broken.
> 
> The other way around.  "volatile" has pretty weak semantics, and
> certainly not the semantics you think it has, or that you wish it
> had; but *(volatile XX *) doesn't have *any* semantics.  However
> much you cast that pointer it still doesn't point to a volatile
> object.
> 
> GCC will generally take the path of least surprise and perform a
> volatile access anyway, but this has only be decided recently (a
> year ago or so), and there very likely still are some bugs in
> that area.
> 
>> We try to be compatible with plausibly correct compilers, but if 
>> they're completely broken, we're screwed no matter what.
> 
> If you don't know what to expect, you're screwed for sure.
> 
> 
> Anyway, what's the supposed advantage of *(volatile *) vs. using
> a real volatile object?  That you can access that same object in
> a non-volatile way?

You'll have to take that up with Linus and the minds behind Volatile Considered 
Harmful, but the crux of it is that volatile objects are prone to compiler bugs 
too, and if we have to track down a compiler bug, it's a lot easier when we know 
exactly where the load is supposed to be because we deliberately put it there, 
rather than letting the compiler re-order everything that lacks a strict data 
dependency and trying to figure out where in a thousand lines of assembler the 
compiler should have put the load for the volatile object.

If we're going to assume that the compiler has bugs we'll never be able to find, 
we all need to find new careers.  If we're going to assume that it has bugs we 
*can* find, then let's use code that makes it easier to do that.

I initially proposed a patch that made all the objects volatile, on the grounds 
that this was a special case where there wasn't much room to have a 
misunderstanding that resulted in anything worse than wasted loads.  Linus 
objected, and now that I've seen all the responses to the new patchset, I 
understand exactly why.  If our compilers really suck as much as everyone says 
they do, it'll be much easier to detect that with volatile casts than with 
volatile declarations.

	-- Chris

Re: [PATCH 24/24] document volatile atomic_read() behavior

[Segher Boessenkool]

>> Anyway, what's the supposed advantage of *(volatile *) vs. using
>> a real volatile object?  That you can access that same object in
>> a non-volatile way?
>
> You'll have to take that up with Linus and the minds behind Volatile 
> Considered Harmful, but the crux of it is that volatile objects are 
> prone to compiler bugs too, and if we have to track down a compiler 
> bug, it's a lot easier when we know exactly where the load is supposed 
> to be because we deliberately put it there, rather than letting the 
> compiler re-order everything that lacks a strict data dependency and 
> trying to figure out where in a thousand lines of assembler the 
> compiler should have put the load for the volatile object.

So, why not do the access explicitly via an inline asm?  It
generates the same code, it's obviously correct, and it's
even *actually* correct.  Plus, you get good compiler
support (and compiler people support).

> If we're going to assume that the compiler has bugs we'll never be 
> able to find, we all need to find new careers.

If we cannot find the bug in finite time, we cannot observe
the bug in finite time either, so either way that's fine :-)

> If we're going to assume that it has bugs we *can* find, then let's 
> use code that makes it easier to do that.

And I'm saying this is a step in the wrong direction for that.

> I initially proposed a patch that made all the objects volatile, on 
> the grounds that this was a special case where there wasn't much room 
> to have a misunderstanding that resulted in anything worse than wasted 
> loads.  Linus objected, and now that I've seen all the responses to 
> the new patchset, I understand exactly why.  If our compilers really 
> suck as much as everyone says they do, it'll be much easier to detect 
> that with volatile casts than with volatile declarations.

Except that accesses via volatile pointer casts open up a whole
new can of worms.


Segher

Re: [PATCH 24/24] document volatile atomic_read() behavior

[Chris Friesen]

Segher Boessenkool wrote:

> Anyway, what's the supposed advantage of *(volatile *) vs. using
> a real volatile object?  That you can access that same object in
> a non-volatile way?

That's my understanding.  That way accesses where you don't care about 
volatility may be optimised.

For instance, in cases where there are already other things controlling 
visibility (as are needed for atomic increment, for example) you don't 
need to make the access itself volatile.

Chris

Re: [PATCH 24/24] document volatile atomic_read() behavior

[Segher Boessenkool]

>> Anyway, what's the supposed advantage of *(volatile *) vs. using
>> a real volatile object?  That you can access that same object in
>> a non-volatile way?
>
> That's my understanding.  That way accesses where you don't care about 
> volatility may be optimised.

But those accesses might be done non-atomically then (for example,
if the compiler knows it only needs to write one byte, it might not
bother writing the rest), so that's no good if you want to read the
thing atomically too.

> For instance, in cases where there are already other things 
> controlling visibility (as are needed for atomic increment, for 
> example) you don't need to make the access itself volatile.

Hrm, you mean within a lock or similar?  You'll get the same semantics
as volatile anyway there.


Segher