Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Paul E. McKenney]

On Sat, Jan 14, 2017 at 11:54:17AM -0800, Paul E. McKenney wrote:
> On Sat, Jan 14, 2017 at 10:35:50AM +0100, Ingo Molnar wrote:
> > * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

[ . . . ]

> > > + */
> > > +#ifdef CONFIG_PPC
> > > +#define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
> > > +#else /* #ifdef CONFIG_PPC */
> > > +#define smp_mb__after_unlock_lock()	do { } while (0)
> > > +#endif /* #else #ifdef CONFIG_PPC */
> > 
> > Yeah, so I realize that this was pre-existing code, but putting CONFIG_$ARCH
> > #ifdefs into generic headers is generally frowned upon.
> > 
> > The canonical approach would be either to define a helper Kconfig variable that 
> > can be set by PPC (but other architectures don't need to set it), or to expose a 
> > suitable macro (function) for architectures to define in their barrier.h arch 
> > header file.
> 
> Very well, I will add a separate commit for this.  4.11 OK?

Does the patch below seem reasonable?

							Thanx, Paul

------------------------------------------------------------------------

commit 271c0601237c41a279f975563e13837bace0df03
Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Date:   Sat Jan 14 13:32:50 2017 -0800

    rcu: Make arch select smp_mb__after_unlock_lock() strength
    
    The definition of smp_mb__after_unlock_lock() is currently smp_mb()
    for CONFIG_PPC and a no-op otherwise.  It would be better to instead
    provide an architecture-selectable Kconfig option, and select the
    strength of smp_mb__after_unlock_lock() based on that option.  This
    commit therefore creates CONFIG_ARCH_WEAK_RELACQ, has PPC select it,
    and bases the definition of smp_mb__after_unlock_lock() on this new
    CONFIG_ARCH_WEAK_RELACQ Kconfig option.
    
    Reported-by: Ingo Molnar <mingo@kernel.org>
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Will Deacon <will.deacon@arm.com>
    Cc: Boqun Feng <boqun.feng@linux.vnet.ibm.com>
    Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
    Cc: Paul Mackerras <paulus@samba.org>
    Cc: Michael Ellerman <mpe@ellerman.id.au>
    Cc: <linuxppc-dev@lists.ozlabs.org>

diff --git a/arch/Kconfig b/arch/Kconfig
index 99839c23d453..94dd90d33f95 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -316,6 +316,9 @@ config HAVE_CMPXCHG_LOCAL
 config HAVE_CMPXCHG_DOUBLE
 	bool
 
+config ARCH_WEAK_RELACQ
+	bool
+
 config ARCH_WANT_IPC_PARSE_VERSION
 	bool
 
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..e7083d27271e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -165,6 +165,7 @@ config PPC
 	select HAVE_ARCH_HARDENED_USERCOPY
 	select HAVE_KERNEL_GZIP
 	select HAVE_CC_STACKPROTECTOR
+	select ARCH_WEAK_RELACQ
 
 config GENERIC_CSUM
 	def_bool CPU_LITTLE_ENDIAN
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 357b32aaea48..5fdfe874229e 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -1175,11 +1175,11 @@ do { \
  * if the UNLOCK and LOCK are executed by the same CPU or if the
  * UNLOCK and LOCK operate on the same lock variable.
  */
-#ifdef CONFIG_PPC
+#ifdef CONFIG_ARCH_WEAK_RELACQ
 #define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
-#else /* #ifdef CONFIG_PPC */
+#else /* #ifdef CONFIG_ARCH_WEAK_RELACQ */
 #define smp_mb__after_unlock_lock()	do { } while (0)
-#endif /* #else #ifdef CONFIG_PPC */
+#endif /* #else #ifdef CONFIG_ARCH_WEAK_RELACQ */
 
 
 #endif /* __LINUX_RCUPDATE_H */

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Ingo Molnar]

* Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 357b32aaea48..5fdfe874229e 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -1175,11 +1175,11 @@ do { \
>   * if the UNLOCK and LOCK are executed by the same CPU or if the
>   * UNLOCK and LOCK operate on the same lock variable.
>   */
> -#ifdef CONFIG_PPC
> +#ifdef CONFIG_ARCH_WEAK_RELACQ
>  #define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
> -#else /* #ifdef CONFIG_PPC */
> +#else /* #ifdef CONFIG_ARCH_WEAK_RELACQ */
>  #define smp_mb__after_unlock_lock()	do { } while (0)
> -#endif /* #else #ifdef CONFIG_PPC */
> +#endif /* #else #ifdef CONFIG_ARCH_WEAK_RELACQ */
>  
>  

So at the risk of sounding totally pedantic, why not structure it like the 
existing smp_mb__before/after*() primitives in barrier.h?

That allows asm-generic/barrier.h to pick up the definition - for example in the 
case of smp_acquire__after_ctrl_dep() we do:

 #ifndef smp_acquire__after_ctrl_dep
 #define smp_acquire__after_ctrl_dep()           smp_rmb()
 #endif

Which allows Tile to relax it:

  arch/tile/include/asm/barrier.h:#define smp_acquire__after_ctrl_dep()   barrier()

I.e. I'd move the API definition out of rcupdate.h and into barrier.h - even 
though tree-RCU is the only user of this barrier type.

Thanks,

	Ingo

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Paul E. McKenney]

On Sun, Jan 15, 2017 at 08:11:23AM +0100, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > index 357b32aaea48..5fdfe874229e 100644
> > --- a/include/linux/rcupdate.h
> > +++ b/include/linux/rcupdate.h
> > @@ -1175,11 +1175,11 @@ do { \
> >   * if the UNLOCK and LOCK are executed by the same CPU or if the
> >   * UNLOCK and LOCK operate on the same lock variable.
> >   */
> > -#ifdef CONFIG_PPC
> > +#ifdef CONFIG_ARCH_WEAK_RELACQ
> >  #define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
> > -#else /* #ifdef CONFIG_PPC */
> > +#else /* #ifdef CONFIG_ARCH_WEAK_RELACQ */
> >  #define smp_mb__after_unlock_lock()	do { } while (0)
> > -#endif /* #else #ifdef CONFIG_PPC */
> > +#endif /* #else #ifdef CONFIG_ARCH_WEAK_RELACQ */
> >  
> >  
> 
> So at the risk of sounding totally pedantic, why not structure it like the 
> existing smp_mb__before/after*() primitives in barrier.h?
> 
> That allows asm-generic/barrier.h to pick up the definition - for example in the 
> case of smp_acquire__after_ctrl_dep() we do:
> 
>  #ifndef smp_acquire__after_ctrl_dep
>  #define smp_acquire__after_ctrl_dep()           smp_rmb()
>  #endif
> 
> Which allows Tile to relax it:
> 
>   arch/tile/include/asm/barrier.h:#define smp_acquire__after_ctrl_dep()   barrier()
> 
> I.e. I'd move the API definition out of rcupdate.h and into barrier.h - even 
> though tree-RCU is the only user of this barrier type.

I wouldn't have any problem with that, however, some time back it was
moved into RCU because (you guessed it!) RCU is the only user.  ;-)

							Thanx, Paul

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Ingo Molnar]

* Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> On Sun, Jan 15, 2017 at 08:11:23AM +0100, Ingo Molnar wrote:
> > 
> > * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > index 357b32aaea48..5fdfe874229e 100644
> > > --- a/include/linux/rcupdate.h
> > > +++ b/include/linux/rcupdate.h
> > > @@ -1175,11 +1175,11 @@ do { \
> > >   * if the UNLOCK and LOCK are executed by the same CPU or if the
> > >   * UNLOCK and LOCK operate on the same lock variable.
> > >   */
> > > -#ifdef CONFIG_PPC
> > > +#ifdef CONFIG_ARCH_WEAK_RELACQ
> > >  #define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
> > > -#else /* #ifdef CONFIG_PPC */
> > > +#else /* #ifdef CONFIG_ARCH_WEAK_RELACQ */
> > >  #define smp_mb__after_unlock_lock()	do { } while (0)
> > > -#endif /* #else #ifdef CONFIG_PPC */
> > > +#endif /* #else #ifdef CONFIG_ARCH_WEAK_RELACQ */
> > >  
> > >  
> > 
> > So at the risk of sounding totally pedantic, why not structure it like the 
> > existing smp_mb__before/after*() primitives in barrier.h?
> > 
> > That allows asm-generic/barrier.h to pick up the definition - for example in the 
> > case of smp_acquire__after_ctrl_dep() we do:
> > 
> >  #ifndef smp_acquire__after_ctrl_dep
> >  #define smp_acquire__after_ctrl_dep()           smp_rmb()
> >  #endif
> > 
> > Which allows Tile to relax it:
> > 
> >   arch/tile/include/asm/barrier.h:#define smp_acquire__after_ctrl_dep()   barrier()
> > 
> > I.e. I'd move the API definition out of rcupdate.h and into barrier.h - even 
> > though tree-RCU is the only user of this barrier type.
> 
> I wouldn't have any problem with that, however, some time back it was
> moved into RCU because (you guessed it!) RCU is the only user.  ;-)

Indeed ...

[sounds of rummaging around in the Git tree]

I found this commit of yours from ancient history (more than a year ago!):

  commit 12d560f4ea87030667438a169912380be00cea4b
  Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
  Date:   Tue Jul 14 18:35:23 2015 -0700

    rcu,locking: Privatize smp_mb__after_unlock_lock()
    
    RCU is the only thing that uses smp_mb__after_unlock_lock(), and is
    likely the only thing that ever will use it, so this commit makes this
    macro private to RCU.
    
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Will Deacon <will.deacon@arm.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
    Cc: "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>

So I concur and I'm fine with your patch - or with the status quo code as well.

Thanks,

	Ingo

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Paul E. McKenney]

On Sun, Jan 15, 2017 at 08:57:11AM +0100, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > On Sun, Jan 15, 2017 at 08:11:23AM +0100, Ingo Molnar wrote:
> > > 
> > > * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > > 
> > > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > > index 357b32aaea48..5fdfe874229e 100644
> > > > --- a/include/linux/rcupdate.h
> > > > +++ b/include/linux/rcupdate.h
> > > > @@ -1175,11 +1175,11 @@ do { \
> > > >   * if the UNLOCK and LOCK are executed by the same CPU or if the
> > > >   * UNLOCK and LOCK operate on the same lock variable.
> > > >   */
> > > > -#ifdef CONFIG_PPC
> > > > +#ifdef CONFIG_ARCH_WEAK_RELACQ
> > > >  #define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
> > > > -#else /* #ifdef CONFIG_PPC */
> > > > +#else /* #ifdef CONFIG_ARCH_WEAK_RELACQ */
> > > >  #define smp_mb__after_unlock_lock()	do { } while (0)
> > > > -#endif /* #else #ifdef CONFIG_PPC */
> > > > +#endif /* #else #ifdef CONFIG_ARCH_WEAK_RELACQ */
> > > >  
> > > >  
> > > 
> > > So at the risk of sounding totally pedantic, why not structure it like the 
> > > existing smp_mb__before/after*() primitives in barrier.h?
> > > 
> > > That allows asm-generic/barrier.h to pick up the definition - for example in the 
> > > case of smp_acquire__after_ctrl_dep() we do:
> > > 
> > >  #ifndef smp_acquire__after_ctrl_dep
> > >  #define smp_acquire__after_ctrl_dep()           smp_rmb()
> > >  #endif
> > > 
> > > Which allows Tile to relax it:
> > > 
> > >   arch/tile/include/asm/barrier.h:#define smp_acquire__after_ctrl_dep()   barrier()
> > > 
> > > I.e. I'd move the API definition out of rcupdate.h and into barrier.h - even 
> > > though tree-RCU is the only user of this barrier type.
> > 
> > I wouldn't have any problem with that, however, some time back it was
> > moved into RCU because (you guessed it!) RCU is the only user.  ;-)
> 
> Indeed ...
> 
> [sounds of rummaging around in the Git tree]
> 
> I found this commit of yours from ancient history (more than a year ago!):
> 
>   commit 12d560f4ea87030667438a169912380be00cea4b
>   Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>   Date:   Tue Jul 14 18:35:23 2015 -0700
> 
>     rcu,locking: Privatize smp_mb__after_unlock_lock()
>     
>     RCU is the only thing that uses smp_mb__after_unlock_lock(), and is
>     likely the only thing that ever will use it, so this commit makes this
>     macro private to RCU.
>     
>     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>     Cc: Will Deacon <will.deacon@arm.com>
>     Cc: Peter Zijlstra <peterz@infradead.org>
>     Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>     Cc: "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>
> 
> So I concur and I'm fine with your patch - or with the status quo code as well.

I already have the patch queued, so how about I keep it if I get an ack
from the powerpc guys and drop it otherwise?

							Thanx, Paul

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Ingo Molnar]

* Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> > [sounds of rummaging around in the Git tree]
> > 
> > I found this commit of yours from ancient history (more than a year ago!):
> > 
> >   commit 12d560f4ea87030667438a169912380be00cea4b
> >   Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >   Date:   Tue Jul 14 18:35:23 2015 -0700
> > 
> >     rcu,locking: Privatize smp_mb__after_unlock_lock()
> >     
> >     RCU is the only thing that uses smp_mb__after_unlock_lock(), and is
> >     likely the only thing that ever will use it, so this commit makes this
> >     macro private to RCU.
> >     
> >     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >     Cc: Will Deacon <will.deacon@arm.com>
> >     Cc: Peter Zijlstra <peterz@infradead.org>
> >     Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >     Cc: "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>
> > 
> > So I concur and I'm fine with your patch - or with the status quo code as well.
> 
> I already have the patch queued, so how about I keep it if I get an ack
> from the powerpc guys and drop it otherwise?

Yeah, sounds good! Your patch made me look up 'RelAcq' so it has documentation 
value as well ;-)

Thanks,

	Ingo

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Paul E. McKenney]

On Sun, Jan 15, 2017 at 10:40:58AM +0100, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > > [sounds of rummaging around in the Git tree]
> > > 
> > > I found this commit of yours from ancient history (more than a year ago!):
> > > 
> > >   commit 12d560f4ea87030667438a169912380be00cea4b
> > >   Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > >   Date:   Tue Jul 14 18:35:23 2015 -0700
> > > 
> > >     rcu,locking: Privatize smp_mb__after_unlock_lock()
> > >     
> > >     RCU is the only thing that uses smp_mb__after_unlock_lock(), and is
> > >     likely the only thing that ever will use it, so this commit makes this
> > >     macro private to RCU.
> > >     
> > >     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > >     Cc: Will Deacon <will.deacon@arm.com>
> > >     Cc: Peter Zijlstra <peterz@infradead.org>
> > >     Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > >     Cc: "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>
> > > 
> > > So I concur and I'm fine with your patch - or with the status quo code as well.
> > 
> > I already have the patch queued, so how about I keep it if I get an ack
> > from the powerpc guys and drop it otherwise?
> 
> Yeah, sounds good! Your patch made me look up 'RelAcq' so it has documentation 
> value as well ;-)

;-) ;-) ;-)

Looking forward, my guess would be that if some other code needs
smp_mb__after_unlock_lock() or if some other architecture needs
non-smb_mb() special handling, I should consider making it work the
same as smp_mb__after_atomic() and friends.  Does that seem like a
reasonable thought?

							Thanx, Paul

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Ingo Molnar]

* Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> On Sun, Jan 15, 2017 at 10:40:58AM +0100, Ingo Molnar wrote:
> > 
> > * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> > 
> > > > [sounds of rummaging around in the Git tree]
> > > > 
> > > > I found this commit of yours from ancient history (more than a year ago!):
> > > > 
> > > >   commit 12d560f4ea87030667438a169912380be00cea4b
> > > >   Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > >   Date:   Tue Jul 14 18:35:23 2015 -0700
> > > > 
> > > >     rcu,locking: Privatize smp_mb__after_unlock_lock()
> > > >     
> > > >     RCU is the only thing that uses smp_mb__after_unlock_lock(), and is
> > > >     likely the only thing that ever will use it, so this commit makes this
> > > >     macro private to RCU.
> > > >     
> > > >     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > >     Cc: Will Deacon <will.deacon@arm.com>
> > > >     Cc: Peter Zijlstra <peterz@infradead.org>
> > > >     Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > >     Cc: "linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>
> > > > 
> > > > So I concur and I'm fine with your patch - or with the status quo code as well.
> > > 
> > > I already have the patch queued, so how about I keep it if I get an ack
> > > from the powerpc guys and drop it otherwise?
> > 
> > Yeah, sounds good! Your patch made me look up 'RelAcq' so it has documentation 
> > value as well ;-)
> 
> ;-) ;-) ;-)
> 
> Looking forward, my guess would be that if some other code needs
> smp_mb__after_unlock_lock() or if some other architecture needs
> non-smb_mb() special handling, I should consider making it work the
> same as smp_mb__after_atomic() and friends.  Does that seem like a
> reasonable thought?

Yeah, absolutely - it's just that the pattern triggered the 'this looks a bit too 
specialized' response in me, but after seeing the details (again ...) I agree that 
this time is different!

Thanks,

	Ingo

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Michael Ellerman]

"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> writes:

> On Sat, Jan 14, 2017 at 11:54:17AM -0800, Paul E. McKenney wrote:
>> On Sat, Jan 14, 2017 at 10:35:50AM +0100, Ingo Molnar wrote:
>> > * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
>
> [ . . . ]
>
>> > > + */
>> > > +#ifdef CONFIG_PPC
>> > > +#define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
>> > > +#else /* #ifdef CONFIG_PPC */
>> > > +#define smp_mb__after_unlock_lock()	do { } while (0)
>> > > +#endif /* #else #ifdef CONFIG_PPC */
>> > 
>> > Yeah, so I realize that this was pre-existing code, but putting CONFIG_$ARCH
>> > #ifdefs into generic headers is generally frowned upon.
>> > 
>> > The canonical approach would be either to define a helper Kconfig variable that 
>> > can be set by PPC (but other architectures don't need to set it), or to expose a 
>> > suitable macro (function) for architectures to define in their barrier.h arch 
>> > header file.
>> 
>> Very well, I will add a separate commit for this.  4.11 OK?
>
> Does the patch below seem reasonable?
>
> 							Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit 271c0601237c41a279f975563e13837bace0df03
> Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Date:   Sat Jan 14 13:32:50 2017 -0800
>
>     rcu: Make arch select smp_mb__after_unlock_lock() strength
>     
>     The definition of smp_mb__after_unlock_lock() is currently smp_mb()
>     for CONFIG_PPC and a no-op otherwise.  It would be better to instead
>     provide an architecture-selectable Kconfig option, and select the
>     strength of smp_mb__after_unlock_lock() based on that option.  This
>     commit therefore creates CONFIG_ARCH_WEAK_RELACQ, has PPC select it,
>     and bases the definition of smp_mb__after_unlock_lock() on this new
>     CONFIG_ARCH_WEAK_RELACQ Kconfig option.
>     
>     Reported-by: Ingo Molnar <mingo@kernel.org>
>     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>     Cc: Peter Zijlstra <peterz@infradead.org>
>     Cc: Will Deacon <will.deacon@arm.com>
>     Cc: Boqun Feng <boqun.feng@linux.vnet.ibm.com>
>     Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>     Cc: Paul Mackerras <paulus@samba.org>
>     Cc: Michael Ellerman <mpe@ellerman.id.au>
>     Cc: <linuxppc-dev@lists.ozlabs.org>

Personally I'd call it ARCH_WEAK_RELEASE_ACQUIRE, which is longer but
clearer I think. But it's not a big deal, so which ever you prefer.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering

[Paul E. McKenney]

On Mon, Jan 23, 2017 at 07:12:03PM +1100, Michael Ellerman wrote:
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> writes:
> 
> > On Sat, Jan 14, 2017 at 11:54:17AM -0800, Paul E. McKenney wrote:
> >> On Sat, Jan 14, 2017 at 10:35:50AM +0100, Ingo Molnar wrote:
> >> > * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> >
> > [ . . . ]
> >
> >> > > + */
> >> > > +#ifdef CONFIG_PPC
> >> > > +#define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
> >> > > +#else /* #ifdef CONFIG_PPC */
> >> > > +#define smp_mb__after_unlock_lock()	do { } while (0)
> >> > > +#endif /* #else #ifdef CONFIG_PPC */
> >> > 
> >> > Yeah, so I realize that this was pre-existing code, but putting CONFIG_$ARCH
> >> > #ifdefs into generic headers is generally frowned upon.
> >> > 
> >> > The canonical approach would be either to define a helper Kconfig variable that 
> >> > can be set by PPC (but other architectures don't need to set it), or to expose a 
> >> > suitable macro (function) for architectures to define in their barrier.h arch 
> >> > header file.
> >> 
> >> Very well, I will add a separate commit for this.  4.11 OK?
> >
> > Does the patch below seem reasonable?
> >
> > 							Thanx, Paul
> >
> > ------------------------------------------------------------------------
> >
> > commit 271c0601237c41a279f975563e13837bace0df03
> > Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Date:   Sat Jan 14 13:32:50 2017 -0800
> >
> >     rcu: Make arch select smp_mb__after_unlock_lock() strength
> >     
> >     The definition of smp_mb__after_unlock_lock() is currently smp_mb()
> >     for CONFIG_PPC and a no-op otherwise.  It would be better to instead
> >     provide an architecture-selectable Kconfig option, and select the
> >     strength of smp_mb__after_unlock_lock() based on that option.  This
> >     commit therefore creates CONFIG_ARCH_WEAK_RELACQ, has PPC select it,
> >     and bases the definition of smp_mb__after_unlock_lock() on this new
> >     CONFIG_ARCH_WEAK_RELACQ Kconfig option.
> >     
> >     Reported-by: Ingo Molnar <mingo@kernel.org>
> >     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> >     Cc: Peter Zijlstra <peterz@infradead.org>
> >     Cc: Will Deacon <will.deacon@arm.com>
> >     Cc: Boqun Feng <boqun.feng@linux.vnet.ibm.com>
> >     Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >     Cc: Paul Mackerras <paulus@samba.org>
> >     Cc: Michael Ellerman <mpe@ellerman.id.au>
> >     Cc: <linuxppc-dev@lists.ozlabs.org>
> 
> Personally I'd call it ARCH_WEAK_RELEASE_ACQUIRE, which is longer but
> clearer I think. But it's not a big deal, so which ever you prefer.

ARCH_WEAK_RELEASE_ACQUIRE it is!

> Acked-by: Michael Ellerman <mpe@ellerman.id.au>

Applied, thank you!

							Thanx, Paul