The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] locking/qspinlock: use xchg with _mb in slowpath for arm64
@ 2025-09-16  3:39 pengyu
  2025-09-16 13:27 ` Waiman Long
  2025-09-16 14:10 ` Peter Zijlstra
  0 siblings, 2 replies; 8+ messages in thread
From: pengyu @ 2025-09-16  3:39 UTC (permalink / raw)
  To: peterz, mingo, will, boqun.feng; +Cc: longman, linux-kernel, Yu Peng

From: Yu Peng <pengyu@kylinos.cn>

A hardlock detected on arm64: rq->lock was released, but a CPU
blocked at mcs_node->locked and timed out.

We found xchg_tail and atomic_try_cmpxchg_relaxed used _relaxed
versions without memory barriers. Suspected insufficient coherence
guarantees on some arm64 microarchitectures, potentially leading to
the following issues occurred:

CPU0:                                           CPU1:
// Set tail to CPU0
old = xchg_tail(lock, tail);

//CPU0 read tail is itself
if ((val & _Q_TAIL_MASK) == tail)
                                                // CPU1 exchanges the tail
                                                old = xchg_tail(lock, tail)
//assuming CPU0 not see tail change
atomic_try_cmpxchg_relaxed(
	  &lock->val, &val, _Q_LOCKED_VAL)
//released without notifying CPU1
goto release;
                                                //hardlock detected
                                                arch_mcs_spin_lock_contended(
                                                      &node->locked)

Therefore, xchg_tail and atomic_try_cmpxchg using _mb to replace _relaxed.

Signed-off-by: pengyu <pengyu@kylinos.cn>
---
 kernel/locking/qspinlock.c | 4 ++++
 kernel/locking/qspinlock.h | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/kernel/locking/qspinlock.c b/kernel/locking/qspinlock.c
index af8d122bb649..5e0c489d0d81 100644
--- a/kernel/locking/qspinlock.c
+++ b/kernel/locking/qspinlock.c
@@ -350,7 +350,11 @@ void __lockfunc queued_spin_lock_slowpath(struct qspinlock *lock, u32 val)
 	 *       PENDING will make the uncontended transition fail.
 	 */
 	if ((val & _Q_TAIL_MASK) == tail) {
+#if defined(CONFIG_ARM64)
+		if (atomic_try_cmpxchg(&lock->val, &val, _Q_LOCKED_VAL))
+#else
 		if (atomic_try_cmpxchg_relaxed(&lock->val, &val, _Q_LOCKED_VAL))
+#endif
 			goto release; /* No contention */
 	}

diff --git a/kernel/locking/qspinlock.h b/kernel/locking/qspinlock.h
index d69958a844f7..c89a6877b96b 100644
--- a/kernel/locking/qspinlock.h
+++ b/kernel/locking/qspinlock.h
@@ -113,12 +113,17 @@ static __always_inline void clear_pending_set_locked(struct qspinlock *lock)
  */
 static __always_inline u32 xchg_tail(struct qspinlock *lock, u32 tail)
 {
+#if defined(CONFIG_ARM64)
+	return (u32)xchg(&lock->tail,
+				 tail >> _Q_TAIL_OFFSET) << _Q_TAIL_OFFSET;
+#else
 	/*
 	 * We can use relaxed semantics since the caller ensures that the
 	 * MCS node is properly initialized before updating the tail.
 	 */
 	return (u32)xchg_relaxed(&lock->tail,
 				 tail >> _Q_TAIL_OFFSET) << _Q_TAIL_OFFSET;
+#endif
 }

 #else /* _Q_PENDING_BITS == 8 */

2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-09-19 10:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-16  3:39 [PATCH] locking/qspinlock: use xchg with _mb in slowpath for arm64 pengyu
2025-09-16 13:27 ` Waiman Long
2025-09-16 14:10 ` Peter Zijlstra
2025-09-16 16:00   ` Will Deacon
2025-09-17 10:51     ` pengyu
2025-09-17 11:37       ` Will Deacon
2025-09-19 10:22         ` pengyu
2025-09-16 16:58   ` Waiman Long

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox