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
