linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Waiman Long <waiman.long@hp.com>
Cc: arnd@arndb.de, linux-arch@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, rostedt@goodmis.org,
	akpm@linux-foundation.org, walken@google.com,
	andi@firstfloor.org, riel@redhat.com, paulmck@linux.vnet.ibm.com,
	torvalds@linux-foundation.org, oleg@redhat.com,
	Peter Zijlstra <peterz@infradead.org>
Subject: [RFC][PATCH 5/7] qspinlock: Optimize the pending case
Date: Mon, 10 Mar 2014 16:42:41 +0100	[thread overview]
Message-ID: <20140310155543.644544511@infradead.org> (raw)
In-Reply-To: 20140310154236.038181843@infradead.org

[-- Attachment #1: peterz-qspinlock-pending-test_and_set_bit.patch --]
[-- Type: text/plain, Size: 2958 bytes --]

Replace the initial set-pending cmpxchg() loop with an unconditional
test-and-set bit (x86: bts) instruction.

It looses the direct trylock state transition; however since that should
be very unlikely (we've just done a trylock) that shouldn't be a
problem.

Signed-off-by: Peter Zijlstra <peterz@infradead.org>
---
 include/asm-generic/qspinlock_types.h |    2 +
 kernel/locking/qspinlock.c            |   60 +++++++++++++++++++---------------
 2 files changed, 36 insertions(+), 26 deletions(-)

--- a/include/asm-generic/qspinlock_types.h
+++ b/include/asm-generic/qspinlock_types.h
@@ -59,6 +59,8 @@ typedef struct qspinlock {
 #define _Q_TAIL_CPU_BITS	(32 - _Q_TAIL_CPU_OFFSET)
 #define _Q_TAIL_CPU_MASK	(((1U << _Q_TAIL_CPU_BITS) - 1) << _Q_TAIL_CPU_OFFSET)
 
+#define _Q_TAIL_MASK		(_Q_TAIL_IDX_MASK | _Q_TAIL_CPU_MASK)
+
 #define _Q_LOCKED_VAL		(1U << _Q_LOCKED_OFFSET)
 #define _Q_PENDING_VAL		(1U << _Q_PENDING_OFFSET)
 
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -83,6 +83,37 @@ static inline struct mcs_spinlock *decod
 	return per_cpu_ptr(&mcs_nodes[idx], cpu);
 }
 
+/*
+ * 0,0,1 -> 0,1,* ; pending
+ *
+ * Ignore the locked bit; if we set pending and locked happens to be clear
+ * we'll fall through on the subsequent wait.
+ */
+static int __always_inline
+try_set_pending(struct qspinlock *lock, u32 val)
+{
+	if (val & ~_Q_LOCKED_MASK)
+		return 0; /* fail; queue */
+
+	/*
+	 * If we find the pending bit was already set; fail and queue.
+	 */
+	if (atomic_test_and_set_bit(_Q_PENDING_OFFSET, &lock->val))
+		return 0;
+
+	/*
+	 * If we raced and someone concurrently set the tail; no problem. He
+	 * need not have observed our pending bit and can have claimed the
+	 * lock.
+	 *
+	 * The next node in line however will wait for the pending to go away
+	 * again though, so in effect we've just flipped order between two
+	 * contenders which already had undetermined order as per the race.
+	 */
+
+	return 1;
+}
+
 #define _Q_LOCKED_PENDING_MASK	(_Q_LOCKED_MASK | _Q_PENDING_MASK)
 
 /**
@@ -115,34 +146,10 @@ void queue_spin_lock_slowpath(struct qsp
 	BUILD_BUG_ON(CONFIG_NR_CPUS >= (1U << _Q_TAIL_CPU_BITS));
 
 	/*
-	 * trylock || pending
-	 *
-	 * 0,0,0 -> 0,0,1 ; trylock
 	 * 0,0,1 -> 0,1,1 ; pending
 	 */
-	for (;;) {
-		/*
-		 * If we observe any contention; queue.
-		 */
-		if (val & ~_Q_LOCKED_MASK)
-			goto queue;
-
-		new = _Q_LOCKED_VAL;
-		if (val == new)
-			new |= _Q_PENDING_VAL;
-
-		old = atomic_cmpxchg(&lock->val, val, new);
-		if (old == val)
-			break;
-
-		val = old;
-	}
-
-	/*
-	 * we won the trylock
-	 */
-	if (new == _Q_LOCKED_VAL)
-		return;
+	if (!try_set_pending(lock, val))
+		goto queue;
 
 	/*
 	 * we're pending, wait for the owner to go away.
@@ -186,6 +193,7 @@ void queue_spin_lock_slowpath(struct qsp
 	 * 0,0,0 -> 0,0,1 ; trylock
 	 * p,y,x -> n,y,x ; prev = xchg(lock, node)
 	 */
+	val = atomic_read(&lock->val);
 	for (;;) {
 		new = _Q_LOCKED_VAL;
 		if (val)

  parent reply	other threads:[~2014-03-10 15:42 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-10 15:42 [RFC][PATCH 0/7] locking: qspinlock Peter Zijlstra
2014-03-10 15:42 ` [RFC][PATCH 1/7] qspinlock: Introducing a 4-byte queue spinlock implementation Peter Zijlstra
2014-03-10 15:42   ` Peter Zijlstra
2014-03-13 13:07   ` Peter Zijlstra
2014-03-10 15:42 ` [RFC][PATCH 2/7] qspinlock, x86: Enable x86 to use queue spinlock Peter Zijlstra
2014-03-10 15:42   ` Peter Zijlstra
2014-03-10 15:42 ` [RFC][PATCH 3/7] qspinlock: Add pending bit Peter Zijlstra
2014-03-10 15:42   ` Peter Zijlstra
2014-03-10 15:42 ` [RFC][PATCH 4/7] x86: Add atomic_test_and_set_bit() Peter Zijlstra
2014-03-10 15:42 ` Peter Zijlstra [this message]
2014-03-10 15:42   ` [RFC][PATCH 5/7] qspinlock: Optimize the pending case Peter Zijlstra
2014-03-10 15:42 ` [RFC][PATCH 6/7] qspinlock: Optimize xchg_tail Peter Zijlstra
2014-03-10 15:42   ` Peter Zijlstra
2014-03-10 15:42 ` [RFC][PATCH 7/7] qspinlock: Optimize for smaller NR_CPUS Peter Zijlstra
2014-03-10 15:42   ` Peter Zijlstra
2014-03-11 10:45 ` [RFC][PATCH 0/7] locking: qspinlock Ingo Molnar
2014-03-11 11:02   ` Peter Zijlstra
2014-03-11 11:04     ` Ingo Molnar
2014-03-12  3:17   ` Waiman Long
2014-03-12  6:24     ` Peter Zijlstra
2014-03-12 15:32       ` Peter Zijlstra
2014-03-12 19:00       ` Waiman Long
2014-03-12  2:31 ` Dave Chinner
2014-03-12  3:11   ` Steven Rostedt
2014-03-12  4:26     ` Dave Chinner
2014-03-12 10:07       ` Steven Rostedt
2014-03-12 15:57         ` Peter Zijlstra
2014-03-12 16:06           ` Linus Torvalds
2014-03-12 16:19           ` Steven Rostedt
2014-03-12 16:19             ` Steven Rostedt
2014-03-12 16:23             ` Peter Zijlstra
2014-03-12 16:23               ` Peter Zijlstra
2014-03-12  6:15   ` Peter Zijlstra
2014-03-12 23:48     ` Dave Chinner

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=20140310155543.644544511@infradead.org \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=arnd@arndb.de \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=riel@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --cc=waiman.long@hp.com \
    --cc=walken@google.com \
    --cc=x86@kernel.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).