linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: linux-kernel@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
	oleg@redhat.com, akpm@linux-foundation.org, mingo@redhat.com,
	dave@stgolabs.net, manfred@colorfullife.com, tj@kernel.org,
	arnd@arndb.de, linux-arch@vger.kernel.org, will.deacon@arm.com,
	peterz@infradead.org, stern@rowland.harvard.edu,
	parri.andrea@gmail.com, torvalds@linux-foundation.org,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: [PATCH v3 8/9] locking: Remove spin_unlock_wait() generic definitions
Date: Fri,  7 Jul 2017 12:28:25 -0700	[thread overview]
Message-ID: <1499455706-9398-8-git-send-email-paulmck@linux.vnet.ibm.com> (raw)
In-Reply-To: <20170707192704.GA6144@linux.vnet.ibm.com>

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 spin_unlock_wait() and related
definitions from core code.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Ingo Molnar <mingo@redhat.com>
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>
---
 include/asm-generic/qspinlock.h |  14 -----
 include/linux/spinlock.h        |  31 -----------
 include/linux/spinlock_up.h     |   6 ---
 kernel/locking/qspinlock.c      | 117 ----------------------------------------
 4 files changed, 168 deletions(-)

diff --git a/include/asm-generic/qspinlock.h b/include/asm-generic/qspinlock.h
index 9f0681bf1e87..66260777d644 100644
--- a/include/asm-generic/qspinlock.h
+++ b/include/asm-generic/qspinlock.h
@@ -22,17 +22,6 @@
 #include <asm-generic/qspinlock_types.h>
 
 /**
- * queued_spin_unlock_wait - wait until the _current_ lock holder releases the lock
- * @lock : Pointer to queued spinlock structure
- *
- * There is a very slight possibility of live-lock if the lockers keep coming
- * and the waiter is just unfortunate enough to not see any unlock state.
- */
-#ifndef queued_spin_unlock_wait
-extern void queued_spin_unlock_wait(struct qspinlock *lock);
-#endif
-
-/**
  * queued_spin_is_locked - is the spinlock locked?
  * @lock: Pointer to queued spinlock structure
  * Return: 1 if it is locked, 0 otherwise
@@ -41,8 +30,6 @@ extern void queued_spin_unlock_wait(struct qspinlock *lock);
 static __always_inline int queued_spin_is_locked(struct qspinlock *lock)
 {
 	/*
-	 * See queued_spin_unlock_wait().
-	 *
 	 * Any !0 state indicates it is locked, even if _Q_LOCKED_VAL
 	 * isn't immediately observable.
 	 */
@@ -135,6 +122,5 @@ static __always_inline bool virt_spin_lock(struct qspinlock *lock)
 #define arch_spin_trylock(l)		queued_spin_trylock(l)
 #define arch_spin_unlock(l)		queued_spin_unlock(l)
 #define arch_spin_lock_flags(l, f)	queued_spin_lock(l)
-#define arch_spin_unlock_wait(l)	queued_spin_unlock_wait(l)
 
 #endif /* __ASM_GENERIC_QSPINLOCK_H */
diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index d9510e8522d4..ef018a6e4985 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -130,12 +130,6 @@ do {								\
 #define smp_mb__before_spinlock()	smp_wmb()
 #endif
 
-/**
- * raw_spin_unlock_wait - wait until the spinlock gets unlocked
- * @lock: the spinlock in question.
- */
-#define raw_spin_unlock_wait(lock)	arch_spin_unlock_wait(&(lock)->raw_lock)
-
 #ifdef CONFIG_DEBUG_SPINLOCK
  extern void do_raw_spin_lock(raw_spinlock_t *lock) __acquires(lock);
 #define do_raw_spin_lock_flags(lock, flags) do_raw_spin_lock(lock)
@@ -369,31 +363,6 @@ static __always_inline int spin_trylock_irq(spinlock_t *lock)
 	raw_spin_trylock_irqsave(spinlock_check(lock), flags); \
 })
 
-/**
- * spin_unlock_wait - Interpose between successive critical sections
- * @lock: the spinlock whose critical sections are to be interposed.
- *
- * Semantically this is equivalent to a spin_lock() immediately
- * followed by a spin_unlock().  However, most architectures have
- * more efficient implementations in which the spin_unlock_wait()
- * cannot block concurrent lock acquisition, and in some cases
- * where spin_unlock_wait() does not write to the lock variable.
- * Nevertheless, spin_unlock_wait() can have high overhead, so if
- * you feel the need to use it, please check to see if there is
- * a better way to get your job done.
- *
- * The ordering guarantees provided by spin_unlock_wait() are:
- *
- * 1.  All accesses preceding the spin_unlock_wait() happen before
- *     any accesses in later critical sections for this same lock.
- * 2.  All accesses following the spin_unlock_wait() happen after
- *     any accesses in earlier critical sections for this same lock.
- */
-static __always_inline void spin_unlock_wait(spinlock_t *lock)
-{
-	raw_spin_unlock_wait(&lock->rlock);
-}
-
 static __always_inline int spin_is_locked(spinlock_t *lock)
 {
 	return raw_spin_is_locked(&lock->rlock);
diff --git a/include/linux/spinlock_up.h b/include/linux/spinlock_up.h
index 0d9848de677d..612fb530af41 100644
--- a/include/linux/spinlock_up.h
+++ b/include/linux/spinlock_up.h
@@ -26,11 +26,6 @@
 #ifdef CONFIG_DEBUG_SPINLOCK
 #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);
-}
-
 static inline void arch_spin_lock(arch_spinlock_t *lock)
 {
 	lock->slock = 0;
@@ -73,7 +68,6 @@ static inline void arch_spin_unlock(arch_spinlock_t *lock)
 
 #else /* DEBUG_SPINLOCK */
 #define arch_spin_is_locked(lock)	((void)(lock), 0)
-#define arch_spin_unlock_wait(lock)	do { barrier(); (void)(lock); } while (0)
 /* for sched/core.c and kernel_lock.c: */
 # define arch_spin_lock(lock)		do { barrier(); (void)(lock); } while (0)
 # define arch_spin_lock_flags(lock, flags)	do { barrier(); (void)(lock); } while (0)
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index b2caec7315af..64a9051e4c2c 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -267,123 +267,6 @@ static __always_inline u32  __pv_wait_head_or_lock(struct qspinlock *lock,
 #define queued_spin_lock_slowpath	native_queued_spin_lock_slowpath
 #endif
 
-/*
- * Various notes on spin_is_locked() and spin_unlock_wait(), which are
- * 'interesting' functions:
- *
- * PROBLEM: some architectures have an interesting issue with atomic ACQUIRE
- * operations in that the ACQUIRE applies to the LOAD _not_ the STORE (ARM64,
- * PPC). Also qspinlock has a similar issue per construction, the setting of
- * the locked byte can be unordered acquiring the lock proper.
- *
- * This gets to be 'interesting' in the following cases, where the /should/s
- * end up false because of this issue.
- *
- *
- * CASE 1:
- *
- * So the spin_is_locked() correctness issue comes from something like:
- *
- *   CPU0				CPU1
- *
- *   global_lock();			local_lock(i)
- *     spin_lock(&G)			  spin_lock(&L[i])
- *     for (i)				  if (!spin_is_locked(&G)) {
- *       spin_unlock_wait(&L[i]);	    smp_acquire__after_ctrl_dep();
- *					    return;
- *					  }
- *					  // deal with fail
- *
- * Where it is important CPU1 sees G locked or CPU0 sees L[i] locked such
- * that there is exclusion between the two critical sections.
- *
- * The load from spin_is_locked(&G) /should/ be constrained by the ACQUIRE from
- * spin_lock(&L[i]), and similarly the load(s) from spin_unlock_wait(&L[i])
- * /should/ be constrained by the ACQUIRE from spin_lock(&G).
- *
- * Similarly, later stuff is constrained by the ACQUIRE from CTRL+RMB.
- *
- *
- * CASE 2:
- *
- * For spin_unlock_wait() there is a second correctness issue, namely:
- *
- *   CPU0				CPU1
- *
- *   flag = set;
- *   smp_mb();				spin_lock(&l)
- *   spin_unlock_wait(&l);		if (!flag)
- *					  // add to lockless list
- *					spin_unlock(&l);
- *   // iterate lockless list
- *
- * Which wants to ensure that CPU1 will stop adding bits to the list and CPU0
- * will observe the last entry on the list (if spin_unlock_wait() had ACQUIRE
- * semantics etc..)
- *
- * Where flag /should/ be ordered against the locked store of l.
- */
-
-/*
- * queued_spin_lock_slowpath() can (load-)ACQUIRE the lock before
- * issuing an _unordered_ store to set _Q_LOCKED_VAL.
- *
- * This means that the store can be delayed, but no later than the
- * store-release from the unlock. This means that simply observing
- * _Q_LOCKED_VAL is not sufficient to determine if the lock is acquired.
- *
- * There are two paths that can issue the unordered store:
- *
- *  (1) clear_pending_set_locked():	*,1,0 -> *,0,1
- *
- *  (2) set_locked():			t,0,0 -> t,0,1 ; t != 0
- *      atomic_cmpxchg_relaxed():	t,0,0 -> 0,0,1
- *
- * However, in both cases we have other !0 state we've set before to queue
- * ourseves:
- *
- * For (1) we have the atomic_cmpxchg_acquire() that set _Q_PENDING_VAL, our
- * load is constrained by that ACQUIRE to not pass before that, and thus must
- * observe the store.
- *
- * For (2) we have a more intersting scenario. We enqueue ourselves using
- * xchg_tail(), which ends up being a RELEASE. This in itself is not
- * sufficient, however that is followed by an smp_cond_acquire() on the same
- * word, giving a RELEASE->ACQUIRE ordering. This again constrains our load and
- * guarantees we must observe that store.
- *
- * Therefore both cases have other !0 state that is observable before the
- * unordered locked byte store comes through. This means we can use that to
- * wait for the lock store, and then wait for an unlock.
- */
-#ifndef queued_spin_unlock_wait
-void queued_spin_unlock_wait(struct qspinlock *lock)
-{
-	u32 val;
-
-	for (;;) {
-		val = atomic_read(&lock->val);
-
-		if (!val) /* not locked, we're done */
-			goto done;
-
-		if (val & _Q_LOCKED_MASK) /* locked, go wait for unlock */
-			break;
-
-		/* not locked, but pending, wait until we observe the lock */
-		cpu_relax();
-	}
-
-	/* any unlock is good */
-	while (atomic_read(&lock->val) & _Q_LOCKED_MASK)
-		cpu_relax();
-
-done:
-	smp_acquire__after_ctrl_dep();
-}
-EXPORT_SYMBOL(queued_spin_unlock_wait);
-#endif
-
 #endif /* _GEN_PV_LOCK_SLOWPATH */
 
 /**
-- 
2.5.2

  parent reply	other threads:[~2017-07-07 19:30 UTC|newest]

Thread overview: 122+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-29 23:59 [PATCH RFC 0/26] Remove spin_unlock_wait() Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 01/26] netfilter: Replace spin_unlock_wait() with lock/unlock pair Paul E. McKenney
     [not found]   ` <a6642feb-2f3a-980f-5ed6-2deb79563e6b@colorfullife.com>
2017-07-02  2:00     ` Paul E. McKenney
2017-07-03 14:39     ` Alan Stern
2017-07-03 17:14       ` Paul E. McKenney
2017-07-03 19:01         ` Manfred Spraul
2017-07-03 19:57           ` Alan Stern
2017-07-06 18:43             ` Manfred Spraul
2017-07-03 20:04         ` Alan Stern
2017-07-03 20:53           ` Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 02/26] task_work: " Paul E. McKenney
2017-06-30 11:04   ` Oleg Nesterov
2017-06-30 12:50     ` Paul E. McKenney
2017-06-30 15:20       ` Oleg Nesterov
2017-06-30 16:16         ` Paul E. McKenney
2017-06-30 17:21           ` Paul E. McKenney
2017-06-30 19:21           ` Oleg Nesterov
2017-06-30 19:50             ` Alan Stern
2017-06-30 20:04               ` Paul E. McKenney
2017-06-30 20:02             ` Paul E. McKenney
2017-06-30 20:19               ` Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 03/26] sched: " Paul E. McKenney
2017-06-30 10:31   ` Arnd Bergmann
2017-06-30 12:35     ` Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 04/26] completion: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 05/26] exit: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 07/26] drivers/ata: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 08/26] locking: Remove spin_unlock_wait() generic definitions Paul E. McKenney
2017-06-30  9:19   ` Will Deacon
2017-06-30 12:38     ` Paul E. McKenney
2017-06-30 13:13       ` Will Deacon
2017-06-30 22:18         ` Paul E. McKenney
2017-07-03 13:15           ` Will Deacon
2017-07-03 16:18             ` Paul E. McKenney
2017-07-03 16:40               ` Linus Torvalds
2017-07-03 17:13                 ` Will Deacon
2017-07-03 22:30                   ` Paul E. McKenney
2017-07-03 22:49                     ` Linus Torvalds
2017-07-04  0:39                       ` Paul E. McKenney
2017-07-04  0:54                         ` Paul E. McKenney
2017-07-03 21:10                 ` Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 09/26] alpha: Remove spin_unlock_wait() arch-specific definitions Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 10/26] arc: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 11/26] arm: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 12/26] arm64: " Paul E. McKenney
2017-06-30  9:20   ` Will Deacon
2017-06-30 17:29     ` Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 13/26] blackfin: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 14/26] hexagon: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 15/26] ia64: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 16/26] m32r: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 18/26] mips: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 19/26] mn10300: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 20/26] parisc: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 21/26] powerpc: " Paul E. McKenney
2017-07-02  3:58   ` Boqun Feng
2017-07-05 23:57     ` Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 22/26] s390: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 23/26] sh: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 24/26] sparc: " Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 25/26] tile: " Paul E. McKenney
2017-06-30  0:06   ` Linus Torvalds
2017-06-30  0:09     ` Paul E. McKenney
2017-06-30  0:14       ` Paul E. McKenney
2017-06-30  0:10     ` Linus Torvalds
2017-06-30  0:24       ` Paul E. McKenney
2017-06-30  0:01 ` [PATCH RFC 26/26] xtensa: " Paul E. McKenney
     [not found] ` <1498780894-8253-6-git-send-email-paulmck@linux.vnet.ibm.com>
2017-07-01 19:23   ` [PATCH RFC 06/26] ipc: Replace spin_unlock_wait() with lock/unlock pair Manfred Spraul
2017-07-02  3:16     ` Paul E. McKenney
2017-07-05 23:29 ` [PATCH v2 0/9] Remove spin_unlock_wait() Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 1/9] net/netfilter/nf_conntrack_core: Fix net_conntrack_lock() Paul E. McKenney
2017-07-06 18:45     ` Manfred Spraul
2017-07-06 20:26       ` Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 2/9] task_work: Replace spin_unlock_wait() with lock/unlock pair Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 3/9] sched: " Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 4/9] completion: " Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 5/9] exit: " Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 6/9] ipc: " Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 7/9] drivers/ata: " Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 8/9] locking: Remove spin_unlock_wait() generic definitions Paul E. McKenney
2017-07-05 23:31   ` [PATCH v2 9/9] arch: Remove spin_unlock_wait() arch-specific definitions Paul E. McKenney
2017-07-06 14:12   ` [PATCH v2 0/9] Remove spin_unlock_wait() David Laight
2017-07-06 15:21     ` Paul E. McKenney
2017-07-06 16:10       ` Peter Zijlstra
2017-07-06 16:24         ` Paul E. McKenney
2017-07-06 16:41           ` Peter Zijlstra
2017-07-06 17:03             ` Paul E. McKenney
2017-07-06 16:49           ` Alan Stern
2017-07-06 16:54             ` Peter Zijlstra
2017-07-06 19:37               ` Alan Stern
2017-07-06 16:05     ` Peter Zijlstra
2017-07-06 16:20       ` Paul E. McKenney
2017-07-06 16:50         ` Peter Zijlstra
2017-07-06 17:08           ` Will Deacon
2017-07-06 17:29             ` Paul E. McKenney
2017-07-06 17:18           ` Paul E. McKenney
2017-07-07  8:31           ` Ingo Molnar
2017-07-07  8:44             ` Peter Zijlstra
2017-07-07 10:33               ` Ingo Molnar
2017-07-07 11:23                 ` Peter Zijlstra
2017-07-07 14:41             ` Paul E. McKenney
2017-07-08  8:43               ` Ingo Molnar
2017-07-08 11:41                 ` Paul E. McKenney
2017-07-07 17:47             ` Manfred Spraul
2017-07-08  8:35               ` Ingo Molnar
2017-07-08 11:39                 ` Paul E. McKenney
2017-07-08 12:30                   ` Ingo Molnar
2017-07-08 14:45                     ` Paul E. McKenney
2017-07-08 16:21                     ` Alan Stern
2017-07-10 17:22                       ` Manfred Spraul
2017-07-07  8:06       ` Ingo Molnar
2017-07-07  9:32         ` Ingo Molnar
2017-07-07 19:27   ` [PATCH v3 " Paul E. McKenney
2017-07-07 19:28     ` [PATCH v3 1/9] net/netfilter/nf_conntrack_core: Fix net_conntrack_lock() Paul E. McKenney
2017-07-07 19:28     ` [PATCH v3 2/9] task_work: Replace spin_unlock_wait() with lock/unlock pair Paul E. McKenney
2017-07-07 19:28     ` [PATCH v3 3/9] sched: " Paul E. McKenney
2017-07-07 19:28     ` [PATCH v3 4/9] completion: " Paul E. McKenney
2017-07-07 19:28     ` [PATCH v3 5/9] exit: " Paul E. McKenney
2017-07-07 19:28     ` [PATCH v3 6/9] ipc: " Paul E. McKenney
2017-07-07 19:28     ` [PATCH v3 7/9] drivers/ata: " Paul E. McKenney
2017-07-07 19:28     ` Paul E. McKenney [this message]
2017-07-07 19:28     ` [PATCH v3 9/9] arch: Remove spin_unlock_wait() arch-specific definitions Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1499455706-9398-8-git-send-email-paulmck@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=dave@stgolabs.net \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manfred@colorfullife.com \
    --cc=mingo@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=parri.andrea@gmail.com \
    --cc=peterz@infradead.org \
    --cc=stern@rowland.harvard.edu \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).