From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752704AbbCGBz5 (ORCPT <rfc822;w@1wt.eu>);
	Fri, 6 Mar 2015 20:55:57 -0500
Received: from youngberry.canonical.com ([91.189.89.112]:49970 "EHLO
	youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751223AbbCGBz4 (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 6 Mar 2015 20:55:56 -0500
Date: Sat, 7 Mar 2015 09:55:26 +0800
From: Ming Lei <ming.lei@canonical.com>
To: Davidlohr Bueso <dave@stgolabs.net>
Cc: Jason Low <jason.low2@hp.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Ingo Molnar <mingo@kernel.org>, Sasha Levin <sasha.levin@oracle.com>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Dave Jones <davej@codemonkey.org.uk>
Subject: Re: softlockups in multi_cpu_stop
Message-ID: <20150307095526.5ffb1bf0@tom-ThinkPad-T410>
In-Reply-To: <1425680137.19505.63.camel@stgolabs.net>
References: <54F41516.6060608@oracle.com>
	<54F98F1F.3080107@oracle.com>
	<20150306123233.GA9972@gmail.com>
	<1425662342.19505.41.camel@stgolabs.net>
	<1425668223.2475.94.camel@j-VirtualBox>
	<CA+55aFwZufdV5Q7Wm+b2F8KurtgXsJ_eNe9b6_TOSUhuW_GfSg@mail.gmail.com>
	<1425670188.2475.113.camel@j-VirtualBox>
	<1425676346.2475.135.camel@j-VirtualBox>
	<1425680137.19505.63.camel@stgolabs.net>
X-Mailer: Claws Mail 3.8.0 (GTK+ 2.24.10; x86_64-pc-linux-gnu)
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, 06 Mar 2015 14:15:37 -0800
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Fri, 2015-03-06 at 13:12 -0800, Jason Low wrote:
> > In owner_running() there are 2 conditions that would make it return
> > false: if the owner changed or if the owner is not running. However,
> > that patch continues spinning if there is a "new owner" but it does not
> > take into account that we may want to stop spinning if the owner is not
> > running (due to getting rescheduled).
> 
> So you're rationale is that we're missing this need_resched:
> 
> 	while (owner_running(sem, owner)) {
> 		/* abort spinning when need_resched */
> 		if (need_resched()) {
> 			rcu_read_unlock();
> 			return false;
> 		}
> 	}
> 
> Because the owner_running() would return false, right? Yeah that makes
> sense, as missing a resched is a bug, as opposed to our heuristics being
> so painfully off.
> 
> Sasha, Ming (Cc'ed), does this address the issues you guys are seeing?

For the xfstest lockup, what matters is that the owner isn't running, since
the following simple change does fix the issue:

diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index 06e2214..5e08705 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -358,8 +358,9 @@ bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct *owner)
 	}
 	rcu_read_unlock();
 
-	if (READ_ONCE(sem->owner))
-		return true; /* new owner, continue spinning */
+	owner = READ_ONCE(sem->owner);
+	if (owner && owner->on_cpu)
+		return true;
 
 	/*
 	 * When the owner is not set, the lock could be free or


Thanks,
Ming Lei