LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Ingo Molnar <mingo@elte.hu>
Cc: "H. Peter Anvin" <hpa@zytor.com>, Vivek Goyal <vgoyal@redhat.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	tglx@linutronix.de, mingo@redhat.com, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH], issue EOI to APIC prior to calling crash_kexec in die_nmi path
Date: Wed, 06 Feb 2008 17:31:11 -0700	[thread overview]
Message-ID: <m1r6fpd2uo.fsf@ebiederm.dsl.xmission.com> (raw)
In-Reply-To: <20080206233657.GB12393@elte.hu> (Ingo Molnar's message of "Thu, 7 Feb 2008 00:36:57 +0100")

Ingo Molnar <mingo@elte.hu> writes:

> * H. Peter Anvin <hpa@zytor.com> wrote:
>
>>> I am wondering if interrupts are disabled on crashing cpu or if 
>>> crashing cpu is inside die_nmi(), how would it stop/prevent delivery 
>>> of NMI IPI to other cpus.
>>
>> I don't see how it would.
>
> cross-CPU IPIs are a bit fragile on some PC platforms. So if the kexec 
> code relies on getting IPIs to all other CPUs, it might not be able to 
> do it reliably. There might be limitations on how many APIC irqs there 
> can be queued at a time, and if those slots are used up and the CPU is 
> not servicing irqs then stuff gets retried. This might even affect NMIs 
> sent via APIC messages - not sure about that.



The design was as follows:
- Doing anything in the crashing kernel is unreliable.
- We do not have the information to do anything useful in the recovery/target
  kernel.
- Having the other cpus stopped is very nice as it reduces the amount of
  weirdness happening.  We do not share the same text or data addresses
  so stopping the other cpus is not mandatory.  On some other architectures
  there are cpu tables that must live at a fixed address but this is not
  the case on x86.
- Having the location the other cpus were running at is potentially very
  interesting debugging information.

Therefore the intent of the code is to send an NMI to each other cpu.  With
a timeout of a second or so.  So that if the NMI do not get sent we continue
on.

There is certainly still room for improving the robustness by not shutting
down the ioapics and using less general infrastructure code on that path.
That said I would be a little surprised if that is what is biting us.

Looking at the patch the local_irq_enable() is totally bogus.  As soon
was we hit machine_crash_shutdown the first thing we do is disable irqs.

I'm wondering if someone was using the switch cpus on crash patch that was
floating around.  That would require the ipis to work.

I don't know if nmi_exit makes sense.  There are enough layers of abstraction
in that piece of code I can't quickly spot the part that is banging the hardware.

The location of nmi_exit in the patch is clearly wrong.  crash_kexec is a noop
if we don't have a crash kernel loaded (and if we are not the first cpu into it),
so if we don't execute the crash code something weird may happen.  Further the
code is just more maintainable if that kind of code lives in machine_crash_shutdown.



Eric

  parent reply	other threads:[~2008-02-07  0:48 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-06 19:25 Neil Horman
2008-02-06 19:40 ` Vivek Goyal
2008-02-06 20:12   ` Neil Horman
2008-02-06 20:21     ` H. Peter Anvin
2008-02-06 21:04       ` Neil Horman
2008-02-06 20:35     ` Vivek Goyal
2008-02-06 22:00 ` Ingo Molnar
2008-02-06 22:48   ` Vivek Goyal
2008-02-06 22:53     ` Ingo Molnar
2008-02-06 22:56     ` H. Peter Anvin
2008-02-06 23:36       ` Ingo Molnar
2008-02-06 23:50         ` Vivek Goyal
2008-02-07  0:31         ` Eric W. Biederman [this message]
2008-02-07  0:39           ` Ingo Molnar
2008-02-07  1:30             ` Eric W. Biederman
2008-02-07 12:17           ` Neil Horman
2008-02-07 12:24             ` Ingo Molnar
2008-02-07 20:37               ` Neil Horman
2008-02-08 16:14               ` Neil Horman
2008-02-08 16:45                 ` Vivek Goyal
2008-02-08 17:26                   ` Neil Horman
2008-02-12 21:08                   ` Neil Horman
2008-02-15 14:02                     ` Eric W. Biederman
2008-02-20 14:57                     ` Neil Horman
2008-02-08 16:54               ` Andi Kleen

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=m1r6fpd2uo.fsf@ebiederm.dsl.xmission.com \
    --to=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mingo@redhat.com \
    --cc=nhorman@tuxdriver.com \
    --cc=tglx@linutronix.de \
    --cc=vgoyal@redhat.com \
    --subject='Re: [PATCH], issue EOI to APIC prior to calling crash_kexec in die_nmi path' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).