public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Rusty Russell <rusty@rustcorp.com.au>,
	npiggin@suse.de, Thomas Gleixner <tglx@linutronix.de>,
	Arjan van de Ven <arjan@infradead.org>,
	jens.axboe@oracle.com
Subject: Re: [PATCH -v2] use per cpu data for single cpu ipi calls
Date: Thu, 29 Jan 2009 19:49:53 +0100	[thread overview]
Message-ID: <20090129184953.GA14810@elte.hu> (raw)
In-Reply-To: <alpine.LFD.2.00.0901290959540.3123@localhost.localdomain>


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> And are you really sure it cannot be called from within interrupts? I'm 
> finding a lot of callers of smp_call_function_single(), and while I 
> couldn't find any that look like interrupts, I also couldn't find any 
> indication that it never happens.

smp_call_function_single() must not be called from irqs-off code - and 
that implicitly includes most IRQ handlers as well. (most of which run 
with hardirqs off)

We check for that via:

        /* Can deadlock when called with interrupts disabled */
        WARN_ON(irqs_disabled());

[ although it should probably be extended to '&& !in_interrupt()' as well, 
  to cover the subset of IRQ handlers running with irqs enabled - which is 
  small but nonzero. It would also cover softirqs. ]

There are 3 reasons why sending IPIs from an irqs off critical section is 
a bad idea currently:

1) the implementation is deadlockable as you noted. That is self-inflicted
   stupidity and fixable.

2) the _concept_ of two CPUs sending IPIs to each other at once and
   expecting them to be handled and then waiting for them is a deadlock.
   We could allow async IPIs from irqs-off sections, but isnt that a too
   narrow special-case?

3) the third reason is nasty: the act of sending an IPI on x86 hardware
   needs to happen with interrupts enabled. In the lowlevel IPI sending
   code we first poll the hardware whether it's ready to accept new IPIs:
   via apic_wait_icr_idle().

   There've been incidents in the past where the ICR-ready (Interrupt 
   Command Register - ready) bit would deadlock the box in certain 
   situations (usually with wackier APIC implementations and if there's 
   too many IPIs 'in flight' and the only way to make progress is to allow 
   IPIs to happen locally - i.e. to have irqs on), if the IPI was sent 
   from an irqs-off section.

So i'm not sure we could relax things here without expecting too much of 
the hardware.

What i like least about kernel/smp.c is the kmalloc(GFP_ATOMIC) - and we 
had to add that in a late -rc for an essential bugfix.

IMO that kmalloc() has "layering violation" written all over it. It turns 
the pure act of 'sending messages' into a conceptually heavy and wide 
facility. Is the buffering of async messages really such a big deal in 
practice? Does anyone have any numbers?

Btw., there _is_ a relatively atomic, safe and instantaneous way of 
sending IPIs on x86: sending NMIs. But that has its own host of issues 
both on the hardware and on the software side ...

	Ingo

  parent reply	other threads:[~2009-01-29 18:50 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-28 16:38 Buggy IPI and MTRR code on low memory Steven Rostedt
2009-01-28 16:41 ` Steven Rostedt
2009-01-28 16:46 ` Peter Zijlstra
2009-01-28 16:56   ` Steven Rostedt
2009-01-28 17:00     ` Peter Zijlstra
2009-01-28 17:24   ` Steven Rostedt
2009-01-28 18:20     ` Peter Zijlstra
2009-01-28 18:52       ` Steven Rostedt
2009-01-28 18:22     ` Arjan van de Ven
2009-01-28 18:34       ` Steven Rostedt
2009-01-28 21:12 ` Andrew Morton
2009-01-28 21:13   ` Andrew Morton
2009-01-28 21:23     ` Steven Rostedt
2009-01-28 22:07       ` Andrew Morton
2009-01-28 22:47         ` Steven Rostedt
2009-01-28 23:20           ` Andrew Morton
2009-01-28 23:50             ` Steven Rostedt
2009-01-28 23:25 ` Rusty Russell
2009-01-28 23:41   ` Steven Rostedt
2009-01-29  0:52   ` [PATCH] use per cpu data for single cpu ipi calls Steven Rostedt
2009-01-29  1:30     ` Andrew Morton
2009-01-29  1:56       ` Steven Rostedt
2009-01-29  8:49       ` Peter Zijlstra
2009-01-29 11:13         ` Ingo Molnar
2009-01-29 11:41           ` Peter Zijlstra
2009-01-29 13:42             ` Ingo Molnar
2009-01-29 14:07             ` Steven Rostedt
2009-01-29 15:08         ` [PATCH -v2] " Steven Rostedt
2009-01-29 15:33           ` Peter Zijlstra
2009-01-29 16:17             ` Ingo Molnar
2009-01-29 17:21           ` Linus Torvalds
2009-01-29 17:44             ` Steven Rostedt
2009-01-29 17:50               ` Steven Rostedt
2009-01-29 18:08               ` Linus Torvalds
2009-01-29 18:11                 ` Steven Rostedt
2009-01-29 18:23                 ` Peter Zijlstra
2009-01-29 18:31                   ` Steven Rostedt
2009-01-29 18:39                   ` Linus Torvalds
2009-01-29 18:44                     ` Peter Zijlstra
2009-01-30 11:23                       ` Jens Axboe
2009-01-30 12:32                         ` [PATCH -v3] " Peter Zijlstra
2009-01-30 12:38                           ` Jens Axboe
2009-01-30 12:48                             ` Peter Zijlstra
2009-01-30 12:55                               ` Jens Axboe
2009-01-30 12:56                                 ` Jens Axboe
2009-01-30 13:00                                   ` Peter Zijlstra
2009-01-30 13:02                           ` [PATCH -v4] " Peter Zijlstra
2009-01-30 14:51                             ` Ingo Molnar
2009-01-30 16:04                           ` [PATCH -v3] " Linus Torvalds
2009-01-30 16:16                             ` Peter Zijlstra
2009-01-31  8:44                               ` Jens Axboe
2009-01-29 18:49                 ` Ingo Molnar [this message]
2009-01-30  1:55                 ` [PATCH -v2] " Rusty Russell
2009-01-29 17:47             ` Peter Zijlstra
2009-01-29 17:55               ` Peter Zijlstra
2009-01-29 18:08                 ` Steven Rostedt
2009-01-30  1:11           ` Rusty Russell

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=20090129184953.GA14810@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@infradead.org \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=npiggin@suse.de \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=rusty@rustcorp.com.au \
    --cc=tglx@linutronix.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox