Re: [PATCH tip/core/rcu 2/3] srcu: Force full grace-period ordering
From: Ingo Molnar <mingo@kernel.org>
Date: 2017-01-15 07:57:18
Also in:
lkml
* Paul E. McKenney [off-list ref] wrote:
On Sun, Jan 15, 2017 at 08:11:23AM +0100, Ingo Molnar wrote:quoted
* Paul E. McKenney [off-list ref] wrote:quoted
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 [off-list ref]
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 [off-list ref]
Cc: Will Deacon [off-list ref]
Cc: Peter Zijlstra [off-list ref]
Cc: Benjamin Herrenschmidt [off-list ref]
Cc: "linux-arch@vger.kernel.org" [off-list ref]
So I concur and I'm fine with your patch - or with the status quo code as well.
Thanks,
Ingo