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

Groups > linux.kernel > #1444144

Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem

Show all headers | View raw

On Fri, Jul 15, 2016 at 05:39:46PM +0800, Pan Xinhui wrote:
> >I'm thinking you're trying to say this:
> >
> >
> >CPU0			CPU1			CPU2
> >
> >__pv_queued_spin_unlock_slowpath()
> >  ...
> >  smp_store_release(&l->locked, 0);
> >			__pv_queued_spin_lock_slowpath()
> >			  ...
> >			  pv_queued_spin_steal_lock()
> >			    cmpxchg(&l->locked, 0, _Q_LOCKED_VAL) == 0
> >
> >
> >						pv_wait_head_or_lock()
> >
> >  pv_kick(node->cpu);  ---------------------->	  pv_wait(&l->locked, _Q_SLOW_VAL);
> >
> >			__pv_queued_spin_unlock()
> >			  cmpxchg(&l->locked, _Q_LOCKED_VAL, 0) == _Q_LOCKED_VAL
> >
> >						  for () {
> >						    trylock_clear_pending();
> >						    cpu_relax();
> >						  }
> >
> >						  pv_wait(&l->locked, _Q_SLOW_VAL);
> >
> >
> >Which is indeed 'bad', but not fatal, note that the later pv_wait() will
> >not in fact go wait, since l->locked will _not_ be _Q_SLOW_VAL.

> 
> the problem is that "this later pv_wait will do nothing as l->locked
> is not _Q_SLOW_VAL", So it is not paravirt friendly then. we will go
> into the trylock loop again and again until the lock is unlocked.

Agreed, which is 'bad'. But the patch spoke about a missing wakeup,
which is worse, as that would completely inhibit progress.

> So if we are kicked by the unlock_slowpath, and the lock is stealed by
> someone else,  we need hash its node again and set l->locked to
> _Q_SLOW_VAL, then enter pv_wait.

Right, let me go think about this a bit.

> but I am worried about lock stealing. could the node in the queue
> starve for a long time? I notice the latency of pv_wait on an
> over-commited guest can be bigger than 300us. I have not seen such
> starving case, but I think it is possible to happen.

I share that worry, which is why we limit the steal attempt to one.
But yes, theoretically its possible to starve things AFAICT.

We've not come up with sensible way to completely avoid starvation.

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

Thread

Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem Peter Zijlstra <peterz@infradead.org> - 2016-07-15 10:50 +0200
  Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem Pan Xinhui <xinhui@linux.vnet.ibm.com> - 2016-07-15 11:40 +0200
    Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem Peter Zijlstra <peterz@infradead.org> - 2016-07-15 12:10 +0200
      Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem Peter Zijlstra <peterz@infradead.org> - 2016-07-15 18:40 +0200
        Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem Boqun Feng <boqun.feng@gmail.com> - 2016-07-16 03:20 +0200
          Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem Wanpeng Li <kernellwp@gmail.com> - 2016-07-18 01:30 +0200
        Re: [PATCH v2 2/5] locking/pvqspinlock: Fix missed PV wakeup problem xinhui <xinhui.pan@linux.vnet.ibm.com> - 2016-07-21 08:50 +0200

csiph-web