Date:	Mon, 02 Feb 2015 14:03:33 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	paulmck@...ux.vnet.ibm.com
CC:	Peter Zijlstra <peterz@...radead.org>,
	Kent Overstreet <kmo@...erainc.com>,
	Sedat Dilek <sedat.dilek@...il.com>,
	Dave Jones <davej@...emonkey.org.uk>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, Chris Mason <clm@...com>
Subject: Re: Linux 3.19-rc3

On 02/02/2015 11:11 AM, Paul E. McKenney wrote:
> On Tue, Jan 20, 2015 at 09:03:12AM -0500, Peter Hurley wrote:
>> On 01/19/2015 07:30 PM, Paul E. McKenney wrote:
>>> On Tue, Jan 06, 2015 at 12:47:53PM -0800, Paul E. McKenney wrote:
>>>> On Tue, Jan 06, 2015 at 02:57:37PM -0500, Peter Hurley wrote:
>>>
>>> [ . . . ]
>>>
>>>> David Miller's call, actually.
>>>>
>>>> But the rule is that if it is an atomic read-modify-write operation and it
>>>> returns a value, then the operation itself needs to include full memory
>>>> barriers before and after (as in the caller doesn't need to add them).
>>>> Otherwise, the operation does not need to include memory ordering.
>>>> Since xchg(), atomic_xchg(), and atomic_long_xchg() all return a value,
>>>> their implementations must include full memory barriers before and after.
>>>>
>>>> Pretty straightforward.  ;-)
>>>
>>> Hello again, Peter,
>>>
>>> Were you going to push a patch clarifying this?
>>
>> Hi Paul,
>>
>> As you pointed out, atomic_ops.txt is for arch implementors, so I wasn't
>> planning on patching that file.
>>
>> I've been meaning to write up something specifically for everyone else but
>> my own bugs have kept me from that. [That, and I'm not sure what I write
>> will be suitable for Documentation.]
> 
> Well, upon revisiting this after coming back from travel, I am more inclined
> to agree that a change would be good.  I doubt if you would be the only
> person who might be confused.  So how about the following patch?

I think this is much clearer to both audiences. Thanks!

Regards,
Peter Hurley

[minor corrections below]

> ------------------------------------------------------------------------
> 
> documentation: Clarify memory-barrier semantics of atomic operations
> 
> All value-returning atomic read-modify-write operations must provide full
> memory-barrier semantics on both sides of the operation.  This commit
> clarifies the documentation to make it clear that these memory-barrier
> semantics are provided by the operations themselves, not by their callers.
> 
> Reported-by: Peter Hurley <peter@...leysoftware.com>
> Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> 
> diff --git a/Documentation/atomic_ops.txt b/Documentation/atomic_ops.txt
> index 183e41bdcb69..672201e79c49 100644
> --- a/Documentation/atomic_ops.txt
> +++ b/Documentation/atomic_ops.txt
> @@ -201,11 +201,11 @@ These routines add 1 and subtract 1, respectively, from the given
>  atomic_t and return the new counter value after the operation is
>  performed.
>  
> -Unlike the above routines, it is required that explicit memory
> -barriers are performed before and after the operation.  It must be
> -done such that all memory operations before and after the atomic
> -operation calls are strongly ordered with respect to the atomic
> -operation itself.
> +Unlike the above routines, it is required that these primitives
> +inlude explicit memory barriers that are performed before and after
   ^^^^^^
   include|provide
> +the operation.  It must be done such that all memory operations before
> +and after the atomic operation calls are strongly ordered with respect
> +to the atomic operation itself.
>  
>  For example, it should behave as if a smp_mb() call existed both
>  before and after the atomic operation.
> @@ -233,21 +233,21 @@ These two routines increment and decrement by 1, respectively, the
>  given atomic counter.  They return a boolean indicating whether the
>  resulting counter value was zero or not.
>  
> -It requires explicit memory barrier semantics around the operation as
> -above.
> +Again, these primitive provide explicit memory barrier semantics around
                         ^
                         s
> +the atomic operation.
>  
>  	int atomic_sub_and_test(int i, atomic_t *v);
>  
>  This is identical to atomic_dec_and_test() except that an explicit
> -decrement is given instead of the implicit "1".  It requires explicit
> -memory barrier semantics around the operation.
> +decrement is given instead of the implicit "1".  This primitive must
> +provide explicit memory barrier semantics around the operation.
>  
>  	int atomic_add_negative(int i, atomic_t *v);
>  
> -The given increment is added to the given atomic counter value.  A
> -boolean is return which indicates whether the resulting counter value
> -is negative.  It requires explicit memory barrier semantics around the
> -operation.
> +The given increment is added to the given atomic counter value.  A boolean
> +is return which indicates whether the resulting counter value is negative.
> +This primitive must provide explicit memory barrier semantics around
> +the operation.
>  
>  Then:
>  
> @@ -257,7 +257,7 @@ This performs an atomic exchange operation on the atomic variable v, setting
>  the given new value.  It returns the old value that the atomic variable v had
>  just before the operation.
>  
> -atomic_xchg requires explicit memory barriers around the operation.
> +atomic_xchg must provide explicit memory barriers around the operation.
>  
>  	int atomic_cmpxchg(atomic_t *v, int old, int new);
>  
> @@ -266,7 +266,7 @@ with the given old and new values. Like all atomic_xxx operations,
>  atomic_cmpxchg will only satisfy its atomicity semantics as long as all
>  other accesses of *v are performed through atomic_xxx operations.
>  
> -atomic_cmpxchg requires explicit memory barriers around the operation.
> +atomic_cmpxchg must provide explicit memory barriers around the operation.
>  
>  The semantics for atomic_cmpxchg are the same as those defined for 'cas'
>  below.
> @@ -279,8 +279,8 @@ If the atomic value v is not equal to u, this function adds a to v, and
>  returns non zero. If v is equal to u then it returns zero. This is done as
>  an atomic operation.
>  
> -atomic_add_unless requires explicit memory barriers around the operation
> -unless it fails (returns 0).
> +atomic_add_unless must provide explicit memory barriers around the
> +operation unless it fails (returns 0).
>  
>  atomic_inc_not_zero, equivalent to atomic_add_unless(v, 1, 0)
>  
> @@ -460,9 +460,9 @@ the return value into an int.  There are other places where things
>  like this occur as well.
>  
>  These routines, like the atomic_t counter operations returning values,
> -require explicit memory barrier semantics around their execution.  All
> -memory operations before the atomic bit operation call must be made
> -visible globally before the atomic bit operation is made visible.
> +must provide explicit memory barrier semantics around their execution.
> +All memory operations before the atomic bit operation call must be
> +made visible globally before the atomic bit operation is made visible.
>  Likewise, the atomic bit operation must be visible globally before any
>  subsequent memory operation is made visible.  For example:
>  
> @@ -536,8 +536,9 @@ except that two underscores are prefixed to the interface name.
>  These non-atomic variants also do not require any special memory
>  barrier semantics.
>  
> -The routines xchg() and cmpxchg() need the same exact memory barriers
> -as the atomic and bit operations returning values.
> +The routines xchg() and cmpxchg() must provide the same exact
> +memory-barrier semantics as the atomic and bit operations returning
> +values.
>  
>  Spinlocks and rwlocks have memory barrier expectations as well.
>  The rule to follow is simple:
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists