Re: [linux-safety] [PATCH 1/4] x86/mce: do not overwrite no_way_out if mce_end() fails

From: Borislav Petkov <bp@alien8.de>
To: Gabriele Paoloni <gabriele.paoloni@intel.com>
Cc: tony.luck@intel.com, tglx@linutronix.de, mingo@redhat.com,
	x86@kernel.org, hpa@zytor.com, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-safety@lists.elisa.tech
Subject: Re: [linux-safety] [PATCH 1/4] x86/mce: do not overwrite no_way_out if mce_end() fails
Date: Fri, 20 Nov 2020 18:07:37 +0100	[thread overview]
Message-ID: <20201120170737.GD712@zn.tnic> (raw)
In-Reply-To: <20201118151552.1412-2-gabriele.paoloni@intel.com>

On Wed, Nov 18, 2020 at 03:15:49PM +0000, Gabriele Paoloni wrote:
> Currently if mce_end() fails no_way_out is set equal to worst.
> worst is the worst severirty that was found in the MCA banks
		     ^^^^^^^^^

Please introduce a spellchecker into your patch creation workflow.

> associated to the current CPU; however at this point no_way_out
	     ^
	     with

> could be already set by mca_start() by looking at all severities

I think you mean "could have been already set" here

> of all CPUs that entered the MCE handler.
> if mce_end() fails we first check if no_way_out is already set and

Please use passive voice in your commit message: no "we" or "I", etc.

Also, pls start new sentences with a capital letter and end them with a
fullstop.

> if so we stick to it, otherwise we use the local worst value

So basically you're trying to say here that no_way_out might have been
already set and other CPUs could overwrite it and that should not
happen.

Is that what you mean?

> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kernel/cpu/mce/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index 4102b866e7c0..b990892c6766 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -1385,7 +1385,7 @@ noinstr void do_machine_check(struct pt_regs *regs)
>  	 */
>  	if (!lmce) {
>  		if (mce_end(order) < 0)
> -			no_way_out = worst >= MCE_PANIC_SEVERITY;
> +			no_way_out = no_way_out ? no_way_out : worst >= MCE_PANIC_SEVERITY;

I had to stare at this a bit to figure out what you're doing. So how
about simplifying this:

			if (!no_way_out)
				no_way_out = worst >= MCE_PANIC_SEVERITY;

?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#183): https://lists.elisa.tech/g/linux-safety/message/183
Mute This Topic: https://lists.elisa.tech/mt/78342501/5278000
Group Owner: linux-safety+owner@lists.elisa.tech
Unsubscribe: https://lists.elisa.tech/g/linux-safety/unsub [linux-safety@archiver.kernel.org]
-=-=-=-=-=-=-=-=-=-=-=-