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

Groups > linux.kernel > #1678300 > unrolled thread

[PATCH RFC 0/26] Remove spin_unlock_wait()

Back to article view | Back to linux.kernel

Contents

  [PATCH RFC 0/26] Remove spin_unlock_wait() "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:00 +0200
    [PATCH RFC 13/26] blackfin: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 08/26] locking: Remove spin_unlock_wait() generic definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
      Re: [PATCH RFC 08/26] locking: Remove spin_unlock_wait() generic  definitions Will Deacon <will.deacon@arm.com> - 2017-06-30 11:20 +0200
        Re: [PATCH RFC 08/26] locking: Remove spin_unlock_wait() generic  definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 14:50 +0200
          Re: [PATCH RFC 08/26] locking: Remove spin_unlock_wait() generic  definitions Will Deacon <will.deacon@arm.com> - 2017-06-30 15:20 +0200
            Re: [PATCH RFC 08/26] locking: Remove spin_unlock_wait() generic  definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-01 00:20 +0200
              Re: [PATCH RFC 08/26] locking: Remove spin_unlock_wait() generic  definitions Will Deacon <will.deacon@arm.com> - 2017-07-03 15:20 +0200
    [PATCH RFC 23/26] sh: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 16/26] m32r: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 21/26] powerpc: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
      Re: [PATCH RFC 21/26] powerpc: Remove spin_unlock_wait()  arch-specific definitions Boqun Feng <boqun.feng@gmail.com> - 2017-07-02 06:00 +0200
    [PATCH RFC 25/26] tile: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
      Re: [PATCH RFC 25/26] tile: Remove spin_unlock_wait() arch-specific definitions Linus Torvalds <torvalds@linux-foundation.org> - 2017-06-30 02:10 +0200
        Re: [PATCH RFC 25/26] tile: Remove spin_unlock_wait() arch-specific definitions Linus Torvalds <torvalds@linux-foundation.org> - 2017-06-30 02:20 +0200
          Re: [PATCH RFC 25/26] tile: Remove spin_unlock_wait() arch-specific  definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:30 +0200
        Re: [PATCH RFC 25/26] tile: Remove spin_unlock_wait() arch-specific  definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:20 +0200
        Re: [PATCH RFC 25/26] tile: Remove spin_unlock_wait() arch-specific  definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:20 +0200
    [PATCH RFC 18/26] mips: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 19/26] mn10300: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 20/26] parisc: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 15/26] ia64: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 11/26] arm: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 22/26] s390: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 01/26] netfilter: Replace spin_unlock_wait() with lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
      Re: [PATCH RFC 01/26] netfilter: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-02 04:10 +0200
    [PATCH RFC 04/26] completion: Replace spin_unlock_wait() with lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 07/26] drivers/ata: Replace spin_unlock_wait() with lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 14/26] hexagon: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 09/26] alpha: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 24/26] sparc: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 26/26] xtensa: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:10 +0200
    [PATCH RFC 03/26] sched: Replace spin_unlock_wait() with lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:20 +0200
      Re: [PATCH RFC 03/26] sched: Replace spin_unlock_wait() with  lock/unlock pair Arnd Bergmann <arnd@arndb.de> - 2017-06-30 12:40 +0200
        Re: [PATCH RFC 03/26] sched: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 14:40 +0200
    [PATCH RFC 05/26] exit: Replace spin_unlock_wait() with lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:20 +0200
    [PATCH RFC 12/26] arm64: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:20 +0200
      Re: [PATCH RFC 12/26] arm64: Remove spin_unlock_wait() arch-specific  definitions Will Deacon <will.deacon@arm.com> - 2017-06-30 11:30 +0200
        Re: [PATCH RFC 12/26] arm64: Remove spin_unlock_wait() arch-specific  definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 19:40 +0200
    [PATCH RFC 10/26] arc: Remove spin_unlock_wait() arch-specific definitions "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:20 +0200
    [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 02:20 +0200
      Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair Oleg Nesterov <oleg@redhat.com> - 2017-06-30 13:10 +0200
        Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 15:00 +0200
          Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair Oleg Nesterov <oleg@redhat.com> - 2017-06-30 17:30 +0200
            Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 18:20 +0200
              Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 19:30 +0200
              Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair Oleg Nesterov <oleg@redhat.com> - 2017-06-30 21:30 +0200
                Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair Alan Stern <stern@rowland.harvard.edu> - 2017-06-30 22:00 +0200
                  Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 22:10 +0200
                Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 22:10 +0200
                  Re: [PATCH RFC 02/26] task_work: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-06-30 22:20 +0200
    Re: [PATCH RFC 06/26] ipc: Replace spin_unlock_wait() with  lock/unlock pair Manfred Spraul <manfred@colorfullife.com> - 2017-07-01 21:30 +0200
      Re: [PATCH RFC 06/26] ipc: Replace spin_unlock_wait() with  lock/unlock pair "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-07-02 05:20 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1678312 — [PATCH RFC 20/26] parisc: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "James E.J. Bottomley" <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: <linux-parisc@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/parisc/include/asm/spinlock.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/parisc/include/asm/spinlock.h b/arch/parisc/include/asm/spinlock.h
index e32936cd7f10..55bfe4affca3 100644
--- a/arch/parisc/include/asm/spinlock.h
+++ b/arch/parisc/include/asm/spinlock.h
@@ -14,13 +14,6 @@ static inline int arch_spin_is_locked(arch_spinlock_t *x)
 
 #define arch_spin_lock(lock) arch_spin_lock_flags(lock, 0)
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *x)
-{
-	volatile unsigned int *a = __ldcw_align(x);
-
-	smp_cond_load_acquire(a, VAL);
-}
-
 static inline void arch_spin_lock_flags(arch_spinlock_t *x,
 					 unsigned long flags)
 {
-- 
2.5.2

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

#1678313 — [PATCH RFC 15/26] ia64: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: <linux-ia64@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/ia64/include/asm/spinlock.h | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/arch/ia64/include/asm/spinlock.h b/arch/ia64/include/asm/spinlock.h
index ca9e76149a4a..df2c121164b8 100644
--- a/arch/ia64/include/asm/spinlock.h
+++ b/arch/ia64/include/asm/spinlock.h
@@ -76,22 +76,6 @@ static __always_inline void __ticket_spin_unlock(arch_spinlock_t *lock)
 	ACCESS_ONCE(*p) = (tmp + 2) & ~1;
 }
 
-static __always_inline void __ticket_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	int	*p = (int *)&lock->lock, ticket;
-
-	ia64_invala();
-
-	for (;;) {
-		asm volatile ("ld4.c.nc %0=[%1]" : "=r"(ticket) : "r"(p) : "memory");
-		if (!(((ticket >> TICKET_SHIFT) ^ ticket) & TICKET_MASK))
-			return;
-		cpu_relax();
-	}
-
-	smp_acquire__after_ctrl_dep();
-}
-
 static inline int __ticket_spin_is_locked(arch_spinlock_t *lock)
 {
 	long tmp = ACCESS_ONCE(lock->lock);
@@ -143,11 +127,6 @@ static __always_inline void arch_spin_lock_flags(arch_spinlock_t *lock,
 	arch_spin_lock(lock);
 }
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	__ticket_spin_unlock_wait(lock);
-}
-
 #define arch_read_can_lock(rw)		(*(volatile int *)(rw) >= 0)
 #define arch_write_can_lock(rw)	(*(volatile int *)(rw) == 0)
 
-- 
2.5.2

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

#1678314 — [PATCH RFC 11/26] arm: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/arm/include/asm/spinlock.h | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/arch/arm/include/asm/spinlock.h b/arch/arm/include/asm/spinlock.h
index 4bec45442072..c030143c18c6 100644
--- a/arch/arm/include/asm/spinlock.h
+++ b/arch/arm/include/asm/spinlock.h
@@ -52,22 +52,6 @@ static inline void dsb_sev(void)
  * memory.
  */
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	u16 owner = READ_ONCE(lock->tickets.owner);
-
-	for (;;) {
-		arch_spinlock_t tmp = READ_ONCE(*lock);
-
-		if (tmp.tickets.owner == tmp.tickets.next ||
-		    tmp.tickets.owner != owner)
-			break;
-
-		wfe();
-	}
-	smp_acquire__after_ctrl_dep();
-}
-
 #define arch_spin_lock_flags(lock, flags) arch_spin_lock(lock)
 
 static inline void arch_spin_lock(arch_spinlock_t *lock)
-- 
2.5.2

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

#1678315 — [PATCH RFC 22/26] s390: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: <linux-s390@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/s390/include/asm/spinlock.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/s390/include/asm/spinlock.h b/arch/s390/include/asm/spinlock.h
index f7838ecd83c6..217ee5210c32 100644
--- a/arch/s390/include/asm/spinlock.h
+++ b/arch/s390/include/asm/spinlock.h
@@ -98,13 +98,6 @@ static inline void arch_spin_unlock(arch_spinlock_t *lp)
 		: "cc", "memory");
 }
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	while (arch_spin_is_locked(lock))
-		arch_spin_relax(lock);
-	smp_acquire__after_ctrl_dep();
-}
-
 /*
  * Read-write spinlocks, allowing multiple readers
  * but only one writer.
-- 
2.5.2

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

#1678318 — [PATCH RFC 01/26] netfilter: Replace spin_unlock_wait() with lock/unlock pair

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore replaces the spin_unlock_wait() calls
in nf_conntrack_lock() and nf_conntrack_all_lock() with spin_lock()
followed immediately by spin_unlock().  These functions do not appear
to be invoked on any fastpaths.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: Florian Westphal <fw@strlen.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <netfilter-devel@vger.kernel.org>
Cc: <coreteam@netfilter.org>
Cc: <netdev@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 net/netfilter/nf_conntrack_core.c | 26 ++++++++------------------
 1 file changed, 8 insertions(+), 18 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index e847dbaa0c6b..9f997859d160 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -99,15 +99,11 @@ void nf_conntrack_lock(spinlock_t *lock) __acquires(lock)
 	spin_lock(lock);
 	while (unlikely(nf_conntrack_locks_all)) {
 		spin_unlock(lock);
-
-		/*
-		 * Order the 'nf_conntrack_locks_all' load vs. the
-		 * spin_unlock_wait() loads below, to ensure
-		 * that 'nf_conntrack_locks_all_lock' is indeed held:
-		 */
-		smp_rmb(); /* spin_lock(&nf_conntrack_locks_all_lock) */
-		spin_unlock_wait(&nf_conntrack_locks_all_lock);
+		/* Wait for nf_conntrack_locks_all_lock holder to release ... */
+		spin_lock(&nf_conntrack_locks_all_lock);
+		spin_unlock(&nf_conntrack_locks_all_lock);
 		spin_lock(lock);
+		/* ... and retry. */
 	}
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_lock);
@@ -150,17 +146,11 @@ static void nf_conntrack_all_lock(void)
 
 	spin_lock(&nf_conntrack_locks_all_lock);
 	nf_conntrack_locks_all = true;
-
-	/*
-	 * Order the above store of 'nf_conntrack_locks_all' against
-	 * the spin_unlock_wait() loads below, such that if
-	 * nf_conntrack_lock() observes 'nf_conntrack_locks_all'
-	 * we must observe nf_conntrack_locks[] held:
-	 */
-	smp_mb(); /* spin_lock(&nf_conntrack_locks_all_lock) */
-
 	for (i = 0; i < CONNTRACK_LOCKS; i++) {
-		spin_unlock_wait(&nf_conntrack_locks[i]);
+		/* Wait for any current holder to release lock. */
+		spin_lock(&nf_conntrack_locks[i]);
+		spin_unlock(&nf_conntrack_locks[i]);
+		/* Next acquisition will see nf_conntrack_locks_all == true. */
 	}
 }
 
-- 
2.5.2

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

#1679420 — Re: [PATCH RFC 01/26] netfilter: Replace spin_unlock_wait() with lock/unlock pair

On Sat, Jul 01, 2017 at 09:44:12PM +0200, Manfred Spraul wrote:
> Hi Paul,
> 
> On 06/30/2017 02:01 AM, Paul E. McKenney wrote:
> >There is no agreed-upon definition of spin_unlock_wait()'s semantics,
> >and it appears that all callers could do just as well with a lock/unlock
> >pair.  This commit therefore replaces the spin_unlock_wait() calls
> >in nf_conntrack_lock() and nf_conntrack_all_lock() with spin_lock()
> >followed immediately by spin_unlock().  These functions do not appear
> >to be invoked on any fastpaths.
> >
> >Signed-off-by: Paul E. McKenney<paulmck@linux.vnet.ibm.com>
> >Cc: Pablo Neira Ayuso<pablo@netfilter.org>
> >Cc: Jozsef Kadlecsik<kadlec@blackhole.kfki.hu>
> >Cc: Florian Westphal<fw@strlen.de>
> >Cc: "David S. Miller"<davem@davemloft.net>
> >Cc:<netfilter-devel@vger.kernel.org>
> >Cc:<coreteam@netfilter.org>
> >Cc:<netdev@vger.kernel.org>
> >Cc: Will Deacon<will.deacon@arm.com>
> >Cc: Peter Zijlstra<peterz@infradead.org>
> >Cc: Alan Stern<stern@rowland.harvard.edu>
> >Cc: Andrea Parri<parri.andrea@gmail.com>
> >Cc: Linus Torvalds<torvalds@linux-foundation.org>
> >---
> >  net/netfilter/nf_conntrack_core.c | 26 ++++++++------------------
> >  1 file changed, 8 insertions(+), 18 deletions(-)
> >
> >diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> >index e847dbaa0c6b..9f997859d160 100644
> >--- a/net/netfilter/nf_conntrack_core.c
> >+++ b/net/netfilter/nf_conntrack_core.c
> >@@ -99,15 +99,11 @@ void nf_conntrack_lock(spinlock_t *lock) __acquires(lock)
> >  	spin_lock(lock);
> >  	while (unlikely(nf_conntrack_locks_all)) {
> I think here an ACQUIRE is missing.
> >  		spin_unlock(lock);
> >-
> >-		/*
> >-		 * Order the 'nf_conntrack_locks_all' load vs. the
> >-		 * spin_unlock_wait() loads below, to ensure
> >-		 * that 'nf_conntrack_locks_all_lock' is indeed held:
> >-		 */
> >-		smp_rmb(); /* spin_lock(&nf_conntrack_locks_all_lock) */
> >-		spin_unlock_wait(&nf_conntrack_locks_all_lock);
> >+		/* Wait for nf_conntrack_locks_all_lock holder to release ... */
> >+		spin_lock(&nf_conntrack_locks_all_lock);
> >+		spin_unlock(&nf_conntrack_locks_all_lock);
> >  		spin_lock(lock);
> >+		/* ... and retry. */
> >  	}
> >  }
> As far as I see, nf_conntrack_locks[] nests inside
> nf_conntrack_lock_all_lock.
> So
>    spin_lock(&nf_conntrack_locks_all_lock);
>    spin_lock(lock);
>    spin_unlock(&nf_conntrack_locks_all_lock);
> 
> can replace the retry logic.
> 
> Correct? Then what about the attached patch?

At first glance, it looks correct to me, thank you!  I have replaced my
patch with this one for testing and further review.

							Thanx, Paul

> --
>     Manfred
> 
> 

> >From 453e7a77f3756d939c754031b092cbdfbd149559 Mon Sep 17 00:00:00 2001
> From: Manfred Spraul <manfred@colorfullife.com>
> Date: Sun, 21 Aug 2016 07:17:55 +0200
> Subject: [PATCH] net/netfilter/nf_conntrack_core: Fix net_conntrack_lock()
> 
> As we want to remove spin_unlock_wait() and replace it with explicit
> spin_lock()/spin_unlock() calls, we can use this to simplify the
> locking.
> 
> In addition:
> - Reading nf_conntrack_locks_all needs ACQUIRE memory ordering.
> - The new code avoids the backwards loop.
> 
> Only slightly tested, I did not manage to trigger calls to
> nf_conntrack_all_lock().
> 
> Fixes: b16c29191dc8
> Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
> Cc: <stable@vger.kernel.org>
> Cc: Sasha Levin <sasha.levin@oracle.com>
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: netfilter-devel@vger.kernel.org
> ---
>  net/netfilter/nf_conntrack_core.c | 44 +++++++++++++++++++++------------------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index e847dba..1193565 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -96,19 +96,24 @@ static struct conntrack_gc_work conntrack_gc_work;
> 
>  void nf_conntrack_lock(spinlock_t *lock) __acquires(lock)
>  {
> +	/* 1) Acquire the lock */
>  	spin_lock(lock);
> -	while (unlikely(nf_conntrack_locks_all)) {
> -		spin_unlock(lock);
> 
> -		/*
> -		 * Order the 'nf_conntrack_locks_all' load vs. the
> -		 * spin_unlock_wait() loads below, to ensure
> -		 * that 'nf_conntrack_locks_all_lock' is indeed held:
> -		 */
> -		smp_rmb(); /* spin_lock(&nf_conntrack_locks_all_lock) */
> -		spin_unlock_wait(&nf_conntrack_locks_all_lock);
> -		spin_lock(lock);
> -	}
> +	/* 2) read nf_conntrack_locks_all, with ACQUIRE semantics */
> +	if (likely(smp_load_acquire(&nf_conntrack_locks_all) == false))
> +		return;
> +
> +	/* fast path failed, unlock */
> +	spin_unlock(lock);
> +
> +	/* Slow path 1) get global lock */
> +	spin_lock(&nf_conntrack_locks_all_lock);
> +
> +	/* Slow path 2) get the lock we want */
> +	spin_lock(lock);
> +
> +	/* Slow path 3) release the global lock */
> +	spin_unlock(&nf_conntrack_locks_all_lock);
>  }
>  EXPORT_SYMBOL_GPL(nf_conntrack_lock);
> 
> @@ -149,18 +154,17 @@ static void nf_conntrack_all_lock(void)
>  	int i;
> 
>  	spin_lock(&nf_conntrack_locks_all_lock);
> -	nf_conntrack_locks_all = true;
> 
> -	/*
> -	 * Order the above store of 'nf_conntrack_locks_all' against
> -	 * the spin_unlock_wait() loads below, such that if
> -	 * nf_conntrack_lock() observes 'nf_conntrack_locks_all'
> -	 * we must observe nf_conntrack_locks[] held:
> -	 */
> -	smp_mb(); /* spin_lock(&nf_conntrack_locks_all_lock) */
> +	nf_conntrack_locks_all = true;
> 
>  	for (i = 0; i < CONNTRACK_LOCKS; i++) {
> -		spin_unlock_wait(&nf_conntrack_locks[i]);
> +		spin_lock(&nf_conntrack_locks[i]);
> +
> +		/* This spin_unlock provides the "release" to ensure that
> +		 * nf_conntrack_locks_all==true is visible to everyone that
> +		 * acquired spin_lock(&nf_conntrack_locks[]).
> +		 */
> +		spin_unlock(&nf_conntrack_locks[i]);
>  	}
>  }
> 
> -- 
> 2.9.4
> 

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

#1678319 — [PATCH RFC 04/26] completion: Replace spin_unlock_wait() with lock/unlock pair

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore replaces the spin_unlock_wait() call in
completion_done() with spin_lock() followed immediately by spin_unlock().
This should be safe from a performance perspective because the lock
will be held only the wakeup happens really quickly.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 kernel/sched/completion.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/completion.c b/kernel/sched/completion.c
index 53f9558fa925..d4b89a59629f 100644
--- a/kernel/sched/completion.c
+++ b/kernel/sched/completion.c
@@ -307,14 +307,9 @@ bool completion_done(struct completion *x)
 	 * If ->done, we need to wait for complete() to release ->wait.lock
 	 * otherwise we can end up freeing the completion before complete()
 	 * is done referencing it.
-	 *
-	 * The RMB pairs with complete()'s RELEASE of ->wait.lock and orders
-	 * the loads of ->done and ->wait.lock such that we cannot observe
-	 * the lock before complete() acquires it while observing the ->done
-	 * after it's acquired the lock.
 	 */
-	smp_rmb();
-	spin_unlock_wait(&x->wait.lock);
+	spin_lock(&x->wait.lock);
+	spin_unlock(&x->wait.lock);
 	return true;
 }
 EXPORT_SYMBOL(completion_done);
-- 
2.5.2

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

#1678320 — [PATCH RFC 07/26] drivers/ata: Replace spin_unlock_wait() with lock/unlock pair

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore eliminates the spin_unlock_wait() call and
associated else-clause and hoists the then-clause's lock and unlock out of
the "if" statement.  This should be safe from a performance perspective
because according to Tejun there should be few if any drivers that don't
set their own error handler.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: Tejun Heo <tj@kernel.org>
Cc: <linux-ide@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 drivers/ata/libata-eh.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index ef68232b5222..779f6f18c1f4 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -645,12 +645,11 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
 	 * completions are honored.  A scmd is determined to have
 	 * timed out iff its associated qc is active and not failed.
 	 */
+	spin_lock_irqsave(ap->lock, flags);
 	if (ap->ops->error_handler) {
 		struct scsi_cmnd *scmd, *tmp;
 		int nr_timedout = 0;
 
-		spin_lock_irqsave(ap->lock, flags);
-
 		/* This must occur under the ap->lock as we don't want
 		   a polled recovery to race the real interrupt handler
 
@@ -700,12 +699,11 @@ void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap,
 		if (nr_timedout)
 			__ata_port_freeze(ap);
 
-		spin_unlock_irqrestore(ap->lock, flags);
 
 		/* initialize eh_tries */
 		ap->eh_tries = ATA_EH_MAX_TRIES;
-	} else
-		spin_unlock_wait(ap->lock);
+	}
+	spin_unlock_irqrestore(ap->lock, flags);
 
 }
 EXPORT_SYMBOL(ata_scsi_cmd_error_handler);
-- 
2.5.2

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

#1678322 — [PATCH RFC 14/26] hexagon: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Richard Kuo <rkuo@codeaurora.org>
Cc: <linux-hexagon@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/hexagon/include/asm/spinlock.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/hexagon/include/asm/spinlock.h b/arch/hexagon/include/asm/spinlock.h
index a1c55788c5d6..53a8d5885887 100644
--- a/arch/hexagon/include/asm/spinlock.h
+++ b/arch/hexagon/include/asm/spinlock.h
@@ -179,11 +179,6 @@ static inline unsigned int arch_spin_trylock(arch_spinlock_t *lock)
  */
 #define arch_spin_lock_flags(lock, flags) arch_spin_lock(lock)
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	smp_cond_load_acquire(&lock->lock, !VAL);
-}
-
 #define arch_spin_is_locked(x) ((x)->lock != 0)
 
 #define arch_read_lock_flags(lock, flags) arch_read_lock(lock)
-- 
2.5.2

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

#1678323 — [PATCH RFC 09/26] alpha: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: <linux-alpha@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/alpha/include/asm/spinlock.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/alpha/include/asm/spinlock.h b/arch/alpha/include/asm/spinlock.h
index a40b9fc0c6c3..718ac0b64adf 100644
--- a/arch/alpha/include/asm/spinlock.h
+++ b/arch/alpha/include/asm/spinlock.h
@@ -16,11 +16,6 @@
 #define arch_spin_lock_flags(lock, flags) arch_spin_lock(lock)
 #define arch_spin_is_locked(x)	((x)->lock != 0)
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	smp_cond_load_acquire(&lock->lock, !VAL);
-}
-
 static inline int arch_spin_value_unlocked(arch_spinlock_t lock)
 {
         return lock.lock == 0;
-- 
2.5.2

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

#1678324 — [PATCH RFC 24/26] sparc: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <sparclinux@vger.kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/sparc/include/asm/spinlock_32.h | 5 -----
 arch/sparc/include/asm/spinlock_64.h | 5 -----
 2 files changed, 10 deletions(-)

diff --git a/arch/sparc/include/asm/spinlock_32.h b/arch/sparc/include/asm/spinlock_32.h
index 8011e79f59c9..67345b2dc408 100644
--- a/arch/sparc/include/asm/spinlock_32.h
+++ b/arch/sparc/include/asm/spinlock_32.h
@@ -14,11 +14,6 @@
 
 #define arch_spin_is_locked(lock) (*((volatile unsigned char *)(lock)) != 0)
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	smp_cond_load_acquire(&lock->lock, !VAL);
-}
-
 static inline void arch_spin_lock(arch_spinlock_t *lock)
 {
 	__asm__ __volatile__(
diff --git a/arch/sparc/include/asm/spinlock_64.h b/arch/sparc/include/asm/spinlock_64.h
index 07c9f2e9bf57..923d57f9b79d 100644
--- a/arch/sparc/include/asm/spinlock_64.h
+++ b/arch/sparc/include/asm/spinlock_64.h
@@ -26,11 +26,6 @@
 
 #define arch_spin_is_locked(lp)	((lp)->lock != 0)
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	smp_cond_load_acquire(&lock->lock, !VAL);
-}
-
 static inline void arch_spin_lock(arch_spinlock_t *lock)
 {
 	unsigned long tmp;
-- 
2.5.2

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

#1678325 — [PATCH RFC 26/26] xtensa: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Chris Zankel <chris@zankel.net>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Cc: <linux-xtensa@linux-xtensa.org>
---
 arch/xtensa/include/asm/spinlock.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/xtensa/include/asm/spinlock.h b/arch/xtensa/include/asm/spinlock.h
index a36221cf6363..3bb49681ee24 100644
--- a/arch/xtensa/include/asm/spinlock.h
+++ b/arch/xtensa/include/asm/spinlock.h
@@ -33,11 +33,6 @@
 
 #define arch_spin_is_locked(x) ((x)->slock != 0)
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	smp_cond_load_acquire(&lock->slock, !VAL);
-}
-
 #define arch_spin_lock_flags(lock, flags) arch_spin_lock(lock)
 
 static inline void arch_spin_lock(arch_spinlock_t *lock)
-- 
2.5.2

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

#1678331 — [PATCH RFC 03/26] sched: Replace spin_unlock_wait() with lock/unlock pair

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore replaces the spin_unlock_wait() call in
do_task_dead() with spin_lock() followed immediately by spin_unlock().
This should be safe from a performance perspective because the lock is
this tasks ->pi_lock, and this is called only after the task exits.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 kernel/sched/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index e91138fcde86..6dea3d9728c8 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3461,7 +3461,8 @@ void __noreturn do_task_dead(void)
 	 * is held by try_to_wake_up()
 	 */
 	smp_mb();
-	raw_spin_unlock_wait(&current->pi_lock);
+	raw_spin_lock(&current->pi_lock);
+	raw_spin_unlock(&current->pi_lock);
 
 	/* Causes final put_task_struct in finish_task_switch(): */
 	__set_current_state(TASK_DEAD);
-- 
2.5.2

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

#1678685 — Re: [PATCH RFC 03/26] sched: Replace spin_unlock_wait() with lock/unlock pair

On Fri, Jun 30, 2017 at 2:01 AM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> There is no agreed-upon definition of spin_unlock_wait()'s semantics,
> and it appears that all callers could do just as well with a lock/unlock
> pair.  This commit therefore replaces the spin_unlock_wait() call in
> do_task_dead() with spin_lock() followed immediately by spin_unlock().
> This should be safe from a performance perspective because the lock is
> this tasks ->pi_lock, and this is called only after the task exits.
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index e91138fcde86..6dea3d9728c8 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3461,7 +3461,8 @@ void __noreturn do_task_dead(void)
>          * is held by try_to_wake_up()
>          */
>         smp_mb();
> -       raw_spin_unlock_wait(&current->pi_lock);
> +       raw_spin_lock(&current->pi_lock);
> +       raw_spin_unlock(&current->pi_lock);

Does the raw_spin_lock()/raw_spin_unlock() imply an smp_mb() or stronger?
Maybe it would be clearer to remove the extra barrier if so.

     Arnd

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

#1678770 — Re: [PATCH RFC 03/26] sched: Replace spin_unlock_wait() with lock/unlock pair

On Fri, Jun 30, 2017 at 12:31:50PM +0200, Arnd Bergmann wrote:
> On Fri, Jun 30, 2017 at 2:01 AM, Paul E. McKenney
> <paulmck@linux.vnet.ibm.com> wrote:
> > There is no agreed-upon definition of spin_unlock_wait()'s semantics,
> > and it appears that all callers could do just as well with a lock/unlock
> > pair.  This commit therefore replaces the spin_unlock_wait() call in
> > do_task_dead() with spin_lock() followed immediately by spin_unlock().
> > This should be safe from a performance perspective because the lock is
> > this tasks ->pi_lock, and this is called only after the task exits.
> >
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index e91138fcde86..6dea3d9728c8 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -3461,7 +3461,8 @@ void __noreturn do_task_dead(void)
> >          * is held by try_to_wake_up()
> >          */
> >         smp_mb();
> > -       raw_spin_unlock_wait(&current->pi_lock);
> > +       raw_spin_lock(&current->pi_lock);
> > +       raw_spin_unlock(&current->pi_lock);
> 
> Does the raw_spin_lock()/raw_spin_unlock() imply an smp_mb() or stronger?
> Maybe it would be clearer to remove the extra barrier if so.

No, it does not in general, but it does on most architectures, and
there are ways to allow those architectures to gain the benefit of their
stronger locks.  For example, would this work?  

> >          * is held by try_to_wake_up()
> >          */
> > -       smp_mb();
> > -       raw_spin_unlock_wait(&current->pi_lock);
> > +       smp_mb__before_spinlock();
> > +       raw_spin_lock(&current->pi_lock);
> > +       raw_spin_unlock(&current->pi_lock);

							Thanx, Paul

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

#1678332 — [PATCH RFC 05/26] exit: Replace spin_unlock_wait() with lock/unlock pair

There is no agreed-upon definition of spin_unlock_wait()'s semantics, and
it appears that all callers could do just as well with a lock/unlock pair.
This commit therefore replaces the spin_unlock_wait() call in do_exit()
with spin_lock() followed immediately by spin_unlock().  This should be
safe from a performance perspective because the lock is a per-task lock,
and this is happening only at task-exit time.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 kernel/exit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 516acdb0e0ec..1a976e47ddd1 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -832,7 +832,8 @@ void __noreturn do_exit(long code)
 	 * Ensure that we must observe the pi_state in exit_mm() ->
 	 * mm_release() -> exit_pi_state_list().
 	 */
-	raw_spin_unlock_wait(&tsk->pi_lock);
+	raw_spin_lock(&tsk->pi_lock);
+	raw_spin_unlock(&tsk->pi_lock);
 
 	if (unlikely(in_atomic())) {
 		pr_info("note: %s[%d] exited with preempt_count %d\n",
-- 
2.5.2

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

#1678333 — [PATCH RFC 12/26] arm64: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: <linux-arm-kernel@lists.infradead.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/arm64/include/asm/spinlock.h | 58 ++++-----------------------------------
 1 file changed, 5 insertions(+), 53 deletions(-)

diff --git a/arch/arm64/include/asm/spinlock.h b/arch/arm64/include/asm/spinlock.h
index cae331d553f8..f445bd7f2b9f 100644
--- a/arch/arm64/include/asm/spinlock.h
+++ b/arch/arm64/include/asm/spinlock.h
@@ -26,58 +26,6 @@
  * The memory barriers are implicit with the load-acquire and store-release
  * instructions.
  */
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	unsigned int tmp;
-	arch_spinlock_t lockval;
-	u32 owner;
-
-	/*
-	 * Ensure prior spin_lock operations to other locks have completed
-	 * on this CPU before we test whether "lock" is locked.
-	 */
-	smp_mb();
-	owner = READ_ONCE(lock->owner) << 16;
-
-	asm volatile(
-"	sevl\n"
-"1:	wfe\n"
-"2:	ldaxr	%w0, %2\n"
-	/* Is the lock free? */
-"	eor	%w1, %w0, %w0, ror #16\n"
-"	cbz	%w1, 3f\n"
-	/* Lock taken -- has there been a subsequent unlock->lock transition? */
-"	eor	%w1, %w3, %w0, lsl #16\n"
-"	cbz	%w1, 1b\n"
-	/*
-	 * The owner has been updated, so there was an unlock->lock
-	 * transition that we missed. That means we can rely on the
-	 * store-release of the unlock operation paired with the
-	 * load-acquire of the lock operation to publish any of our
-	 * previous stores to the new lock owner and therefore don't
-	 * need to bother with the writeback below.
-	 */
-"	b	4f\n"
-"3:\n"
-	/*
-	 * Serialise against any concurrent lockers by writing back the
-	 * unlocked lock value
-	 */
-	ARM64_LSE_ATOMIC_INSN(
-	/* LL/SC */
-"	stxr	%w1, %w0, %2\n"
-	__nops(2),
-	/* LSE atomics */
-"	mov	%w1, %w0\n"
-"	cas	%w0, %w0, %2\n"
-"	eor	%w1, %w1, %w0\n")
-	/* Somebody else wrote to the lock, GOTO 10 and reload the value */
-"	cbnz	%w1, 2b\n"
-"4:"
-	: "=&r" (lockval), "=&r" (tmp), "+Q" (*lock)
-	: "r" (owner)
-	: "memory");
-}
 
 #define arch_spin_lock_flags(lock, flags) arch_spin_lock(lock)
 
@@ -176,7 +124,11 @@ static inline int arch_spin_value_unlocked(arch_spinlock_t lock)
 
 static inline int arch_spin_is_locked(arch_spinlock_t *lock)
 {
-	smp_mb(); /* See arch_spin_unlock_wait */
+	/*
+	 * Ensure prior spin_lock operations to other locks have completed
+	 * on this CPU before we test whether "lock" is locked.
+	 */
+	smp_mb(); /* ^^^ */
 	return !arch_spin_value_unlocked(READ_ONCE(*lock));
 }
 
-- 
2.5.2

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

#1678629 — Re: [PATCH RFC 12/26] arm64: Remove spin_unlock_wait() arch-specific definitions

On Thu, Jun 29, 2017 at 05:01:20PM -0700, Paul E. McKenney wrote:
> There is no agreed-upon definition of spin_unlock_wait()'s semantics,
> and it appears that all callers could do just as well with a lock/unlock
> pair.  This commit therefore removes the underlying arch-specific
> arch_spin_unlock_wait().
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: <linux-arm-kernel@lists.infradead.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Andrea Parri <parri.andrea@gmail.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> ---
>  arch/arm64/include/asm/spinlock.h | 58 ++++-----------------------------------
>  1 file changed, 5 insertions(+), 53 deletions(-)

I'm going to miss this code.

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

Will

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

#1679026 — Re: [PATCH RFC 12/26] arm64: Remove spin_unlock_wait() arch-specific definitions

On Fri, Jun 30, 2017 at 10:20:57AM +0100, Will Deacon wrote:
> On Thu, Jun 29, 2017 at 05:01:20PM -0700, Paul E. McKenney wrote:
> > There is no agreed-upon definition of spin_unlock_wait()'s semantics,
> > and it appears that all callers could do just as well with a lock/unlock
> > pair.  This commit therefore removes the underlying arch-specific
> > arch_spin_unlock_wait().
> > 
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: <linux-arm-kernel@lists.infradead.org>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Alan Stern <stern@rowland.harvard.edu>
> > Cc: Andrea Parri <parri.andrea@gmail.com>
> > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > ---
> >  arch/arm64/include/asm/spinlock.h | 58 ++++-----------------------------------
> >  1 file changed, 5 insertions(+), 53 deletions(-)
> 
> I'm going to miss this code.

;-) ;-) ;-)

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

Applied, thank you!

							Thanx, Paul

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

#1678334 — [PATCH RFC 10/26] arc: Remove spin_unlock_wait() arch-specific definitions

There is no agreed-upon definition of spin_unlock_wait()'s semantics,
and it appears that all callers could do just as well with a lock/unlock
pair.  This commit therefore removes the underlying arch-specific
arch_spin_unlock_wait().

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: <linux-snps-arc@lists.infradead.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andrea Parri <parri.andrea@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 arch/arc/include/asm/spinlock.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arc/include/asm/spinlock.h b/arch/arc/include/asm/spinlock.h
index 233d5ffe6ec7..a325e6a36523 100644
--- a/arch/arc/include/asm/spinlock.h
+++ b/arch/arc/include/asm/spinlock.h
@@ -16,11 +16,6 @@
 #define arch_spin_is_locked(x)	((x)->slock != __ARCH_SPIN_LOCK_UNLOCKED__)
 #define arch_spin_lock_flags(lock, flags)	arch_spin_lock(lock)
 
-static inline void arch_spin_unlock_wait(arch_spinlock_t *lock)
-{
-	smp_cond_load_acquire(&lock->slock, !VAL);
-}
-
 #ifdef CONFIG_ARC_HAS_LLSC
 
 static inline void arch_spin_lock(arch_spinlock_t *lock)
-- 
2.5.2

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

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web