Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1443200

Re: [PATCH] locking/pvqspinlock: restore/set vcpu_hashed state after failing adaptive locking spinning

From xinhui <xinhui.pan@linux.vnet.ibm.com>
Newsgroups linux.kernel
Subject Re: [PATCH] locking/pvqspinlock: restore/set vcpu_hashed state after failing adaptive locking spinning
Date 2016-07-14 10:30 +0200
Message-ID <rUKbV-7dQ-67@gated-at.bofh.it> (permalink)
References <rUJIR-6Ft-5@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw



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);
>

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread


Thread

[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

csiph-web