Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760151AbZAHPYD (ORCPT ); Thu, 8 Jan 2009 10:24:03 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755181AbZAHPXv (ORCPT ); Thu, 8 Jan 2009 10:23:51 -0500 Received: from casper.infradead.org ([85.118.1.10]:35425 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751557AbZAHPXu (ORCPT ); Thu, 8 Jan 2009 10:23:50 -0500 Subject: Re: [PATCH -v7][RFC]: mutex: implement adaptive spinning From: Peter Zijlstra To: Steven Rostedt Cc: Ingo Molnar , Linus Torvalds , "Paul E. McKenney" , Gregory Haskins , Matthew Wilcox , Andi Kleen , Chris Mason , Andrew Morton , Linux Kernel Mailing List , linux-fsdevel , linux-btrfs , Thomas Gleixner , Nick Piggin , Peter Morreale , Sven Dietrich In-Reply-To: References: <1231347442.11687.344.camel@twins> <1231365115.11687.361.camel@twins> <1231366716.11687.377.camel@twins> <1231408718.11687.400.camel@twins> <20090108141808.GC11629@elte.hu> <1231426014.11687.456.camel@twins> Content-Type: text/plain Content-Transfer-Encoding: 7bit Date: Thu, 08 Jan 2009 16:23:40 +0100 Message-Id: <1231428220.11687.464.camel@twins> Mime-Version: 1.0 X-Mailer: Evolution 2.24.2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2668 Lines: 96 On Thu, 2009-01-08 at 10:09 -0500, Steven Rostedt wrote: > On Thu, 8 Jan 2009, Peter Zijlstra wrote: > > Index: linux-2.6/kernel/sched.c > > =================================================================== > > --- linux-2.6.orig/kernel/sched.c > > +++ linux-2.6/kernel/sched.c > > @@ -4672,6 +4672,72 @@ need_resched_nonpreemptible: > > } > > EXPORT_SYMBOL(schedule); > > > > +#ifdef CONFIG_SMP > > +/* > > + * Look out! "owner" is an entirely speculative pointer > > + * access and not reliable. > > + */ > > +int spin_on_owner(struct mutex *lock, struct thread_info *owner) > > +{ > > + unsigned int cpu; > > + struct rq *rq; > > + int ret = 1; > > + > > + if (unlikely(!sched_feat(OWNER_SPIN))) > > I would remove the "unlikely", if someone turns OWNER_SPIN off, then you > have the wrong decision being made. Choices by users should never be in a > "likely" or "unlikely" annotation. It's discrimination ;-) in the unlikely case we schedule(), that seems expensive enough to want to make the spin case ever so slightly faster. > > + return 0; > > + > > + preempt_disable(); > > +#ifdef CONFIG_DEBUG_PAGEALLOC > > + /* > > + * Need to access the cpu field knowing that > > + * DEBUG_PAGEALLOC could have unmapped it if > > + * the mutex owner just released it and exited. > > + */ > > + if (probe_kernel_address(&owner->cpu, cpu)) > > + goto out; > > +#else > > + cpu = owner->cpu; > > +#endif > > + > > + /* > > + * Even if the access succeeded (likely case), > > + * the cpu field may no longer be valid. > > + */ > > + if (cpu >= nr_cpumask_bits) > > + goto out; > > + > > + /* > > + * We need to validate that we can do a > > + * get_cpu() and that we have the percpu area. > > + */ > > + if (!cpu_online(cpu)) > > + goto out; > > Should we need to do a "get_cpu" or something? Couldn't the CPU disappear > between these two calls. Or does it do a stop-machine and the preempt > disable will protect us? Did you miss the preempt_disable() a bit up? > > + > > + rq = cpu_rq(cpu); > > + > > + for (;;) { > > + if (lock->owner != owner) > > + break; > > + > > + /* > > + * Is that owner really running on that cpu? > > + */ > > + if (task_thread_info(rq->curr) != owner) > > + break; > > + > > + if (need_resched()) { > > + ret = 0; > > + break; > > + } > > + > > + cpu_relax(); > > + } > > +out: > > + preempt_enable_no_resched(); > > + return ret; > > +} > > +#endif -- 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/