public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Peter Zijlstra <peterz@infradead.org>
Cc: David Miller <davem@davemloft.net>,
	torvalds@linux-foundation.org, linux-kernel@vger.kernel.org,
	akpm@linux-foundation.org, viro@zeniv.linux.org.uk,
	alan@lxorguk.ukuu.org.uk, Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [git pull] scheduler/misc fixes
Date: Fri, 25 Apr 2008 12:51:49 +0200	[thread overview]
Message-ID: <20080425105149.GA11227@elte.hu> (raw)
In-Reply-To: <1209118778.7115.417.camel@twins>


* Peter Zijlstra <peterz@infradead.org> wrote:

> OK, so David came up with the idea that it might be the reschedule IPI 
> (smp_receive_signal_client) that did the wakeup resulting in the 
> following scenario:
> 
>  <idle > 60s >
>  <resched-IPI> -> nohz_restart() -> restart timer
>                                  -> schedule()
> 
>  <run stuff>
>  <timer> -> softlockup_tick() -> BUG!
>  
> doing irq_enter/exit() for smp_receive_signal_client() did indeed fix
> the whole issue. (x86 also has this bug - its just darn hard to generate
> 60s+ nohz periods due to the shitty clocks/timers)
> 
> So per b) any nohz wake needs to be done with an interrupt _and_ all
> such interrupts must pass through irq_enter().
> 
> As far as I can tell this is not nessecarily true (or desired from a 
> performance POV) for all platforms, imagine the core2 monitor/mwait 
> idle that wakes up because of a memory write. This doesn't require an 
> interrupt at all to wake up.
> 
> So, are we going to require all waking interrupts (IPIs and regular) 
> to do the irq_enter/exit() dance and add the perhaps unneeded overhead 
> to these paths and require the non-interrupt driven wake-ups like 
> monitor/mwait to do the touch_softlockup_watchdog() themselves?
> 
> Or,
> 
> Is Ingo's initial patch to make nohz_restart() also touch the 
> softlockup watchdog the best fix (now that we understand what 
> happens)?

thanks Peter for the detailed analysis.

I think we should not require all IRQ contexts to do irq_enter/exit - 
not because of the overhead (these are mainly really small costs), but 
mostly because as this example has shown, nothing besides this rather 
elusive softlockup false-positive enforces their correctness. So we'll 
always lag behind significantly and wont ever achieve a truly correct 
solution.

a much stronger approach would be to: require all exit-idle paths to 
touch the softlockup watchdog (we are evidently not locked up there), 
and require the timer IRQ to touch it as well if it hits an idle 
context. We restart the scheduler tick in the exit-idle path anyway, 
otherwise the scheduler breaks visibly - so that's the right place to 
put the touch_softlockup_watchdog() call.

I.e. i'd pretty much concur with you that the best solution is the very 
first patch - which is also in the pull to Linus - we just now 
understand exactly why ;-) David, do you agree?

	Ingo

  reply	other threads:[~2008-04-25 10:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-24 22:55 [git pull] scheduler/misc fixes Ingo Molnar
2008-04-25  3:46 ` David Miller
2008-04-25  7:48   ` Peter Zijlstra
2008-04-25  7:57     ` David Miller
2008-04-25  8:13       ` Peter Zijlstra
2008-04-25  8:24       ` Peter Zijlstra
2008-04-25  8:30         ` David Miller
2008-04-25 10:19     ` Peter Zijlstra
2008-04-25 10:51       ` Ingo Molnar [this message]
2008-04-25 20:07         ` David Miller
2008-04-27 18:05       ` Thomas Gleixner
2008-04-25  8:04   ` Ingo Molnar
2008-04-25  8:07     ` David Miller

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=20080425105149.GA11227@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox