Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753020AbbD3UZk (ORCPT ); Thu, 30 Apr 2015 16:25:40 -0400 Received: from g2t2354.austin.hp.com ([15.217.128.53]:56749 "EHLO g2t2354.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752516AbbD3UZg (ORCPT ); Thu, 30 Apr 2015 16:25:36 -0400 Message-ID: <55428FBE.6000103@hp.com> Date: Thu, 30 Apr 2015 16:25:34 -0400 From: Waiman Long User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130109 Thunderbird/10.0.12 MIME-Version: 1.0 To: Davidlohr Bueso CC: Peter Zijlstra , Ingo Molnar , linux-kernel@vger.kernel.org, Jason Low , Scott J Norton , Douglas Hatch Subject: Re: [PATCH v3] locking/rwsem: reduce spinlock contention in wakeup after up_read/up_write References: <1429898069-28907-1-git-send-email-Waiman.Long@hp.com> <1429907956.10273.65.camel@stgolabs.net> <553E9B25.4060802@hp.com> <1430245038.2004.21.camel@stgolabs.net> <554137EC.4030700@hp.com> <1430403123.2011.16.camel@stgolabs.net> In-Reply-To: <1430403123.2011.16.camel@stgolabs.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3111 Lines: 64 On 04/30/2015 10:12 AM, Davidlohr Bueso wrote: > On Wed, 2015-04-29 at 15:58 -0400, Waiman Long wrote: >> A write lock can also be acquired by a spinning writer in >> rwsem_try_write_lock_unqueued() where wait_lock isn't used. With >> multiple down_read's, it is possible that the first exiting reader wakes >> up a writer who acquires the write lock while the other readers are >> waiting for acquiring the wait_lock. > Except that readers that do the wakeup do not call __rwsem_do_wake() if > there is an active writer: > > /* If there are no active locks, wake the front queued process(es). > * > * If there are no writers and we are first in the queue, > * wake our own waiter to join the existing active readers ! > */ > if (count == RWSEM_WAITING_BIAS || > (count> RWSEM_WAITING_BIAS&& > adjustment != -RWSEM_ACTIVE_READ_BIAS)) > sem = __rwsem_do_wake(sem, RWSEM_WAKE_ANY); > > And for the reader part, your rwsem_has_active_writer() check, while > avoiding the counter atomic update, would break current semantics in > that we still do the next reader grant -- also note the unlikely() > predictor ;) And this is done with the counter, so by using the owner, > you would have a race between the cmpxchg and rwsem_set_owner(). Yes, > its a small window (specially after commit 7a215f89), but there > nonetheless and could cause even more bogus wakeups. Again, I simply do > not like mixing these two -- we get away with it with the optimistic > spinning because we just iterate again, but this is not the case. I think I know what you are talking about. However, I don't think it is really a race condition. In up_write(): rwsem_clear_owner(sem); __up_write(sem); The owner field is cleared before the lock holding writer decrement the count. So there is simply no way a reader will infer from the count that there is no writer in the rwsem while the owner field is still set unless it has just been stolen by another writer. In this case, __rwsem_do_wake() will have to back out from the reader grant anyway. It is possible that the second writer release the lock just before __rwsem_do_wake() is called, perhaps an interrupt happen. In this case, 2nd writer will be waiting for the wait_lock in rwsem_wake() to do the wakeup duty. So there should be no missed wakeup. If you are still worrying about this, I can put the owner check in rwsem_wake() function after taking the wait_lock instead. That should remove any concern that you have. > Ultimately, is this really an issue? Do you have numbers that could > justify such a change? I suspect all the benchmark results you posted in > the patch are from reducing the spinlock contention, not from this. This is true. This change has no statistically significant contribution (just a tiny bit, perhaps) for improving benchmark results. Cheers, Longman -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/