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 S1756772AbbCGG5M (ORCPT <rfc822;w@1wt.eu>);
	Sat, 7 Mar 2015 01:57:12 -0500
Received: from g9t5009.houston.hp.com ([15.240.92.67]:45653 "EHLO
	g9t5009.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753605AbbCGG5K (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Sat, 7 Mar 2015 01:57:10 -0500
Message-ID: <1425711422.2475.380.camel@j-VirtualBox>
Subject: Re: softlockups in multi_cpu_stop
From: Jason Low <jason.low2@hp.com>
To: Ming Lei <ming.lei@canonical.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        Davidlohr Bueso <dave@stgolabs.net>, Ingo Molnar <mingo@kernel.org>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Paul McKenney <paulmck@linux.vnet.ibm.com>,
        Sasha Levin <sasha.levin@oracle.com>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Dave Jones <davej@codemonkey.org.uk>, jason.low2@hp.com
Date: Fri, 06 Mar 2015 22:57:02 -0800
In-Reply-To: <CACVXFVMDkEQMg-rj5cKipe1Z6fJQpH6LKimf02JBGsDkVqk3Gg@mail.gmail.com>
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>
	 <1425702688.2475.363.camel@j-VirtualBox>
	 <CACVXFVMDkEQMg-rj5cKipe1Z6fJQpH6LKimf02JBGsDkVqk3Gg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.2.3-0ubuntu6 
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, 2015-03-07 at 13:54 +0800, Ming Lei wrote:
> On Sat, Mar 7, 2015 at 12:31 PM, Jason Low <jason.low2@hp.com> wrote:
> > On Fri, 2015-03-06 at 13:12 -0800, Jason Low wrote:
> > Cc: Ming Lei <ming.lei@canonical.com>
> > Cc: Davidlohr Bueso <dave@stgolabs.net>
> > Signed-off-by: Jason Low <jason.low2@hp.com>
> 
> Reported-and-tested-by: Ming Lei <ming.lei@canonical.com>

Thanks!

> >  static noinline
> >  bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct *owner)
> >  {
> >         long count;
> >
> >         rcu_read_lock();
> > -       while (owner_running(sem, owner)) {
> > -               /* abort spinning when need_resched */
> > -               if (need_resched()) {
> > +       while (sem->owner == owner) {
> > +               /*
> > +                * Ensure we emit the owner->on_cpu, dereference _after_
> > +                * checking sem->owner still matches owner, if that fails,
> > +                * owner might point to free()d memory, if it still matches,
> > +                * the rcu_read_lock() ensures the memory stays valid.
> > +                */
> > +               barrier();
> > +
> > +               /* abort spinning when need_resched or owner is not running */
> > +               if (!owner->on_cpu || need_resched()) {
> 
> BTW, could the need_resched() be handled in loop of
> rwsem_optimistic_spin() directly? Then code may get
> simplified a bit.

We still need the need_resched() check here, since if the thread needs
to reschedule, it should immediately stop spinning for the lock.
Otherwise, it could potentially spin for a long time before it checks
for it needs to reschedule.