From patchwork Fri Apr 16 21:26:24 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Zijlstra X-Patchwork-Id: 50355 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from bilbo.ozlabs.org (localhost [127.0.0.1]) by ozlabs.org (Postfix) with ESMTP id 7C49EB7F71 for ; Sat, 17 Apr 2010 07:26:50 +1000 (EST) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6272CB7CF5 for ; Sat, 17 Apr 2010 07:26:41 +1000 (EST) Received: from e35131.upc-e.chello.nl ([213.93.35.131] helo=dyad.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.69 #1 (Red Hat Linux)) id 1O2t3b-0008EA-Qu for linuxppc-dev@lists.ozlabs.org; Fri, 16 Apr 2010 21:26:36 +0000 Received: by dyad.programming.kicks-ass.net (Postfix, from userid 65534) id C3EB6F3E6; Fri, 16 Apr 2010 23:27:54 +0200 (CEST) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on dyad X-Spam-Level: X-Spam-Status: No, score=-1.4 required=5.0 tests=ALL_TRUSTED autolearn=failed version=3.2.5 Received: from [IPv6:::1] (dyad [192.168.0.60]) by dyad.programming.kicks-ass.net (Postfix) with ESMTP id 0EC87F7E5; Fri, 16 Apr 2010 23:27:43 +0200 (CEST) Subject: Re: Possible bug with mutex adaptative spinning From: Peter Zijlstra To: Benjamin Herrenschmidt In-Reply-To: <1271212509.13059.135.camel@pasglop> References: <1271212509.13059.135.camel@pasglop> Date: Fri, 16 Apr 2010 23:26:24 +0200 Message-ID: <1271453184.1674.483.camel@laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Cc: linuxppc-dev , "linux-kernel@vger.kernel.org" X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org Errors-To: linuxppc-dev-bounces+patchwork-incoming=ozlabs.org@lists.ozlabs.org On Wed, 2010-04-14 at 12:35 +1000, Benjamin Herrenschmidt wrote: > [PATCH] mutex: Don't spin when the owner CPU is offline or other weird cases > > The current code might spin forever if the CPU owning the mutex has been > offlined, and the last CPU in the system is trying to acquire it, since > mutex_spin_on_owner() will always return 1, telling the caller to spin > until the mutex has been released. > > This patch changes mutex_spin_on_owner() to return 0 (don't spin) in any > case where we aren't sure about the owner struct validity or CPU number, > and if the said CPU is offline. There is no point going back & > re-evaluate spinning in corner cases like that, let's just go to sleep. > > Signed-off-by: Benjamin Herrenschmidt Right, patch looks good, comments are a tad misleading. I've queued the below patch --- Subject: mutex: Don't spin when the owner CPU is offline or other weird cases From: Benjamin Herrenschmidt Date: Fri Apr 16 23:20:00 CEST 2010 Due to recent load-balancer changes that delay the task migration to the next wakeup, the adaptive mutex spinning ends up in a live lock when the owner's CPU gets offlined because the cpu_online() check lives before the owner running check. This patch changes mutex_spin_on_owner() to return 0 (don't spin) in any case where we aren't sure about the owner struct validity or CPU number, and if the said CPU is offline. There is no point going back & re-evaluate spinning in corner cases like that, let's just go to sleep. Signed-off-by: Benjamin Herrenschmidt Signed-off-by: Peter Zijlstra LKML-Reference: <1271212509.13059.135.camel@pasglop> --- kernel/sched.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) Index: linux-2.6/kernel/sched.c =================================================================== --- linux-2.6.orig/kernel/sched.c +++ linux-2.6/kernel/sched.c @@ -3647,7 +3647,7 @@ int mutex_spin_on_owner(struct mutex *lo * the mutex owner just released it and exited. */ if (probe_kernel_address(&owner->cpu, cpu)) - goto out; + return 0; #else cpu = owner->cpu; #endif @@ -3657,14 +3657,14 @@ int mutex_spin_on_owner(struct mutex *lo * the cpu field may no longer be valid. */ if (cpu >= nr_cpumask_bits) - goto out; + return 0; /* * We need to validate that we can do a * get_cpu() and that we have the percpu area. */ if (!cpu_online(cpu)) - goto out; + return 0; rq = cpu_rq(cpu); @@ -3683,7 +3683,7 @@ int mutex_spin_on_owner(struct mutex *lo cpu_relax(); } -out: + return 1; } #endif