linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon@arm.com>
To: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, peterz@infradead.org,
	mingo@kernel.org, boqun.feng@gmail.com,
	paulmck@linux.vnet.ibm.com, longman@redhat.com,
	Will Deacon <will.deacon@arm.com>
Subject: [PATCH v2 02/13] locking/qspinlock: Bound spinning on pending->locked transition in slowpath
Date: Wed, 11 Apr 2018 19:01:09 +0100	[thread overview]
Message-ID: <1523469680-17699-3-git-send-email-will.deacon@arm.com> (raw)
In-Reply-To: <1523469680-17699-1-git-send-email-will.deacon@arm.com>

If a locker taking the qspinlock slowpath reads a lock value indicating
that only the pending bit is set, then it will spin whilst the
concurrent pending->locked transition takes effect.

Unfortunately, there is no guarantee that such a transition will ever be
observed since concurrent lockers could continuously set pending and
hand over the lock amongst themselves, leading to starvation. Whilst
this would probably resolve in practice, it means that it is not
possible to prove liveness properties about the lock and means that lock
acquisition time is unbounded.

Rather than removing the pending->locked spinning from the slowpath
altogether (which has been shown to heavily penalise a 2-threaded
locking stress test on x86), this patch replaces the explicit spinning
with a call to atomic_cond_read_relaxed and allows the architecture to
provide a bound on the number of spins. For architectures that can
respond to changes in cacheline state in their smp_cond_load implementation,
it should be sufficient to use the default bound of 1.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Suggested-by: Waiman Long <longman@redhat.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---
 kernel/locking/qspinlock.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index d880296245c5..396701e8c62d 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -77,6 +77,18 @@
 #endif
 
 /*
+ * The pending bit spinning loop count.
+ * This heuristic is used to limit the number of lockword accesses
+ * made by atomic_cond_read_relaxed when waiting for the lock to
+ * transition out of the "== _Q_PENDING_VAL" state. We don't spin
+ * indefinitely because there's no guarantee that we'll make forward
+ * progress.
+ */
+#ifndef _Q_PENDING_LOOPS
+#define _Q_PENDING_LOOPS	1
+#endif
+
+/*
  * Per-CPU queue node structures; we can never have more than 4 nested
  * contexts: task, softirq, hardirq, nmi.
  *
@@ -306,13 +318,15 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
 		return;
 
 	/*
-	 * wait for in-progress pending->locked hand-overs
+	 * Wait for in-progress pending->locked hand-overs with a bounded
+	 * number of spins so that we guarantee forward progress.
 	 *
 	 * 0,1,0 -> 0,0,1
 	 */
 	if (val == _Q_PENDING_VAL) {
-		while ((val = atomic_read(&lock->val)) == _Q_PENDING_VAL)
-			cpu_relax();
+		int cnt = _Q_PENDING_LOOPS;
+		val = atomic_cond_read_relaxed(&lock->val,
+					       (VAL != _Q_PENDING_VAL) || !cnt--);
 	}
 
 	/*
-- 
2.1.4

  parent reply	other threads:[~2018-04-11 18:01 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-11 18:01 [PATCH v2 00/13] kernel/locking: qspinlock improvements Will Deacon
2018-04-11 18:01 ` [PATCH v2 01/13] barriers: Introduce smp_cond_load_relaxed and atomic_cond_read_relaxed Will Deacon
2018-04-11 18:01 ` Will Deacon [this message]
2018-04-11 18:01 ` [PATCH v2 03/13] locking/qspinlock/x86: Increase _Q_PENDING_LOOPS upper bound Will Deacon
2018-04-11 18:01 ` [PATCH v2 04/13] locking/qspinlock: Remove unbounded cmpxchg loop from locking slowpath Will Deacon
2018-04-11 19:34   ` Waiman Long
2018-04-11 20:35     ` Waiman Long
2018-04-11 19:53   ` Waiman Long
2018-04-12 14:06     ` Will Deacon
2018-04-12 14:16       ` Waiman Long
2018-04-12 14:18         ` Will Deacon
2018-04-11 18:01 ` [PATCH v2 05/13] locking/qspinlock: Kill cmpxchg loop when claiming lock from head of queue Will Deacon
2018-04-11 18:01 ` [PATCH v2 06/13] locking/qspinlock: Use atomic_cond_read_acquire Will Deacon
2018-04-11 18:01 ` [PATCH v2 07/13] locking/mcs: Use smp_cond_load_acquire() in mcs spin loop Will Deacon
2018-04-11 18:01 ` [PATCH v2 08/13] locking/qspinlock: Use smp_cond_load_relaxed to wait for next node Will Deacon
2018-04-11 18:01 ` [PATCH v2 09/13] locking/qspinlock: Merge struct __qspinlock into struct qspinlock Will Deacon
2018-04-11 19:13   ` Waiman Long
2018-04-11 18:01 ` [PATCH v2 10/13] locking/qspinlock: Make queued_spin_unlock use smp_store_release Will Deacon
2018-04-11 18:01 ` [PATCH v2 11/13] locking/qspinlock: Elide back-to-back RELEASE operations with smp_wmb() Will Deacon
2018-04-11 18:01 ` [PATCH v2 12/13] locking/qspinlock: Use try_cmpxchg instead of cmpxchg when locking Will Deacon
2018-04-11 18:01 ` [PATCH v2 13/13] locking/qspinlock: Add stat tracking for pending vs slowpath Will Deacon
2018-04-13  9:24 ` [PATCH v2 00/13] kernel/locking: qspinlock improvements Catalin Marinas

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=1523469680-17699-3-git-send-email-will.deacon@arm.com \
    --to=will.deacon@arm.com \
    --cc=boqun.feng@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    /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).