Re: [PATCH v2] locking/mutex: Prevent lock starvation when spinning is enabled

[Jason Low]

Hi Wanpeng,

On Wed, 2016-08-17 at 09:41 +0800, Wanpeng Li wrote:
> 2016-08-11 2:44 GMT+08:00 Jason Low <jason.low2@xxxxxxx>:
> > Imre reported an issue where threads are getting starved when trying
> > to acquire a mutex. Threads acquiring a mutex can get arbitrarily delayed
> > sleeping on a mutex because other threads can continually steal the lock
> > in the fastpath and/or through optimistic spinning.
> >
> > Waiman has developed patches that allow waiters to return to optimistic
> > spinning, thus reducing the probability that starvation occurs. However,
> > Imre still sees this starvation problem in the workloads when optimistic
> > spinning is disabled.
> >
> > This patch adds an additional boolean to the mutex that gets used in
> > the CONFIG_SMP && !CONFIG_MUTEX_SPIN_ON_OWNER cases. The flag signifies
> > whether or not other threads need to yield to a waiter and gets set
> > when a waiter spends too much time waiting for the mutex. The threshold
> > is currently set to 16 wakeups, and once the wakeup threshold is exceeded,
> > other threads must yield to the top waiter. The flag gets cleared
> > immediately after the top waiter acquires the mutex.
> 
> There is a subtle difference between this patch and Waiman's. Waiman's
> patch will boost any waiter-spinner which is woken up, however, this
> patch will boost the top waiter once the number of any waiter-spinners
> woken up reaches the threshold.

Correct, since when spinning is disabled, we still want to generally
allow other threads to steal the lock even if there are waiters in order
to keep performance good, and only yield the lock when a waiter is
getting 'starved'.

> We can't get any benefit if the
> resource holder which top waiter is waiting for still not release the
> resource.

If the resource holder does not release the resource, that sounds like
an issue with the lock holder.

Unless you're referring to how this doesn't provide immediate benefit to
the top waiter, in which case, I think that is okay since the goal of
the patch is to prevent starvation. We tried disabling 'lock stealing'
anytime there are waiters and that proved to reduce performance by quite
a bit in some workloads.