public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Nicholas Piggin <npiggin@gmail.com>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	Waiman Long <longman@redhat.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	"linux-kernel @ vger . kernel . org"
	<linux-kernel@vger.kernel.org>
Subject: [PATCH v2 11/12] locking/qspinlock: separate pv_wait_node from the non-paravirt path
Date: Wed, 13 Jul 2022 17:07:03 +1000	[thread overview]
Message-ID: <20220713070704.308394-12-npiggin@gmail.com> (raw)
In-Reply-To: <20220713070704.308394-1-npiggin@gmail.com>

pv_wait_node waits until node->locked is non-zero, no need for the
pv case to wait again by also executing the !pv code path.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 kernel/locking/qspinlock.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index 2ebb946a6b80..3255e7804842 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -506,15 +506,18 @@ static void pv_init_node(struct qnode *node)
  * pv_kick_node() is used to set _Q_SLOW_VAL and fill in hash table on its
  * behalf.
  */
-static void pv_wait_node(struct qnode *node, struct qnode *prev)
+static void pv_wait_node_acquire(struct qnode *node, struct qnode *prev)
 {
 	int loop;
 	bool wait_early;
 
 	for (;;) {
 		for (wait_early = false, loop = SPIN_THRESHOLD; loop; loop--) {
-			if (READ_ONCE(node->locked))
+			if (READ_ONCE(node->locked)) {
+				/* Provide the acquire ordering. */
+				smp_load_acquire(&node->locked);
 				return;
+			}
 			if (pv_wait_early(prev, loop)) {
 				wait_early = true;
 				break;
@@ -556,29 +559,23 @@ static void pv_wait_node(struct qnode *node, struct qnode *prev)
 		lockevent_cond_inc(pv_spurious_wakeup,
 				  !READ_ONCE(node->locked));
 	}
-
-	/*
-	 * By now our node->locked should be 1 and our caller will not actually
-	 * spin-wait for it. We do however rely on our caller to do a
-	 * load-acquire for us.
-	 */
 }
 
 /*
  * Called after setting next->locked = 1 when we're the lock owner.
  *
- * Instead of waking the waiters stuck in pv_wait_node() advance their state
- * such that they're waiting in pv_wait_head_or_lock(), this avoids a
+ * Instead of waking the waiters stuck in pv_wait_node_acquire() advance their
+ * state such that they're waiting in pv_wait_head_or_lock(), this avoids a
  * wake/sleep cycle.
  */
 static void pv_kick_node(struct qspinlock *lock, struct qnode *node)
 {
 	/*
 	 * If the vCPU is indeed halted, advance its state to match that of
-	 * pv_wait_node(). If OTOH this fails, the vCPU was running and will
-	 * observe its next->locked value and advance itself.
+	 * pv_wait_node_acquire(). If OTOH this fails, the vCPU was running and
+	 * will observe its next->locked value and advance itself.
 	 *
-	 * Matches with smp_store_mb() and cmpxchg() in pv_wait_node()
+	 * Matches with smp_store_mb() and cmpxchg() in pv_wait_node_acquire()
 	 *
 	 * The write to next->locked in arch_mcs_spin_unlock_contended()
 	 * must be ordered before the read of node->state in the cmpxchg()
@@ -765,8 +762,8 @@ EXPORT_SYMBOL(__pv_queued_spin_unlock);
 
 #else /* CONFIG_PARAVIRT_SPINLOCKS */
 static __always_inline void pv_init_node(struct qnode *node) { }
-static __always_inline void pv_wait_node(struct qnode *node,
-					 struct qnode *prev) { }
+static __always_inline void pv_wait_node_acquire(struct qnode *node,
+						 struct qnode *prev) { }
 static __always_inline void pv_kick_node(struct qspinlock *lock,
 					 struct qnode *node) { }
 static __always_inline u32  pv_wait_head_or_lock(struct qspinlock *lock,
@@ -864,10 +861,11 @@ static __always_inline void queued_spin_lock_mcs_queue(struct qspinlock *lock, b
 		/* Link @node into the waitqueue. */
 		WRITE_ONCE(prev->next, node);
 
-		if (paravirt)
-			pv_wait_node(node, prev);
 		/* Wait for mcs node lock to be released */
-		smp_cond_load_acquire(&node->locked, VAL);
+		if (paravirt)
+			pv_wait_node_acquire(node, prev);
+		else
+			smp_cond_load_acquire(&node->locked, VAL);
 
 		/*
 		 * While waiting for the MCS lock, the next pointer may have
-- 
2.35.1


  parent reply	other threads:[~2022-07-13  7:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-13  7:06 [PATCH v2 00/12] locking/qspinlock: simplify code generation Nicholas Piggin
2022-07-13  7:06 ` [PATCH v2 01/12] locking/qspinlock: remove pv_node abstraction Nicholas Piggin
2022-07-13  7:06 ` [PATCH v2 02/12] locking/qspinlock: inline mcs_spinlock functions into qspinlock Nicholas Piggin
2022-07-13  7:06 ` [PATCH v2 03/12] locking/qspinlock: split common mcs queueing code into its own function Nicholas Piggin
2022-07-13  7:06 ` [PATCH v2 04/12] locking/qspinlock: move pv lock word helpers into qspinlock.c Nicholas Piggin
2022-07-13  7:06 ` [PATCH v2 05/12] locking/qspinlock: be less clever with the preprocessor Nicholas Piggin
2022-07-13  7:06 ` [PATCH v2 06/12] locking/qspinlock: merge qspinlock_paravirt.h into qspinlock.c Nicholas Piggin
2022-07-14 14:16   ` kernel test robot
2022-07-29  3:49     ` Nicholas Piggin
2022-07-14 16:42   ` kernel test robot
2022-07-14 20:28   ` kernel test robot
2022-07-13  7:06 ` [PATCH v2 07/12] locking/qspinlock: remove arch qspinlock_paravirt.h includes Nicholas Piggin
2022-07-14 13:14   ` kernel test robot
2022-07-14 16:21   ` kernel test robot
2022-07-13  7:07 ` [PATCH v2 08/12] locking/qspinlock: stop renaming queued_spin_lock_slowpath to native_queued_spin_lock_slowpath Nicholas Piggin
2022-07-13  7:07 ` [PATCH v2 09/12] locking/qspinlock: rename __pv_init_lock_hash to pv_spinlocks_init Nicholas Piggin
2022-07-13  7:07 ` [PATCH v2 10/12] locking/qspinlock: paravirt use simple trylock in case idx overflows Nicholas Piggin
2022-07-13  7:07 ` Nicholas Piggin [this message]
2022-07-13  7:07 ` [PATCH v2 12/12] locking/qspinlock: simplify pv_wait_head_or_lock calling scheme Nicholas Piggin

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=20220713070704.308394-12-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=will@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