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


Groups > linux.kernel > #1588710

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

From Waiman Long <longman@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH-tip 1/3] locking/rwsem: Check wait_list without lock if spinner present
Date 2017-02-27 16:10 +0100
Message-ID <tfv62-7iD-3@gated-at.bofh.it> (permalink)
References <tdJwt-5DY-5@gated-at.bofh.it> <tdJwt-5DY-3@gated-at.bofh.it> <tfcmJ-2z3-7@gated-at.bofh.it>
Organization Red Hat

Show all headers | View raw


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

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


Thread

[PATCH-tip 1/3] locking/rwsem: Check wait_list without lock if spinner present Waiman Long <longman@redhat.com> - 2017-02-22 19:10 +0100
  Re: [PATCH-tip 1/3] locking/rwsem: Check wait_list without lock if  spinner present Davidlohr Bueso <dave@stgolabs.net> - 2017-02-26 20:10 +0100
    Re: [PATCH-tip 1/3] locking/rwsem: Check wait_list without lock if  spinner present Waiman Long <longman@redhat.com> - 2017-02-27 16:10 +0100

csiph-web