Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761721AbZAOArB (ORCPT ); Wed, 14 Jan 2009 19:47:01 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1759126AbZAOAqu (ORCPT ); Wed, 14 Jan 2009 19:46:50 -0500 Received: from mx2.suse.de ([195.135.220.15]:33313 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758889AbZAOAqs (ORCPT ); Wed, 14 Jan 2009 19:46:48 -0500 Date: Thu, 15 Jan 2009 01:46:45 +0100 From: Nick Piggin To: Peter Zijlstra Cc: Linus Torvalds , Ingo Molnar , "Paul E. McKenney" , Gregory Haskins , Matthew Wilcox , Andi Kleen , Chris Mason , Andrew Morton , Linux Kernel Mailing List , linux-fsdevel , linux-btrfs , Thomas Gleixner , Peter Morreale , Sven Dietrich , Dmitry Adamushko , Johannes Weiner Subject: Re: [PATCH -v11][RFC] mutex: implement adaptive spinning Message-ID: <20090115004645.GB32044@wotan.suse.de> References: <1231774622.4371.96.camel@laptop> <1231859742.442.128.camel@twins> <1231863710.7141.3.camel@twins> <1231864854.7141.8.camel@twins> <1231867314.7141.16.camel@twins> <1231952436.14825.28.camel@laptop> <20090114171800.GA18621@wotan.suse.de> <1231953757.14825.33.camel@laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1231953757.14825.33.camel@laptop> User-Agent: Mutt/1.5.9i Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2377 Lines: 61 On Wed, Jan 14, 2009 at 06:22:36PM +0100, Peter Zijlstra wrote: > On Wed, 2009-01-14 at 18:18 +0100, Nick Piggin wrote: > > > > @@ -173,21 +237,21 @@ __mutex_lock_common(struct mutex *lock, > > > spin_unlock_mutex(&lock->wait_lock, flags); > > > > > > debug_mutex_free_waiter(&waiter); > > > + preempt_enable(); > > > return -EINTR; > > > } > > > __set_task_state(task, state); > > > > > > /* didnt get the lock, go to sleep: */ > > > spin_unlock_mutex(&lock->wait_lock, flags); > > > - schedule(); > > > + __schedule(); > > > > Why does this need to do a preempt-disabled schedule? After we schedule > > away, the next task can do arbitrary things or reschedule itself, so if > > we have not anticipated such a condition here, then I can't see what > > __schedule protects. At least a comment is in order? > > From: > http://programming.kicks-ass.net/kernel-patches/mutex-adaptive-spin/mutex-preempt.patch > > Subject: mutex: preemption fixes > From: Peter Zijlstra > Date: Wed Jan 14 15:36:26 CET 2009 > > The problem is that dropping the spinlock right before schedule is a voluntary > preemption point and can cause a schedule, right after which we schedule again. > > Fix this inefficiency by keeping preemption disabled until we schedule, do this > by explicitly disabling preemption and providing a schedule() variant that > assumes preemption is already disabled. > > Signed-off-by: Peter Zijlstra > > > Pity to add the call overhead to schedule just for this case. > > Good point, seeing any way around that? Hmm, well this is rather a slow path, I would say. I'd prefer not to modify schedule in this way (if we just get scheduled back on after being switched away, the subsequent call to schedule is going to be cache hot and not do too much work). preempt_enable_noresched maybe if you really care, would close up the window even more. But is it really worthwhile? We'd want to see numbers (when in doubt, keep it simpler). > > BTW. __schedule shouldn't need to be asmlinkage? > > TBH I've no clue, probably not, Ingo? -- 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/