Re: Race unlocking-locking mutex

[Ángel González <keisial at gmail dot com>]

On 14/09/13 22:47, David Ahern wrote:
> > David problem should be solved if the unlock atomically assigned it to
> > the waken thread (instead of waiting for it to reacquire the mutex).
> > However, as it is a kernel decision, I think it would require a new
> > futex operation, which stored in uaddr2 the waken tids. Then
> > mutex->__data.__owner could be passed as uaddr2 and the mutex considered
> > also locked if owner != 0.
>
> Not necessarily proposing this for libc, but the product I work on 
> needs a solution for this problem that invokes some kind of fairness 
> -- and without adding too much complexity.
>
> One option that comes to mind is adding a new element in the mutex -- 
> a __prev_owner. On the unlock path set __prev_owner to thread id, 
> release lock, awaken a waiter. If it is an uncontended lock (no tasks 
> awakened or waiting), set __prev_owner to 0.
>
> On the lock path check if __prev_owner is equal to thread id and if so 
> take the slow path into the kernel.
>
> Any particular worrisome race conditions to be wary of?
>
> David
Hmm, yes, it seems doable.
I think it could even be implemented without __prev_owner.

This may be such an implementation (to replace the nptl/lowlevellock.h 
__generic_mutex_lock).

static inline void
__generic_mutex_lock (int *mutex)
{
  unsigned int v;

  /* Mutex available and no waiters, we got the mutex.  (this is the 
fastpath).  */
  if (!atomic_compare_and_exchange_bool_acq(mutex, 1 << 31, 0))
    return;

  atomic_increment (mutex);

  while (1)
    {
      if (!atomic_compare_and_exchange_bool_acq(mutex, 1 << 31, 1))
    {
      return;
    }

      /* We have to wait now. First make sure the futex value we are
     monitoring is locked or with waiters. */
      v = *mutex;
      assert(v != 0);
      if (__builtin_expect(v == 0, 0))
    continue;

      lll_futex_wait (mutex, v,
              // XYZ check mutex flag
              LLL_SHARED);
    }

      while (1)
        {
          if (atomic_bit_test_set (mutex, 31) == 0)
        {
          atomic_decrement (mutex);
          return;
        }

          /* We have to wait now. First make sure the futex value we are
         monitoring is truly negative (i.e. locked). */
          v = *mutex;
          if (v >= 0)
        continue;

          lll_futex_wait (mutex, v,
                  // XYZ check mutex flag
                  LLL_SHARED);
        }

}