Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760426AbZAHP2W (ORCPT ); Thu, 8 Jan 2009 10:28:22 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755164AbZAHP2K (ORCPT ); Thu, 8 Jan 2009 10:28:10 -0500 Received: from hrndva-omtalb.mail.rr.com ([71.74.56.123]:51392 "EHLO hrndva-omtalb.mail.rr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752000AbZAHP2I (ORCPT ); Thu, 8 Jan 2009 10:28:08 -0500 Date: Thu, 8 Jan 2009 10:28:05 -0500 (EST) From: Steven Rostedt X-X-Sender: rostedt@gandalf.stny.rr.com To: Peter Zijlstra 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 Subject: Re: [PATCH -v7][RFC]: mutex: implement adaptive spinning In-Reply-To: <1231428220.11687.464.camel@twins> Message-ID: 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> <1231428220.11687.464.camel@twins> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3188 Lines: 112 On Thu, 8 Jan 2009, Peter Zijlstra wrote: > 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. OK, that makes sense, but I would comment that. Otherwise, it just looks like another misuse of the unlikely annotation. > > > > + 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? No, let me rephrase it better. Does the preempt_disable protect against another CPU from going off line? Does taking a CPU off line do a stop_machine? -- Steve > > > > + > > > + 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/