Messages in this thread |  | | Date | Thu, 14 Jul 2016 16:20:56 +0800 | From | xinhui <> | Subject | Re: [PATCH] locking/pvqspinlock: restore/set vcpu_hashed state after failing adaptive locking spinning |
| |
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); >
|  |