Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1443146 > unrolled thread
| Started by | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| First post | 2016-07-14 10:00 +0200 |
| Last post | 2016-07-14 10:30 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] locking/pvqspinlock: restore/set vcpu_hashed state after failing adaptive locking spinning Wanpeng Li <kernellwp@gmail.com> - 2016-07-14 10:00 +0200
Re: [PATCH] locking/pvqspinlock: restore/set vcpu_hashed state after failing adaptive locking spinning xinhui <xinhui.pan@linux.vnet.ibm.com> - 2016-07-14 10:30 +0200
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2016-07-14 10:00 +0200 |
| Subject | [PATCH] locking/pvqspinlock: restore/set vcpu_hashed state after failing adaptive locking spinning |
| Message-ID | <rUJIR-6Ft-5@gated-at.bofh.it> |
From: Wanpeng Li <wanpeng.li@hotmail.com>
When the lock holder vCPU is racing with the queue head:
CPU 0 (lock holder) CPU1 (queue head)
=================== =================
spin_lock(); spin_lock();
pv_kick_node(); pv_wait_head_or_lock();
if (!lp) {
lp = pv_hash(lock, pn);
xchg(&l->locked, _Q_SLOW_VAL);
}
WRITE_ONCE(pn->state, vcpu_halted);
cmpxchg(&pn->state,
vcpu_halted, vcpu_hashed);
WRITE_ONCE(l->locked, _Q_SLOW_VAL);
(void)pv_hash(lock, pn);
In this case, lock holder inserts the pv_node of queue head into the
hash table and set _Q_SLOW_VAL unnecessary. This patch avoids it by
restoring/setting vcpu_halted state after failing adaptive locking
spinning.
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Waiman Long <Waiman.Long@hpe.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
kernel/locking/qspinlock_paravirt.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
index 21ede57..ac7d20b 100644
--- a/kernel/locking/qspinlock_paravirt.h
+++ b/kernel/locking/qspinlock_paravirt.h
@@ -450,7 +450,7 @@ pv_wait_head_or_lock(struct qspinlock *lock, struct mcs_spinlock *node)
goto gotlock;
}
}
- WRITE_ONCE(pn->state, vcpu_halted);
+ WRITE_ONCE(pn->state, vcpu_hashed);
qstat_inc(qstat_pv_wait_head, true);
qstat_inc(qstat_pv_wait_again, waitcnt);
pv_wait(&l->locked, _Q_SLOW_VAL);
--
1.9.1
[toc] | [next] | [standalone]
| From | xinhui <xinhui.pan@linux.vnet.ibm.com> |
|---|---|
| Date | 2016-07-14 10:30 +0200 |
| Subject | Re: [PATCH] locking/pvqspinlock: restore/set vcpu_hashed state after failing adaptive locking spinning |
| Message-ID | <rUKbV-7dQ-67@gated-at.bofh.it> |
| In reply to | #1443146 |
On 2016年07月14日 15:53, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
>
> When the lock holder vCPU is racing with the queue head:
>
> CPU 0 (lock holder) CPU1 (queue head)
> =================== =================
> spin_lock(); spin_lock();
> pv_kick_node(); pv_wait_head_or_lock();
> if (!lp) {
> lp = pv_hash(lock, pn);
> xchg(&l->locked, _Q_SLOW_VAL);
> }
> WRITE_ONCE(pn->state, vcpu_halted);
> cmpxchg(&pn->state,
> vcpu_halted, vcpu_hashed);
> WRITE_ONCE(l->locked, _Q_SLOW_VAL);
> (void)pv_hash(lock, pn);
>
> In this case, lock holder inserts the pv_node of queue head into the
> hash table and set _Q_SLOW_VAL unnecessary. This patch avoids it by
> restoring/setting vcpu_halted state after failing adaptive locking
> spinning.
>
good catching. this is hash entry leak.
After apply Waiman's pathset "locking/pvqspinlock: Fix missed PV wakeup & support PPC", this is not the big problem.
But we can still save another cmpxchg.
So
Reviewed-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Waiman Long <Waiman.Long@hpe.com>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> ---
> kernel/locking/qspinlock_paravirt.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/locking/qspinlock_paravirt.h b/kernel/locking/qspinlock_paravirt.h
> index 21ede57..ac7d20b 100644
> --- a/kernel/locking/qspinlock_paravirt.h
> +++ b/kernel/locking/qspinlock_paravirt.h
> @@ -450,7 +450,7 @@ pv_wait_head_or_lock(struct qspinlock *lock, struct mcs_spinlock *node)
> goto gotlock;
> }
> }
> - WRITE_ONCE(pn->state, vcpu_halted);
> + WRITE_ONCE(pn->state, vcpu_hashed);
> qstat_inc(qstat_pv_wait_head, true);
> qstat_inc(qstat_pv_wait_again, waitcnt);
> pv_wait(&l->locked, _Q_SLOW_VAL);
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web