[PATCH] Fix memory barrier docs wrt atomic ops

[PATCH] Fix memory barrier docs wrt atomic ops

[David Howells]

Fix the memory barrier documentation to attempt to describe atomic ops
correctly.

atomic_t ops that return a value _do_ imply smp_mb() either side, and so don't
actually require smp_mb__*_atomic_*() special barriers.

Also explains why special barriers exist in addition to normal barriers.

Signed-Off-By: David Howells <dhowells@redhat.com>
---
warthog>diffstat -p1 /tmp/mb.diff 
 Documentation/memory-barriers.txt |   47 +++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index f855031..822fc45 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -829,8 +829,8 @@ There are some more advanced barrier fun
  (*) smp_mb__after_atomic_inc();
 
      These are for use with atomic add, subtract, increment and decrement
-     functions, especially when used for reference counting.  These functions
-     do not imply memory barriers.
+     functions that don't return a value, especially when used for reference
+     counting.  These functions do not imply memory barriers.
 
      As an example, consider a piece of code that marks an object as being dead
      and then decrements the object's reference count:
@@ -1263,15 +1263,15 @@ else.
 ATOMIC OPERATIONS
 -----------------
 
-Though they are technically interprocessor interaction considerations, atomic
-operations are noted specially as they do _not_ generally imply memory
-barriers.  The possible offenders include:
+Whilst they are technically interprocessor interaction considerations, atomic
+operations are noted specially as some of them imply full memory barriers and
+some don't, but they're very heavily relied on as a group throughout the
+kernel.
+
+Any atomic_t operation, for instance, that returns a value implies an
+SMP-conditional general memory barrier (smp_mb()) on each side of the actual
+operation.  These include:
 
-	xchg();
-	cmpxchg();
-	test_and_set_bit();
-	test_and_clear_bit();
-	test_and_change_bit();
 	atomic_cmpxchg();
 	atomic_inc_return();
 	atomic_dec_return();
@@ -1283,20 +1283,30 @@ barriers.  The possible offenders includ
 	atomic_add_negative();
 	atomic_add_unless();
 
-These may be used for such things as implementing LOCK operations or controlling
-the lifetime of objects by decreasing their reference counts.  In such cases
-they need preceding memory barriers.
 
-The following may also be possible offenders as they may be used as UNLOCK
-operations.
+The following, however, do _not_ imply memory barrier effects:
+
+	xchg();
+	cmpxchg();
+	test_and_set_bit();
+	test_and_clear_bit();
+	test_and_change_bit();
+
+These may be used for such things as implementing LOCK-class operations.  In
+such cases they need explicit memory barriers.
+
+The following are also potential offenders as they may be used as UNLOCK-class
+operations, amongst other things, but do _not_ imply memory barriers either:
 
 	set_bit();
 	clear_bit();
 	change_bit();
 	atomic_set();
 
+With these the appropriate explicit memory barrier should be used if necessary.
+
 
-The following are a little tricky:
+The following also don't imply memory barriers, and so may be a little tricky:
 
 	atomic_add();
 	atomic_sub();
@@ -1322,6 +1332,11 @@ operation is protected by a lock, then i
 there's another operation within the critical section with respect to which an
 ordering must be maintained.
 
+[!] Note that special memory barrier primitives are available for these
+situations because on some CPUs the atomic instructions used imply full memory
+barriers, and so barrier instructions are superfluous in conjunction with them,
+and in such cases the special barrier primitives will be no-ops.
+
 See Documentation/atomic_ops.txt for more information.
 
 

Re: [PATCH] Fix memory barrier docs wrt atomic ops

[Nick Piggin]

David Howells wrote:

>  ATOMIC OPERATIONS
>  -----------------
>  
> -Though they are technically interprocessor interaction considerations, atomic
> -operations are noted specially as they do _not_ generally imply memory
> -barriers.  The possible offenders include:
> +Whilst they are technically interprocessor interaction considerations, atomic
> +operations are noted specially as some of them imply full memory barriers and
> +some don't, but they're very heavily relied on as a group throughout the
> +kernel.
> +
> +Any atomic_t operation, for instance, that returns a value implies an
> +SMP-conditional general memory barrier (smp_mb()) on each side of the actual
> +operation.  These include:

Actually: this only applies to operations which _both_ modify their atomic_t
operand and return a value. Eg. atomic_read() does not have barrier semantics.

>  
> -	xchg();
> -	cmpxchg();
> -	test_and_set_bit();
> -	test_and_clear_bit();
> -	test_and_change_bit();
>  	atomic_cmpxchg();
>  	atomic_inc_return();
>  	atomic_dec_return();
> @@ -1283,20 +1283,30 @@ barriers.  The possible offenders includ
>  	atomic_add_negative();
>  	atomic_add_unless();
>  
> -These may be used for such things as implementing LOCK operations or controlling
> -the lifetime of objects by decreasing their reference counts.  In such cases
> -they need preceding memory barriers.
>  
> -The following may also be possible offenders as they may be used as UNLOCK
> -operations.
> +The following, however, do _not_ imply memory barrier effects:
> +
> +	xchg();
> +	cmpxchg();
> +	test_and_set_bit();
> +	test_and_clear_bit();
> +	test_and_change_bit();
> +
> +These may be used for such things as implementing LOCK-class operations.  In
> +such cases they need explicit memory barriers.
> +

I believe all the bitops are essentially the same as the atomic semantics.
That is, if they change their operand and return something, they are full
barriers both ways.

atomic_ops.txt says of them:
   "These routines, like the atomic_t counter operations returning values,
    require explicit memory barrier semantics around their execution."

I think we'd have problems at least with TestSetPageLocked if this were
not the case.

I'm not sure if I like the words imply, explicit, implicit, etc. They're
a bit confusing. provide, semantics may be better?

> +The following are also potential offenders as they may be used as UNLOCK-class
> +operations, amongst other things, but do _not_ imply memory barriers either:
>  
>  	set_bit();
>  	clear_bit();
>  	change_bit();
>  	atomic_set();
>  
> +With these the appropriate explicit memory barrier should be used if necessary.
> +
>  

In particular, when clearing a bit to signal the end of a critical section,
clear_bit must be preceeded by smp_mb__before_clear_bit();

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com 

[PATCH] Futher fix memory barrier docs wrt atomic ops

[David Howells]

Further fix the memory barrier documents to portray bitwise operation memory
barrier effects correctly following Nick Piggin's comments.

It makes the point that any atomic op that both modifies some state in memory
and returns information on that state implies memory barriers on both sides.

This patch depends on:

	[PATCH] Fix memory barrier docs wrt atomic ops 

Signed-Off-By: David Howells <dhowells@redhat.com>
---
warthog>diffstat -p1 /tmp/mb.diff 
 Documentation/memory-barriers.txt |   39 ++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 822fc45..92f0056 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1282,10 +1282,12 @@ operations are noted specially as some o
 some don't, but they're very heavily relied on as a group throughout the
 kernel.
 
-Any atomic_t operation, for instance, that returns a value implies an
-SMP-conditional general memory barrier (smp_mb()) on each side of the actual
-operation.  These include:
+Any atomic operation that modifies some state in memory and returns information
+about the state (old or new) implies an SMP-conditional general memory barrier
+(smp_mb()) on each side of the actual operation.  These include:
 
+	xchg();
+	cmpxchg();
 	atomic_cmpxchg();
 	atomic_inc_return();
 	atomic_dec_return();
@@ -1296,31 +1298,31 @@ operation.  These include:
 	atomic_sub_and_test();
 	atomic_add_negative();
 	atomic_add_unless();
-
-
-The following, however, do _not_ imply memory barrier effects:
-
-	xchg();
-	cmpxchg();
 	test_and_set_bit();
 	test_and_clear_bit();
 	test_and_change_bit();
 
-These may be used for such things as implementing LOCK-class operations.  In
-such cases they need explicit memory barriers.
+These are used for such things as implementing LOCK-class and UNLOCK-class
+operations and adjusting reference counters towards object destruction, and as
+such the implicit memory barrier effects are necessary.
+
 
-The following are also potential offenders as they may be used as UNLOCK-class
-operations, amongst other things, but do _not_ imply memory barriers either:
+The following operation are potential problems as they do _not_ imply memory
+barriers, but might be used for implementing such things as UNLOCK-class
+operations:
 
+	atomic_set();
 	set_bit();
 	clear_bit();
 	change_bit();
-	atomic_set();
 
-With these the appropriate explicit memory barrier should be used if necessary.
+With these the appropriate explicit memory barrier should be used if necessary
+(smp_mb__before_clear_bit() for instance).
 
 
-The following also don't imply memory barriers, and so may be a little tricky:
+The following also do _not_ imply memory barriers, and so may require explicit
+memory barriers under some circumstances (smp_mb__before_atomic_dec() for
+instance)):
 
 	atomic_add();
 	atomic_sub();
@@ -1341,10 +1343,7 @@ specific order.
 
 
 Basically, each usage case has to be carefully considered as to whether memory
-barriers are needed or not.  The simplest rule is probably: if the atomic
-operation is protected by a lock, then it does not require a barrier unless
-there's another operation within the critical section with respect to which an
-ordering must be maintained.
+barriers are needed or not.
 
 [!] Note that special memory barrier primitives are available for these
 situations because on some CPUs the atomic instructions used imply full memory