From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933416AbbBBTdX (ORCPT ); Mon, 2 Feb 2015 14:33:23 -0500 Received: from e31.co.us.ibm.com ([32.97.110.149]:35441 "EHLO e31.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932222AbbBBTdW (ORCPT ); Mon, 2 Feb 2015 14:33:22 -0500 Date: Mon, 2 Feb 2015 11:33:17 -0800 From: "Paul E. McKenney" To: Peter Hurley Cc: Peter Zijlstra , Kent Overstreet , Sedat Dilek , Dave Jones , Linus Torvalds , LKML , Chris Mason Subject: Re: Linux 3.19-rc3 Message-ID: <20150202193317.GF19109@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <54ABDB4B.7070008@hurleysoftware.com> <20150106173808.GB5280@linux.vnet.ibm.com> <54AC224C.4030903@hurleysoftware.com> <20150106192559.GF5280@linux.vnet.ibm.com> <54AC3E31.2080202@hurleysoftware.com> <20150106204753.GI5280@linux.vnet.ibm.com> <20150120003008.GA14445@linux.vnet.ibm.com> <54BE6020.5060609@hurleysoftware.com> <20150202161105.GB19109@linux.vnet.ibm.com> <54CFCA05.2070404@hurleysoftware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54CFCA05.2070404@hurleysoftware.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15020219-8236-0000-0000-0000091F38FF Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 02, 2015 at 02:03:33PM -0500, Peter Hurley wrote: > 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 > > Signed-off-by: Paul E. McKenney > > > > 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 Good catch, switched to "include". > > +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 Good eyes, fixed. Thanx, Paul > > +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: > > >