All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Low <jason.low2@hp.com>
To: Davidlohr Bueso <dave@stgolabs.net>
Cc: 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>,
	Ming Lei <ming.lei@canonical.com>,
	jason.low2@hp.com
Subject: Re: softlockups in multi_cpu_stop
Date: Fri, 06 Mar 2015 17:58:43 -0800	[thread overview]
Message-ID: <1425693523.2475.319.camel@j-VirtualBox> (raw)
In-Reply-To: <1425680137.19505.63.camel@stgolabs.net>

On Fri, 2015-03-06 at 14:15 -0800, Davidlohr Bueso 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.

Actually, the rationale is that when the lock owner reschedules while
holding the lock, we'd want the spinners to stop spinning. The original
owner_running() check takes care of this since it returns false if
->on_cpu gets set to false and the sem->owner != NULL would be false
causing us to stop spinning . However, with the patch, when
owner_running returns false, we check sem->owner, which causes the
->on_cpu check to essentially get ignored.


  parent reply	other threads:[~2015-03-07  1:58 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-02  7:45 sched: softlockups in multi_cpu_stop Sasha Levin
     [not found] ` <CAMiJ5CVWvUhGK=MWYB_CTNs901p=jsT4i5gkWTaHih7qdQdkFQ@mail.gmail.com>
2015-03-04  5:44   ` Rafael David Tinoco
2015-03-06 11:27 ` Sasha Levin
2015-03-06 12:32   ` Ingo Molnar
2015-03-06 14:34     ` Rafael David Tinoco
2015-03-06 14:45       ` Sasha Levin
2015-03-06 15:46         ` Sasha Levin
2015-03-06 17:19     ` Davidlohr Bueso
2015-03-06 18:02       ` Sasha Levin
2015-03-06 21:59         ` Sasha Levin
2015-03-06 18:57       ` Jason Low
2015-03-06 19:05         ` Linus Torvalds
2015-03-06 19:20           ` Davidlohr Bueso
2015-03-06 19:32             ` Linus Torvalds
2015-03-06 19:45               ` Davidlohr Bueso
2015-03-06 19:55               ` Davidlohr Bueso
2015-03-06 20:00                 ` Davidlohr Bueso
2015-03-06 21:42                 ` Linus Torvalds
2015-03-06 19:29           ` Jason Low
2015-03-06 21:12             ` Jason Low
2015-03-06 21:24               ` Linus Torvalds
2015-03-07  1:53                 ` Jason Low
2015-03-06 22:15               ` Davidlohr Bueso
2015-03-07  1:55                 ` Ming Lei
2015-03-07  2:07                   ` Davidlohr Bueso
2015-03-07  2:10                     ` Ming Lei
2015-03-07  2:26                       ` Davidlohr Bueso
2015-03-07  2:29                         ` Davidlohr Bueso
2015-03-07  2:55                           ` Ming Lei
2015-03-07  3:10                             ` Davidlohr Bueso
2015-03-07  3:19                               ` Ming Lei
2015-03-07  3:41                                 ` Davidlohr Bueso
2015-03-07  2:56                       ` Jason Low
2015-03-07  3:08                         ` Ming Lei
2015-03-07  3:10                           ` Davidlohr Bueso
2015-03-07  3:17                           ` Jason Low
2015-03-07  3:39                             ` Ming Lei
2015-03-07  3:53                               ` Jason Low
2015-03-07  1:58                 ` Jason Low [this message]
2015-03-07  4:31               ` Jason Low
2015-03-07  4:44                 ` Davidlohr Bueso
2015-03-07  6:45                   ` Jason Low
2015-03-07  5:54                 ` Ming Lei
2015-03-07  6:57                   ` Jason Low

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1425693523.2475.319.camel@j-VirtualBox \
    --to=jason.low2@hp.com \
    --cc=dave@stgolabs.net \
    --cc=davej@codemonkey.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.lei@canonical.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=sasha.levin@oracle.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.