Re: [PATCH v8] x86: mce: kexec: switch MCE handler for kexec/kdump

[Ingo Molnar <mingo@kernel.org>]

* Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:

> On Thu, Apr 09, 2015 at 10:00:30AM +0200, Ingo Molnar wrote:
> > 
> > * Borislav Petkov <bp@alien8.de> wrote:
> > 
> > > Btw, Ingo had some reservations about this. Ingo?
> > 
> > Yeah, so my concerns are the following:
> > 
> > > kexec disables (or "shoots down") all CPUs other than the crashing 
> > > CPU before entering the 2nd kernel. However, MCA is still enabled so 
> > > if an MCE happens and broadcasts to the CPUs after the main thread 
> > > starts the 2nd kernel (which might not initialize its MCE handler 
> > > yet, or might decide not to enable it) the MCE handler runs only on 
> > > the other CPUs (not on the main thread) leading to kernel panic 
> > > during MCE synchronization. The user-visible effect of this bug is a 
> > > kdump failure.
> > 
> > So the thing is, when we boot up the second kernel there will be a 
> > window where the old handler isn't valid (because the new kernel has 
> > its own pagetables, etc.) and the new handler is not installed yet.
> > 
> > If an MCE hits that window, it's bad luck. (unless the bootup sequence 
> > is rearchitected significantly to allow cross-kernel inheritance of 
> > MCE handlers.)
> > 
> > So I think we can ignore _that_ race.
> > 
> > The more significant question is: what happens when an MCE arrives 
> > whiel the kdump is proceeding - as kdumps can take a long time to 
> > finish when there's a lot of RAM.
> 
> Without this patch, MCE makes idling CPUs unpreferably wake up and 
> needlessly run MCE handler, which disturbs memory so does harm on 
> the kdump. This patch improves not only the transition phase, but 
> also that window.

The way the kdump code stops CPUs already 'disturbs' the state of 
those CPUs.

> > But ... since the 'shootdown' is analogous to a CPU hotplug 
> > CPU-down sequence, I suppose that the existing MCE code should 
> > already properly handle the case where an MCE arrives on a 
> > (supposedly) dead CPU, right?
> 
> Currently not, so Tony mentioned some idea about it (although not 
> included in this patch.)
> 
> > In that case installing a separate MCE handler looks like the 
> > wrong thing.
> 
> One difference bewteen kdump and CPU offline is whether we need handle
> MCEs then or not. In CPU offline situation, running CPUs have to continue
> their normal operations, so it's imporatant to handle MCE (i.e. log and/or
> take recovery action), so I think that should be done in our main MCE
> handler, do_machine_check().

I disagree: if offline CPUs are still active and can produce MCEs then 
they should be reported regardless of whether they were shot down by 
the CPU hotplug code or by kdump.

> But that's not the case in kdump situation (logging or recovering is
> not possible/necessary any more.) So it seems make sense to me to
> separate the handler.

I disagree: for example logging to the screen is still possible and 
should be done if there's an uncorrectable error.

So I agree that MCE policy should be made non-fatal during kdump, but 
I disagree that it needs a separate handler: it should be part of the 
regular MCE handling routines to handle kdump gracefully.

Thanks,

	Ingo