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: [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
WARNING: multiple messages have this Message-ID (diff)
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]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2020-11-20 17:08 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-18 15:15 [PATCH 0/4] x86/MCE: some minor fixes Gabriele Paoloni
2020-11-18 15:15 ` [linux-safety] " Paoloni, Gabriele
2020-11-18 15:15 ` [PATCH 1/4] x86/mce: do not overwrite no_way_out if mce_end() fails Gabriele Paoloni
2020-11-18 15:15 ` [linux-safety] " Paoloni, Gabriele
2020-11-20 17:07 ` Borislav Petkov [this message]
2020-11-20 17:07 ` Borislav Petkov
2020-11-20 17:31 ` Paoloni, Gabriele
2020-11-20 17:31 ` [linux-safety] " Paoloni, Gabriele
2020-11-20 17:33 ` Borislav Petkov
2020-11-20 17:33 ` [linux-safety] " Borislav Petkov
2020-11-23 14:35 ` Borislav Petkov
2020-11-23 14:35 ` [linux-safety] " Borislav Petkov
2020-11-20 17:32 ` Borislav Petkov
2020-11-20 17:32 ` [linux-safety] " Borislav Petkov
2020-11-20 17:35 ` Paoloni, Gabriele
2020-11-20 17:35 ` [linux-safety] " Paoloni, Gabriele
2020-11-18 15:15 ` [PATCH 2/4] x86/mce: move the mce_panic() call and kill_it assignments at the right places Gabriele Paoloni
2020-11-18 15:15 ` [linux-safety] " Paoloni, Gabriele
2020-11-23 14:27 ` Borislav Petkov
2020-11-23 14:27 ` [linux-safety] " Borislav Petkov
2020-11-23 17:06 ` Paoloni, Gabriele
2020-11-23 17:06 ` [linux-safety] " Paoloni, Gabriele
2020-11-23 17:19 ` Borislav Petkov
2020-11-23 17:19 ` [linux-safety] " Borislav Petkov
2020-11-23 17:40 ` Paoloni, Gabriele
2020-11-23 17:40 ` [linux-safety] " Paoloni, Gabriele
2020-11-23 18:07 ` Borislav Petkov
2020-11-23 18:07 ` [linux-safety] " Borislav Petkov
2020-11-18 15:15 ` [PATCH 3/4] x86/mce: for LMCE panic only if mca_cfg.tolerant < 3 Gabriele Paoloni
2020-11-18 15:15 ` [linux-safety] " Paoloni, Gabriele
2020-11-18 15:15 ` [PATCH 4/4] x86/mce: remove redundant call to irq_work_queue() Gabriele Paoloni
2020-11-18 15:15 ` [linux-safety] " Paoloni, Gabriele
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=20201120170737.GD712@zn.tnic \
--to=bp@alien8.de \
--cc=gabriele.paoloni@intel.com \
--cc=hpa@zytor.com \
--cc=linux-edac@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-safety@lists.elisa.tech \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=x86@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.