Skip to content

Commit e9a028b

Browse files
bibo-maointel-lab-lkp
authored andcommitted
locking/pvqspinlock: Use try_cmpxchg() in pv_unhash
We ported pv spinlock to old linux kernel on LoongArch platform, there is error with some stress tests. The error report is something like this for short: kernel BUG at kernel/locking/qspinlock_paravirt.h:261! Oops - BUG[#1]: CPU: 1 PID: 6613 Comm: pidof Not tainted 4.19.190+ torvalds#43 Hardware name: Loongson KVM, BIOS 0.0.0 02/06/2015 ra: 9000000000509cfc do_task_stat+0x29c/0xaf0 ERA: 9000000000291308 __pv_queued_spin_unlock_slowpath+0xf8/0x100 CRMD: 000000b0 (PLV0 -IE -DA +PG DACF=CC DACM=CC -WE) PRMD: 00000000 (PPLV0 -PIE -PWE) ... Call Trace: [<9000000000291308>] __pv_queued_spin_unlock_slowpath+0xf8/0x100 [<9000000000509cf8>] do_task_stat+0x298/0xaf0 [<9000000000502570>] proc_single_show+0x60/0xe0 The problem is that memory accessing is out of order on LoongArch platform, there is contension between pv_unhash() and pv_hash(). CPU0 pv_unhash: CPU1 pv_hash: for_each_hash_entry(he, offset, hash) { for_each_hash_entry(he, offset, hash) { if (READ_ONCE(he->lock) == lock) { struct qspinlock *old = NULL; node = READ_ONCE(he->node); WRITE_ONCE(he->lock, NULL); On LoongArch platform which is out of order, the execution order may be switched like this: > WRITE_ONCE(he->lock, NULL); if (try_cmpxchg(&he->lock, &old, lock)) { WRITE_ONCE(he->node, node); return &he->lock; CPU1 pv_hash() is executing and watch that lock is set with NULL. Write he->node with node of new lock. > node = READ_ONCE(he->node); READ_ONCE(he->node) on CPU0 will return node of new lock rather than itself. Here READ_ONCE/WRITE_ONCE is replaced with try_cmpxchg(). Signed-off-by: Bibo Mao <maobibo@loongson.cn>
1 parent 48f506a commit e9a028b

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

kernel/locking/qspinlock_paravirt.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,10 @@ static struct pv_node *pv_unhash(struct qspinlock *lock)
240240
struct pv_node *node;
241241

242242
for_each_hash_entry(he, offset, hash) {
243-
if (READ_ONCE(he->lock) == lock) {
243+
struct qspinlock *old = lock;
244+
245+
if (try_cmpxchg(&he->lock, &old, NULL)) {
244246
node = READ_ONCE(he->node);
245-
WRITE_ONCE(he->lock, NULL);
246247
return node;
247248
}
248249
}

0 commit comments

Comments
 (0)