Re: [PATCH-tip 1/3] locking/rwsem: Check wait_list without lock if spinner present

[Waiman Long <longman@redhat.com>]

On 02/26/2017 01:49 PM, Davidlohr Bueso wrote:
> On Wed, 22 Feb 2017, Waiman Long wrote:
>
>> We can safely check the wait_list to see if waiters are present without
>> lock when there are spinners to fall back on in case we miss a waiter.
>> The advantage is that we can save a pair of spin_lock/unlock calls
>> when the wait_list is empty. This translates to a reduction in latency
>> and hence slightly better performance.
>
> This benefit is only seen in (rare) situations where there are only
> writers with short hold times, no? I don't really have any objection
> as I doubt the additional load will have any impact on the common case,
> but it would still be nice to have more data for other benchmarks where
> the lock is at least shared at times -- ie: a good thing to measure is
> also fault, mmap related benchmarks.

If a up_write() or up_read() coincides with a writer attempting to lock
(down_write). The unlocker may go into the wake_rwsem path even if on
one is on the wait queue. In this case, this patch can save an unneeded
spin_lock/unlock. This was what happened in the microbenchmark that I used.

The additional load shouldn't have any noticeable performance impact as
the wait_list need to be read sooner or later anyway. BTW, can you
suggest a good benchmark for testing fault, mmap related code paths?

>> +        /*
>> +         * Normally checking wait_list without wait_lock isn't safe
>> +         * as we may miss an incoming waiter. With spinners present,
>> +         * however, we have someone to fall back on in case that
>> +         * happens. This can save a pair of spin_lock/unlock calls
>> +         * when there is no waiter.
>> +         */
>
> I would drop the last part regarding saving the spin_lock, it should be
> evident from the code. 

I will do that.

Cheers,
Longman