Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755975AbbDIR46 (ORCPT ); Thu, 9 Apr 2015 13:56:58 -0400 Received: from e33.co.us.ibm.com ([32.97.110.151]:50158 "EHLO e33.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755724AbbDIR45 (ORCPT ); Thu, 9 Apr 2015 13:56:57 -0400 Date: Thu, 9 Apr 2015 10:56:52 -0700 From: "Paul E. McKenney" To: Linus Torvalds Cc: Ingo Molnar , Jason Low , Peter Zijlstra , Davidlohr Bueso , Tim Chen , Aswin Chandramouleeswaran , LKML Subject: Re: [PATCH 2/2] locking/rwsem: Use a return variable in rwsem_spin_on_owner() Message-ID: <20150409175652.GI6464@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <1428521960-5268-1-git-send-email-jason.low2@hp.com> <1428521960-5268-3-git-send-email-jason.low2@hp.com> <20150409053725.GB13871@gmail.com> <1428561611.3506.78.camel@j-VirtualBox> <20150409075311.GA4645@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15040917-0009-0000-0000-000009FC4D41 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2837 Lines: 66 On Thu, Apr 09, 2015 at 09:47:36AM -0700, Linus Torvalds wrote: > On Thu, Apr 9, 2015 at 12:53 AM, Ingo Molnar wrote: > > > > The point is to generally unify the 'out' paths - i.e. to merge it > > with the rcu_read_unlock() as well, so that we have really simple > > gotos and only a single exit path. > > Maybe just have the rcu read-locking be done in the *caller* (possibly > through using just a helper wrapper function that does nothing but the > locking), so that you can just do a simple "return false" in the > function itself. > > That said, it worries me a bit that we do that spinning while holding > the RCU read lock in the first place. Yes, we stop spinning if > "need_resched()" is set, but what effect - if any - does all of this > have on RCU latency? If somebody is waiting for a RCU grace period, > I'm not seeing that setting need-resched... > > At least with CONFIG_PREEMPT_RCU, the read-unlock is *not* just doing > a preempt-disable, so it's not necessarily just about need_resched(). > It does all the magic with 'rcu_read_unlock_special.s' too.. > > Adding Paul. From a RCU locking standpoint, the thing is basically > (not the real code, edited down): > > rcu_read_lock(); > while (sem->owner == owner) { > if (!owner->on_cpu || need_resched()) > break; > cpu_relax_lowlatency(); > } > rcu_read_unlock(); > > so we busy-loop while holding the RCU read lock while > > sem->owner == owner && owner->on_cpu && !need_resched() > > is true. That is usually not very long, but we've already had > watchdogs go off when we get this wrong, so.. > > Paul, comments? Are there particular latency concerns wrt > CONFIG_PREEMPT_RCU here? Or am I just being silly? If this was a pure spinlock, then the effects of spinning would overwhelm any problems from extended grace periods. But this is a sleeplock. Of course, we stay in the loop only as long as the lock holder is actually running. But given that this is a sleeplock, I am worried that some lock holders might run for long time periods. After all, that is one of the traditional uses for a sleeplock. :-/ If the RCU read-side critical section lasts a few hundred milliseconds, no problem. If it lasts for more than 500 milliseconds, I would start getting concerned. And if such long-term spins are likely, I cannot resist asking if this should be instead using SRCU. If you have your own srcu_struct, you get to delay your own SRCU grace periods as long as you want. ;-) Thanx, Paul -- 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/