pv qspinlocks jumps over a bunch of slowpath code directly to the
queueing part. Split the queueing code into its own function and call it
explicitly in each pv and !pv cases. This will help to untangle the two
cases with subsequent changes.
Signed-off-by: Nicholas Piggin <[email protected]>
---
kernel/locking/qspinlock.c | 208 +++++++++++++++++++------------------
1 file changed, 108 insertions(+), 100 deletions(-)
diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index 32f401e966ab..7360d643de29 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -294,112 +294,14 @@ static __always_inline u32 __pv_wait_head_or_lock(struct qspinlock *lock,
#endif /* _GEN_PV_LOCK_SLOWPATH */
-/**
- * queued_spin_lock_slowpath - acquire the queued spinlock
- * @lock: Pointer to queued spinlock structure
- * @val: Current value of the queued spinlock 32-bit word
- *
- * (queue tail, pending bit, lock value)
- *
- * fast : slow : unlock
- * : :
- * uncontended (0,0,0) -:--> (0,0,1) ------------------------------:--> (*,*,0)
- * : | ^--------.------. / :
- * : v \ \ | :
- * pending : (0,1,1) +--> (0,1,0) \ | :
- * : | ^--' | | :
- * : v | | :
- * uncontended : (n,x,y) +--> (n,0,0) --' | :
- * queue : | ^--' | :
- * : v | :
- * contended : (*,x,y) +--> (*,0,0) ---> (*,0,1) -' :
- * queue : ^--' :
- */
-void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
+static inline void queued_spin_lock_mcs_queue(struct qspinlock *lock)
{
struct qnode *prev, *next, *node;
- u32 old, tail;
+ u32 val, old, tail;
int idx;
BUILD_BUG_ON(CONFIG_NR_CPUS >= (1U << _Q_TAIL_CPU_BITS));
- if (pv_enabled())
- goto pv_queue;
-
- if (virt_spin_lock(lock))
- return;
-
- /*
- * 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) {
- int cnt = _Q_PENDING_LOOPS;
- val = atomic_cond_read_relaxed(&lock->val,
- (VAL != _Q_PENDING_VAL) || !cnt--);
- }
-
- /*
- * If we observe any contention; queue.
- */
- if (val & ~_Q_LOCKED_MASK)
- goto queue;
-
- /*
- * trylock || pending
- *
- * 0,0,* -> 0,1,* -> 0,0,1 pending, trylock
- */
- val = queued_fetch_set_pending_acquire(lock);
-
- /*
- * If we observe contention, there is a concurrent locker.
- *
- * Undo and queue; our setting of PENDING might have made the
- * n,0,0 -> 0,0,0 transition fail and it will now be waiting
- * on @next to become !NULL.
- */
- if (unlikely(val & ~_Q_LOCKED_MASK)) {
-
- /* Undo PENDING if we set it. */
- if (!(val & _Q_PENDING_MASK))
- clear_pending(lock);
-
- goto queue;
- }
-
- /*
- * We're pending, wait for the owner to go away.
- *
- * 0,1,1 -> 0,1,0
- *
- * this wait loop must be a load-acquire such that we match the
- * store-release that clears the locked bit and create lock
- * sequentiality; this is because not all
- * clear_pending_set_locked() implementations imply full
- * barriers.
- */
- if (val & _Q_LOCKED_MASK)
- atomic_cond_read_acquire(&lock->val, !(VAL & _Q_LOCKED_MASK));
-
- /*
- * take ownership and clear the pending bit.
- *
- * 0,1,0 -> 0,0,1
- */
- clear_pending_set_locked(lock);
- lockevent_inc(lock_pending);
- return;
-
- /*
- * End of pending bit optimistic spinning and beginning of MCS
- * queuing.
- */
-queue:
- lockevent_inc(lock_slowpath);
-pv_queue:
node = this_cpu_ptr(&qnodes[0]);
idx = node->count++;
tail = encode_tail(smp_processor_id(), idx);
@@ -567,6 +469,110 @@ void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
*/
__this_cpu_dec(qnodes[0].count);
}
+
+/**
+ * queued_spin_lock_slowpath - acquire the queued spinlock
+ * @lock: Pointer to queued spinlock structure
+ * @val: Current value of the queued spinlock 32-bit word
+ *
+ * (queue tail, pending bit, lock value)
+ *
+ * fast : slow : unlock
+ * : :
+ * uncontended (0,0,0) -:--> (0,0,1) ------------------------------:--> (*,*,0)
+ * : | ^--------.------. / :
+ * : v \ \ | :
+ * pending : (0,1,1) +--> (0,1,0) \ | :
+ * : | ^--' | | :
+ * : v | | :
+ * uncontended : (n,x,y) +--> (n,0,0) --' | :
+ * queue : | ^--' | :
+ * : v | :
+ * contended : (*,x,y) +--> (*,0,0) ---> (*,0,1) -' :
+ * queue : ^--' :
+ */
+void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
+{
+ if (pv_enabled()) {
+ queued_spin_lock_mcs_queue(lock);
+ return;
+ }
+
+ if (virt_spin_lock(lock))
+ return;
+
+ /*
+ * 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) {
+ int cnt = _Q_PENDING_LOOPS;
+ val = atomic_cond_read_relaxed(&lock->val,
+ (VAL != _Q_PENDING_VAL) || !cnt--);
+ }
+
+ /*
+ * If we observe any contention; queue.
+ */
+ if (val & ~_Q_LOCKED_MASK)
+ goto queue;
+
+ /*
+ * trylock || pending
+ *
+ * 0,0,* -> 0,1,* -> 0,0,1 pending, trylock
+ */
+ val = queued_fetch_set_pending_acquire(lock);
+
+ /*
+ * If we observe contention, there is a concurrent locker.
+ *
+ * Undo and queue; our setting of PENDING might have made the
+ * n,0,0 -> 0,0,0 transition fail and it will now be waiting
+ * on @next to become !NULL.
+ */
+ if (unlikely(val & ~_Q_LOCKED_MASK)) {
+
+ /* Undo PENDING if we set it. */
+ if (!(val & _Q_PENDING_MASK))
+ clear_pending(lock);
+
+ goto queue;
+ }
+
+ /*
+ * We're pending, wait for the owner to go away.
+ *
+ * 0,1,1 -> 0,1,0
+ *
+ * this wait loop must be a load-acquire such that we match the
+ * store-release that clears the locked bit and create lock
+ * sequentiality; this is because not all
+ * clear_pending_set_locked() implementations imply full
+ * barriers.
+ */
+ if (val & _Q_LOCKED_MASK)
+ atomic_cond_read_acquire(&lock->val, !(VAL & _Q_LOCKED_MASK));
+
+ /*
+ * take ownership and clear the pending bit.
+ *
+ * 0,1,0 -> 0,0,1
+ */
+ clear_pending_set_locked(lock);
+ lockevent_inc(lock_pending);
+ return;
+
+ /*
+ * End of pending bit optimistic spinning and beginning of MCS
+ * queuing.
+ */
+queue:
+ lockevent_inc(lock_slowpath);
+ queued_spin_lock_mcs_queue(lock);
+}
EXPORT_SYMBOL(queued_spin_lock_slowpath);
/*
@@ -583,6 +589,8 @@ EXPORT_SYMBOL(queued_spin_lock_slowpath);
#undef pv_kick_node
#undef pv_wait_head_or_lock
+#define queued_spin_lock_mcs_queue __pv_queued_spin_lock_mcs_queue
+
#undef queued_spin_lock_slowpath
#define queued_spin_lock_slowpath __pv_queued_spin_lock_slowpath
--
2.35.1
On Tue, Jul 05, 2022 at 12:38:10AM +1000, Nicholas Piggin wrote:
> +void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
> +{
> + if (pv_enabled()) {
> + queued_spin_lock_mcs_queue(lock);
> + return;
> + }
> +
> + if (virt_spin_lock(lock))
> + return;
> +
This reminds me; at the time I meant to make queued_spin_lock_slowpath()
a static_call() and redirect the function appropriately at boot time.
But that was before static_call() was merged and I never seem to have
gotten around to doing that afterwards...
Excerpts from Peter Zijlstra's message of July 6, 2022 3:01 am:
> On Tue, Jul 05, 2022 at 12:38:10AM +1000, Nicholas Piggin wrote:
>> +void queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
>> +{
>> + if (pv_enabled()) {
>> + queued_spin_lock_mcs_queue(lock);
>> + return;
>> + }
>> +
>> + if (virt_spin_lock(lock))
>> + return;
>> +
>
> This reminds me; at the time I meant to make queued_spin_lock_slowpath()
> a static_call() and redirect the function appropriately at boot time.
> But that was before static_call() was merged and I never seem to have
> gotten around to doing that afterwards...
Wouldn't hurt. OTOH hyper optimising the contended path is probably
almost not measurable. Optimising coherency in the contended path
absolutely, but straight line code less so. That said don't let me
stop you :)
Thanks,
Nick