RE: [PATCH v2 0/5] mm/hwpoison: Fix regressions in memory failure handling

["Luck, Tony" <tony.luck@intel.com>]

> > We could, but I don't like it much. By taking the page offline from the relatively
> > kind environment of a regular interrupt, we often avoid taking a machine check
> > (which is an unfriendly environment for software).
>
> Right.
>
> > We could make the action in uc_decode_notifier() configurable. Default=off
> > but with a command line option to enable for systems that are stuck with
> > broadcast machine checks.
>
> So we can figure that out during boot - no need for yet another cmdline
> option.

Yup. I think the boot time test might be something like:

	// Enable UCNA offline for systems with broadcast machine check
	if (!(AMD || LMCE))
		mce_register_decode_chain(&mce_uc_nb);
>
> It still doesn't fix the race and I'd like to fix that instead, in the optimal
> case.
>
> But looking at Shuai's patch, I guess fixing the reporting is fine too - we
> need to fix the commit message to explain why this thing even happens.
>
> I.e., basically what you wrote and Shuai could use that explanation to write
> a commit message explaining what the situation is along with the background so
> that when we go back to this later, we will actually know what is going on.

Agreed. Shaui needs to harvest this thread to fill out the details in the commit
messages.

>
> But looking at
>
>   046545a661af ("mm/hwpoison: fix error page recovered but reported "not recovered"")
>
> That thing was trying to fix the same reporting fail. Why didn't it do that?
>
> Ooooh, now I see what the issue is. He doesn't want to kill the process which
> gets the wrong SIGBUS. Maybe the commit title should've said that:
>
>   mm/hwpoison: Do not send SIGBUS to processes with recovered clean pages
>
> or so.
>
> But how/why is that ok?
>
> Are we confident that
>
> +        * ret = 0 when poison page is a clean page and it's dropped, no
> +        * SIGBUS is needed.
>
> can *always* and *only* happen when there's a CMCI *and* a #MC race and the
> CMCI has won the race?

There are probably other races. Two CPUs both take local #MC on the same page
(maybe not all that rare in threaded processes ... or even with some hot code in 
a shared library).

> Can memory poison return 0 there too, for another reason and we end up *not
> killing* a process which we should have?
>
> Hmmm.

Hmmm indeed. Needs some thought. Though failing to kill a process likely means
it retries the access and comes right back to try again (without the race this time).

>
> > On Intel that would mean not registering the notifier at all. What about AMD?
> > Do you have similar races for MCE_DEFERRED_SEVERITY errors?
>
> Probably. Lemme ask around.
>
> > [1] Some OEMs still do not enable LMCE in their BIOS.
>
> Oh, ofc. Gotta love BIOS. They'll get the message when LMCE becomes obsolete,
> trust me.
>
> Are we force-enabling LMCE in this case when booting?

Linux tries to enable if LMCE is supported, but BIOS has veto power.
See the bit in lmce_supported() that checks MSR_IA32_FEAT_CTL

-Tony