Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1206109 > unrolled thread

Re: [RFC PATCH v2] memory-barriers: remove smp_mb__after_unlock_lock()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() Will Deacon <will.deacon@arm.com> - 2015-08-12 15:50 +0200
    Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-08-12 17:50 +0200
      Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-08-12 20:00 +0200
        Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() Will Deacon <will.deacon@arm.com> - 2015-08-13 12:50 +0200
          Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-08-13 15:20 +0200
      Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() Michael Ellerman <mpe@ellerman.id.au> - 2015-08-17 06:10 +0200
        Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-08-17 08:20 +0200
          Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() Will Deacon <will.deacon@arm.com> - 2015-08-17 11:00 +0200
            Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() Michael Ellerman <mpe@ellerman.id.au> - 2015-08-18 04:00 +0200
              Re: [RFC PATCH v2] memory-barriers: remove  smp_mb__after_unlock_lock() Will Deacon <will.deacon@arm.com> - 2015-08-18 10:40 +0200

#1206109 — Re: [RFC PATCH v2] memory-barriers: remove smp_mb__after_unlock_lock()

Hello Paul,

On Fri, Jul 24, 2015 at 04:30:46PM +0100, Paul E. McKenney wrote:
> On Fri, Jul 24, 2015 at 12:31:01PM +0100, Will Deacon wrote:
> > On Wed, Jul 15, 2015 at 02:12:21PM +0100, Paul E. McKenney wrote:
> > > > > commit 695c05d4b9666c50b40a1c022678b5f6e2e3e771
> > > > > 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>
> > 
> > Are you planning to queue this somewhere? I think it makes sense regardless
> > of whether we change PowerPc or not and ideally it would be merged around
> > the same time as my relaxed atomics series.
> 
> I have is in -rcu.  By default, I will push it to the 4.4 merge window.
> Please let me know if you need it sooner.

The generic relaxed atomics are now queued in -tip, so it would be really
good to see this Documentation update land in 4.3 if at all possible. I
appreciate it's late in the cycle, but it's always worth asking.

Thanks,

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

[toc] | [next] | [standalone]

#1206217

On Wed, Aug 12, 2015 at 02:44:15PM +0100, Will Deacon wrote:
> Hello Paul,
> 
> On Fri, Jul 24, 2015 at 04:30:46PM +0100, Paul E. McKenney wrote:
> > On Fri, Jul 24, 2015 at 12:31:01PM +0100, Will Deacon wrote:
> > > On Wed, Jul 15, 2015 at 02:12:21PM +0100, Paul E. McKenney wrote:
> > > > > > commit 695c05d4b9666c50b40a1c022678b5f6e2e3e771
> > > > > > 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>
> > > 
> > > Are you planning to queue this somewhere? I think it makes sense regardless
> > > of whether we change PowerPc or not and ideally it would be merged around
> > > the same time as my relaxed atomics series.
> > 
> > I have is in -rcu.  By default, I will push it to the 4.4 merge window.
> > Please let me know if you need it sooner.
> 
> The generic relaxed atomics are now queued in -tip, so it would be really
> good to see this Documentation update land in 4.3 if at all possible. I
> appreciate it's late in the cycle, but it's always worth asking.

Can't hurt to give it a try.  I have set -rcu's rcu/next branch to this
commit, and if it passes a few day's worth of testing, I will see what
Ingo has to say about a pull request.

This commit also privatizes smp_mb__after_unlock_lock() as well as
updating documentation.  Looks like we need to strengthen powerpc's
locking primitives, then get rid of smp_mb__after_unlock_lock() entirely.
Or did that already happen and I just missed it?

							Thanx, Paul

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

[toc] | [prev] | [next] | [standalone]

#1206306

On Wed, Aug 12, 2015 at 08:43:46AM -0700, Paul E. McKenney wrote:
> On Wed, Aug 12, 2015 at 02:44:15PM +0100, Will Deacon wrote:
> > Hello Paul,
> > 
> > On Fri, Jul 24, 2015 at 04:30:46PM +0100, Paul E. McKenney wrote:
> > > On Fri, Jul 24, 2015 at 12:31:01PM +0100, Will Deacon wrote:
> > > > On Wed, Jul 15, 2015 at 02:12:21PM +0100, Paul E. McKenney wrote:
> > > > > > > commit 695c05d4b9666c50b40a1c022678b5f6e2e3e771
> > > > > > > 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>
> > > > 
> > > > Are you planning to queue this somewhere? I think it makes sense regardless
> > > > of whether we change PowerPc or not and ideally it would be merged around
> > > > the same time as my relaxed atomics series.
> > > 
> > > I have is in -rcu.  By default, I will push it to the 4.4 merge window.
> > > Please let me know if you need it sooner.
> > 
> > The generic relaxed atomics are now queued in -tip, so it would be really
> > good to see this Documentation update land in 4.3 if at all possible. I
> > appreciate it's late in the cycle, but it's always worth asking.
> 
> Can't hurt to give it a try.  I have set -rcu's rcu/next branch to this
> commit, and if it passes a few day's worth of testing, I will see what
> Ingo has to say about a pull request.
> 
> This commit also privatizes smp_mb__after_unlock_lock() as well as
> updating documentation.  Looks like we need to strengthen powerpc's
> locking primitives, then get rid of smp_mb__after_unlock_lock() entirely.
> Or did that already happen and I just missed it?

And just for completeness, here is the current version of that commit.

							Thanx, Paul

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

 b/Documentation/memory-barriers.txt   |   71 +---------------------------------
 b/arch/powerpc/include/asm/spinlock.h |    2 
 b/include/linux/spinlock.h            |   10 ----
 b/kernel/rcu/tree.h                   |   12 +++++
 4 files changed, 16 insertions(+), 79 deletions(-)

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>

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 318523872db5..eafa6a53f72c 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1854,16 +1854,10 @@ RELEASE are to the same lock variable, but only from the perspective of
 another CPU not holding that lock.  In short, a ACQUIRE followed by an
 RELEASE may -not- be assumed to be a full memory barrier.
 
-Similarly, the reverse case of a RELEASE followed by an ACQUIRE does not
-imply a full memory barrier.  If it is necessary for a RELEASE-ACQUIRE
-pair to produce a full barrier, the ACQUIRE can be followed by an
-smp_mb__after_unlock_lock() invocation.  This will produce a full barrier
-(including transitivity) if either (a) the RELEASE and the ACQUIRE are
-executed by the same CPU or task, or (b) the RELEASE and ACQUIRE act on
-the same variable.  The smp_mb__after_unlock_lock() primitive is free
-on many architectures.  Without smp_mb__after_unlock_lock(), the CPU's
-execution of the critical sections corresponding to the RELEASE and the
-ACQUIRE can cross, so that:
+Similarly, the reverse case of a RELEASE followed by an ACQUIRE does
+not imply a full memory barrier.  Therefore, the CPU's execution of the
+critical sections corresponding to the RELEASE and the ACQUIRE can cross,
+so that:
 
 	*A = a;
 	RELEASE M
@@ -1901,29 +1895,6 @@ the RELEASE would simply complete, thereby avoiding the deadlock.
 	a sleep-unlock race, but the locking primitive needs to resolve
 	such races properly in any case.
 
-With smp_mb__after_unlock_lock(), the two critical sections cannot overlap.
-For example, with the following code, the store to *A will always be
-seen by other CPUs before the store to *B:
-
-	*A = a;
-	RELEASE M
-	ACQUIRE N
-	smp_mb__after_unlock_lock();
-	*B = b;
-
-The operations will always occur in one of the following orders:
-
-	STORE *A, RELEASE, ACQUIRE, smp_mb__after_unlock_lock(), STORE *B
-	STORE *A, ACQUIRE, RELEASE, smp_mb__after_unlock_lock(), STORE *B
-	ACQUIRE, STORE *A, RELEASE, smp_mb__after_unlock_lock(), STORE *B
-
-If the RELEASE and ACQUIRE were instead both operating on the same lock
-variable, only the first of these alternatives can occur.  In addition,
-the more strongly ordered systems may rule out some of the above orders.
-But in any case, as noted earlier, the smp_mb__after_unlock_lock()
-ensures that the store to *A will always be seen as happening before
-the store to *B.
-
 Locks and semaphores may not provide any guarantee of ordering on UP compiled
 systems, and so cannot be counted on in such a situation to actually achieve
 anything at all - especially with respect to I/O accesses - unless combined
@@ -2154,40 +2125,6 @@ But it won't see any of:
 	*E, *F or *G following RELEASE Q
 
 
-However, if the following occurs:
-
-	CPU 1				CPU 2
-	===============================	===============================
-	WRITE_ONCE(*A, a);
-	ACQUIRE M		     [1]
-	WRITE_ONCE(*B, b);
-	WRITE_ONCE(*C, c);
-	RELEASE M	     [1]
-	WRITE_ONCE(*D, d);		WRITE_ONCE(*E, e);
-					ACQUIRE M		     [2]
-					smp_mb__after_unlock_lock();
-					WRITE_ONCE(*F, f);
-					WRITE_ONCE(*G, g);
-					RELEASE M	     [2]
-					WRITE_ONCE(*H, h);
-
-CPU 3 might see:
-
-	*E, ACQUIRE M [1], *C, *B, *A, RELEASE M [1],
-		ACQUIRE M [2], *H, *F, *G, RELEASE M [2], *D
-
-But assuming CPU 1 gets the lock first, CPU 3 won't see any of:
-
-	*B, *C, *D, *F, *G or *H preceding ACQUIRE M [1]
-	*A, *B or *C following RELEASE M [1]
-	*F, *G or *H preceding ACQUIRE M [2]
-	*A, *B, *C, *E, *F or *G following RELEASE M [2]
-
-Note that the smp_mb__after_unlock_lock() is critically important
-here: Without it CPU 3 might see some of the above orderings.
-Without smp_mb__after_unlock_lock(), the accesses are not guaranteed
-to be seen in order unless CPU 3 holds lock M.
-
 
 ACQUIRES VS I/O ACCESSES
 ------------------------
diff --git a/arch/powerpc/include/asm/spinlock.h b/arch/powerpc/include/asm/spinlock.h
index 4dbe072eecbe..523673d7583c 100644
--- a/arch/powerpc/include/asm/spinlock.h
+++ b/arch/powerpc/include/asm/spinlock.h
@@ -28,8 +28,6 @@
 #include <asm/synch.h>
 #include <asm/ppc-opcode.h>
 
-#define smp_mb__after_unlock_lock()	smp_mb()  /* Full ordering for lock. */
-
 #ifdef CONFIG_PPC64
 /* use 0x800000yy when locked, where yy == CPU number */
 #ifdef __BIG_ENDIAN__
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index 0063b24b4f36..16c5ed5a627c 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -130,16 +130,6 @@ do {								\
 #define smp_mb__before_spinlock()	smp_wmb()
 #endif
 
-/*
- * Place this after a lock-acquisition primitive to guarantee that
- * an UNLOCK+LOCK pair act as a full barrier.  This guarantee applies
- * if the UNLOCK and LOCK are executed by the same CPU or if the
- * UNLOCK and LOCK operate on the same lock variable.
- */
-#ifndef smp_mb__after_unlock_lock
-#define smp_mb__after_unlock_lock()	do { } while (0)
-#endif
-
 /**
  * raw_spin_unlock_wait - wait until the spinlock gets unlocked
  * @lock: the spinlock in question.
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 0412030ca882..2e991f8361e4 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -653,3 +653,15 @@ static inline void rcu_nocb_q_lengths(struct rcu_data *rdp, long *ql, long *qll)
 #endif /* #else #ifdef CONFIG_RCU_NOCB_CPU */
 }
 #endif /* #ifdef CONFIG_RCU_TRACE */
+
+/*
+ * Place this after a lock-acquisition primitive to guarantee that
+ * an UNLOCK+LOCK pair act as a full barrier.  This guarantee applies
+ * 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
+#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 */

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

[toc] | [prev] | [next] | [standalone]

#1206711

On Wed, Aug 12, 2015 at 06:59:38PM +0100, Paul E. McKenney wrote:
> On Wed, Aug 12, 2015 at 08:43:46AM -0700, Paul E. McKenney wrote:
> > On Wed, Aug 12, 2015 at 02:44:15PM +0100, Will Deacon wrote:
> > > The generic relaxed atomics are now queued in -tip, so it would be really
> > > good to see this Documentation update land in 4.3 if at all possible. I
> > > appreciate it's late in the cycle, but it's always worth asking.
> > 
> > Can't hurt to give it a try.  I have set -rcu's rcu/next branch to this
> > commit, and if it passes a few day's worth of testing, I will see what
> > Ingo has to say about a pull request.
> > 
> > This commit also privatizes smp_mb__after_unlock_lock() as well as
> > updating documentation.  Looks like we need to strengthen powerpc's
> > locking primitives, then get rid of smp_mb__after_unlock_lock() entirely.
> > Or did that already happen and I just missed it?
> 
> And just for completeness, here is the current version of that commit.
> 
> 							Thanx, Paul
> 
> ------------------------------------------------------------------------
> 
>  b/Documentation/memory-barriers.txt   |   71 +---------------------------------
>  b/arch/powerpc/include/asm/spinlock.h |    2 
>  b/include/linux/spinlock.h            |   10 ----
>  b/kernel/rcu/tree.h                   |   12 +++++
>  4 files changed, 16 insertions(+), 79 deletions(-)
> 
> 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>

Acked-by: Will Deacon <will.deacon@arm.com>

I don't think the PowerPC spinlock change is queued anywhere (I sent it
out as a diff for discussion, but that was it). This patch doesn't rely
on that though, right?

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

[toc] | [prev] | [next] | [standalone]

#1206808

On Thu, Aug 13, 2015 at 11:49:28AM +0100, Will Deacon wrote:
> On Wed, Aug 12, 2015 at 06:59:38PM +0100, Paul E. McKenney wrote:
> > On Wed, Aug 12, 2015 at 08:43:46AM -0700, Paul E. McKenney wrote:
> > > On Wed, Aug 12, 2015 at 02:44:15PM +0100, Will Deacon wrote:
> > > > The generic relaxed atomics are now queued in -tip, so it would be really
> > > > good to see this Documentation update land in 4.3 if at all possible. I
> > > > appreciate it's late in the cycle, but it's always worth asking.
> > > 
> > > Can't hurt to give it a try.  I have set -rcu's rcu/next branch to this
> > > commit, and if it passes a few day's worth of testing, I will see what
> > > Ingo has to say about a pull request.
> > > 
> > > This commit also privatizes smp_mb__after_unlock_lock() as well as
> > > updating documentation.  Looks like we need to strengthen powerpc's
> > > locking primitives, then get rid of smp_mb__after_unlock_lock() entirely.
> > > Or did that already happen and I just missed it?
> > 
> > And just for completeness, here is the current version of that commit.
> > 
> > 							Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > 
> >  b/Documentation/memory-barriers.txt   |   71 +---------------------------------
> >  b/arch/powerpc/include/asm/spinlock.h |    2 
> >  b/include/linux/spinlock.h            |   10 ----
> >  b/kernel/rcu/tree.h                   |   12 +++++
> >  4 files changed, 16 insertions(+), 79 deletions(-)
> > 
> > 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>
> 
> Acked-by: Will Deacon <will.deacon@arm.com>
> 
> I don't think the PowerPC spinlock change is queued anywhere (I sent it
> out as a diff for discussion, but that was it). This patch doesn't rely
> on that though, right?

No, this patch just moves the smp_mb__after_unlock_lock() definition,
it does not change the code generated.

							Thanx, Paul

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

[toc] | [prev] | [next] | [standalone]

#1208354

On Wed, 2015-08-12 at 08:43 -0700, Paul E. McKenney wrote:
> On Wed, Aug 12, 2015 at 02:44:15PM +0100, Will Deacon wrote:
> > Hello Paul,
> > 
> > On Fri, Jul 24, 2015 at 04:30:46PM +0100, Paul E. McKenney wrote:
> > > On Fri, Jul 24, 2015 at 12:31:01PM +0100, Will Deacon wrote:
> > > > On Wed, Jul 15, 2015 at 02:12:21PM +0100, Paul E. McKenney wrote:
> > > > > > > commit 695c05d4b9666c50b40a1c022678b5f6e2e3e771
> > > > > > > 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>
> > > > 
> > > > Are you planning to queue this somewhere? I think it makes sense regardless
> > > > of whether we change PowerPc or not and ideally it would be merged around
> > > > the same time as my relaxed atomics series.
> > > 
> > > I have is in -rcu.  By default, I will push it to the 4.4 merge window.
> > > Please let me know if you need it sooner.
> > 
> > The generic relaxed atomics are now queued in -tip, so it would be really
> > good to see this Documentation update land in 4.3 if at all possible. I
> > appreciate it's late in the cycle, but it's always worth asking.
> 
> Can't hurt to give it a try.  I have set -rcu's rcu/next branch to this
> commit, and if it passes a few day's worth of testing, I will see what
> Ingo has to say about a pull request.
> 
> This commit also privatizes smp_mb__after_unlock_lock() as well as
> updating documentation.  Looks like we need to strengthen powerpc's
> locking primitives, then get rid of smp_mb__after_unlock_lock() entirely.
> Or did that already happen and I just missed it?

No it didn't.

I thought the end result of this thread was that we didn't *need* to change the
powerpc lock semantics? Or did I read it wrong?

ie. the docs now say that RELEASE+ACQUIRE is not a full barrier, which is
consistent with our current implementation.

cheers


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

[toc] | [prev] | [next] | [standalone]

#1208384

On Mon, Aug 17, 2015 at 02:06:07PM +1000, Michael Ellerman wrote:
> On Wed, 2015-08-12 at 08:43 -0700, Paul E. McKenney wrote:
> > On Wed, Aug 12, 2015 at 02:44:15PM +0100, Will Deacon wrote:
> > > Hello Paul,
> > > 
> > > On Fri, Jul 24, 2015 at 04:30:46PM +0100, Paul E. McKenney wrote:
> > > > On Fri, Jul 24, 2015 at 12:31:01PM +0100, Will Deacon wrote:
> > > > > On Wed, Jul 15, 2015 at 02:12:21PM +0100, Paul E. McKenney wrote:
> > > > > > > > commit 695c05d4b9666c50b40a1c022678b5f6e2e3e771
> > > > > > > > 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>
> > > > > 
> > > > > Are you planning to queue this somewhere? I think it makes sense regardless
> > > > > of whether we change PowerPc or not and ideally it would be merged around
> > > > > the same time as my relaxed atomics series.
> > > > 
> > > > I have is in -rcu.  By default, I will push it to the 4.4 merge window.
> > > > Please let me know if you need it sooner.
> > > 
> > > The generic relaxed atomics are now queued in -tip, so it would be really
> > > good to see this Documentation update land in 4.3 if at all possible. I
> > > appreciate it's late in the cycle, but it's always worth asking.
> > 
> > Can't hurt to give it a try.  I have set -rcu's rcu/next branch to this
> > commit, and if it passes a few day's worth of testing, I will see what
> > Ingo has to say about a pull request.
> > 
> > This commit also privatizes smp_mb__after_unlock_lock() as well as
> > updating documentation.  Looks like we need to strengthen powerpc's
> > locking primitives, then get rid of smp_mb__after_unlock_lock() entirely.
> > Or did that already happen and I just missed it?
> 
> No it didn't.
> 
> I thought the end result of this thread was that we didn't *need* to change the
> powerpc lock semantics? Or did I read it wrong?
> 
> ie. the docs now say that RELEASE+ACQUIRE is not a full barrier, which is
> consistent with our current implementation.

That change happened about 1.5 years ago, and I thought that the
current discussion was about reversing it, based in part on the
recent powerpc benchmarks of locking primitives with and without the
sync instruction.  But regardless, I clearly cannot remove either the
smp_mb__after_unlock_lock() or the powerpc definition of it to be smp_mb()
if powerpc unlock/lock is not strengthened.

							Thanx, Paul

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

[toc] | [prev] | [next] | [standalone]

#1208480

On Mon, Aug 17, 2015 at 07:15:01AM +0100, Paul E. McKenney wrote:
> On Mon, Aug 17, 2015 at 02:06:07PM +1000, Michael Ellerman wrote:
> > On Wed, 2015-08-12 at 08:43 -0700, Paul E. McKenney wrote:
> > > On Wed, Aug 12, 2015 at 02:44:15PM +0100, Will Deacon wrote:
> > > > On Fri, Jul 24, 2015 at 04:30:46PM +0100, Paul E. McKenney wrote:
> > > > > On Fri, Jul 24, 2015 at 12:31:01PM +0100, Will Deacon wrote:
> > > > > > On Wed, Jul 15, 2015 at 02:12:21PM +0100, Paul E. McKenney wrote:
> > > > > > > > > commit 695c05d4b9666c50b40a1c022678b5f6e2e3e771
> > > > > > > > > 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>
> > > > > > 
> > > > > > Are you planning to queue this somewhere? I think it makes sense regardless
> > > > > > of whether we change PowerPc or not and ideally it would be merged around
> > > > > > the same time as my relaxed atomics series.
> > > > > 
> > > > > I have is in -rcu.  By default, I will push it to the 4.4 merge window.
> > > > > Please let me know if you need it sooner.
> > > > 
> > > > The generic relaxed atomics are now queued in -tip, so it would be really
> > > > good to see this Documentation update land in 4.3 if at all possible. I
> > > > appreciate it's late in the cycle, but it's always worth asking.
> > > 
> > > Can't hurt to give it a try.  I have set -rcu's rcu/next branch to this
> > > commit, and if it passes a few day's worth of testing, I will see what
> > > Ingo has to say about a pull request.
> > > 
> > > This commit also privatizes smp_mb__after_unlock_lock() as well as
> > > updating documentation.  Looks like we need to strengthen powerpc's
> > > locking primitives, then get rid of smp_mb__after_unlock_lock() entirely.
> > > Or did that already happen and I just missed it?
> > 
> > No it didn't.
> > 
> > I thought the end result of this thread was that we didn't *need* to change the
> > powerpc lock semantics? Or did I read it wrong?
> > 
> > ie. the docs now say that RELEASE+ACQUIRE is not a full barrier, which is
> > consistent with our current implementation.
> 
> That change happened about 1.5 years ago, and I thought that the
> current discussion was about reversing it, based in part on the
> recent powerpc benchmarks of locking primitives with and without the
> sync instruction.  But regardless, I clearly cannot remove either the
> smp_mb__after_unlock_lock() or the powerpc definition of it to be smp_mb()
> if powerpc unlock/lock is not strengthened.

Yup. Peter and I would really like to get rid of smp_mb__after_unlock_lock
entirely, which would mean strengthening the ppc spinlocks. Moving the
barrier primitive into RCU is a good step to prevent more widespread usage
of the barrier, but we'd really like to go further if the performance impact
is deemed acceptable (which is what this thread is about).

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

[toc] | [prev] | [next] | [standalone]

#1209000

On Mon, 2015-08-17 at 09:57 +0100, Will Deacon wrote:
> On Mon, Aug 17, 2015 at 07:15:01AM +0100, Paul E. McKenney wrote:
> > On Mon, Aug 17, 2015 at 02:06:07PM +1000, Michael Ellerman wrote:
> > > On Wed, 2015-08-12 at 08:43 -0700, Paul E. McKenney wrote:
> > > I thought the end result of this thread was that we didn't *need* to change the
> > > powerpc lock semantics? Or did I read it wrong?
> > > 
> > > ie. the docs now say that RELEASE+ACQUIRE is not a full barrier, which is
> > > consistent with our current implementation.
> > 
> > That change happened about 1.5 years ago, and I thought that the
> > current discussion was about reversing it, based in part on the
> > recent powerpc benchmarks of locking primitives with and without the
> > sync instruction.  But regardless, I clearly cannot remove either the
> > smp_mb__after_unlock_lock() or the powerpc definition of it to be smp_mb()
> > if powerpc unlock/lock is not strengthened.
> 
> Yup. Peter and I would really like to get rid of smp_mb__after_unlock_lock
> entirely, which would mean strengthening the ppc spinlocks. Moving the
> barrier primitive into RCU is a good step to prevent more widespread usage
> of the barrier, but we'd really like to go further if the performance impact
> is deemed acceptable (which is what this thread is about).

OK, sorry for completely missing the point, too many balls in the air here.

I'll do some benchmarks and see what we come up with.

cheers


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

[toc] | [prev] | [next] | [standalone]

#1209125

On Tue, Aug 18, 2015 at 02:50:55AM +0100, Michael Ellerman wrote:
> On Mon, 2015-08-17 at 09:57 +0100, Will Deacon wrote:
> > On Mon, Aug 17, 2015 at 07:15:01AM +0100, Paul E. McKenney wrote:
> > > On Mon, Aug 17, 2015 at 02:06:07PM +1000, Michael Ellerman wrote:
> > > > On Wed, 2015-08-12 at 08:43 -0700, Paul E. McKenney wrote:
> > > > I thought the end result of this thread was that we didn't *need* to change the
> > > > powerpc lock semantics? Or did I read it wrong?
> > > > 
> > > > ie. the docs now say that RELEASE+ACQUIRE is not a full barrier, which is
> > > > consistent with our current implementation.
> > > 
> > > That change happened about 1.5 years ago, and I thought that the
> > > current discussion was about reversing it, based in part on the
> > > recent powerpc benchmarks of locking primitives with and without the
> > > sync instruction.  But regardless, I clearly cannot remove either the
> > > smp_mb__after_unlock_lock() or the powerpc definition of it to be smp_mb()
> > > if powerpc unlock/lock is not strengthened.
> > 
> > Yup. Peter and I would really like to get rid of smp_mb__after_unlock_lock
> > entirely, which would mean strengthening the ppc spinlocks. Moving the
> > barrier primitive into RCU is a good step to prevent more widespread usage
> > of the barrier, but we'd really like to go further if the performance impact
> > is deemed acceptable (which is what this thread is about).
> 
> OK, sorry for completely missing the point, too many balls in the air here.

No problem!

> I'll do some benchmarks and see what we come up with.

Thanks, that sounds great. FWIW, there are multiple ways of implementing
the patch (i.e. whether you strengthen lock or unlock). I had a crack at
something here, but it's not tested:

  http://marc.info/?l=linux-arch&m=143758379023849&w=2

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web