Re: [RFC PATCH 1/3] locking/mutex: Try to acquire mutex only if it is unlocked

["Long, Wai Man" <waiman.long@hp.com>]

On 6/9/2014 1:38 PM, Jason Low wrote:
> On Wed, 2014-06-04 at 13:58 -0700, Davidlohr Bueso wrote:
>> On Wed, 2014-06-04 at 13:57 -0700, Davidlohr Bueso wrote:
>>> In addition, how about the following helpers instead:
>>> - mutex_is_unlocked() : count > 0
>>> - mutex_has_waiters() : count < 0, or list_empty(->wait_list)
>>                                        ^ err, that's !list_empty()
> Between checking for (count < 0) or checking for !list_empty(wait_list)
> for waiters:
>
> Now that I think about it, I would expect a mutex_has_waiters() function
> to return !list_empty(wait_list) as that really tells whether or not
> there are waiters. For example, in highly contended cases, there can
> still be waiters on the mutex if count is 1.
>
> Likewise, in places where we currently use "MUTEX_SHOW_NO_WAITER", we
> need to check for (count < 0) to ensure lock->count is a negative value
> before the thread sleeps on the mutex.
>
> One option would be to still remove MUTEX_SHOW_NO_WAITER(), directly use
> atomic_read() in place of the macro, and just comment on why we have an
> extra atomic_read() that may "appear redundant". Another option could be
> to provide a function that checks for "potential waiters" on the mutex.
>
> Any thoughts?
>

For the first MUTEX_SHOW_NO_WAITER() call site, you can replace it with 
a check for (count > 0). The second call site within the for loop, 
however, is a bit more tricky. It has to serve 2 purposes:

1. Opportunistically get the lock
2. Set the count value to -1 to indicate someone is waiting on the lock, 
that is why an xchg() operation has to be done even if its value is 0.

I do agree that the naming isn't that good. Maybe it can be changed to 
something like

static inline int mutex_value_has_waiters(mutex *lock)    { return 
lock->count < 0; }

-Longman