x86/mce: Fix initialization error warning
PATCH 1/1 x86/mce: Fix initialization error warning
When booting kernel with mce=off a loud warning from the mce code is displayed. This causes confusion for end users.
Add a check to see if MCE is available before outputting the warning message.
--- arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/x86/kernel/cpu/mcheck/mce.c +2 -1
@@ -2662,7 +2662,8 @@ static __init int mcheck_init_device(void)free_cpumask_var (mce_device_initialized); err_out: -pr_err ("Unable to init device /dev/mcelog (rc: %d)\n" , err); +if (mce_available (&boot_cpu_data)) +pr_err ("Unable to init device /dev/mcelog (rc: %d)\n" , err);return err; }
Signature · 1 line
1.7.9.3
Borislav Petkov
maintainerQuoted 2 lines · Prarit Bhargava
When booting kernel with mce=off a loud warning from the mce code is displayed. This causes confusion for end users.
Is this what you call a loud warning?
[ 1.885648] mce: Unable to init device /dev/mcelog (rc: -5)
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.Quoted 7 lines · Borislav Petkov
Quoted 2 lines · Prarit Bhargava
When booting kernel with mce=off a loud warning from the mce code is displayed. This causes confusion for end users.
Is this what you call a loud warning?
[ 1.885648] mce: Unable to init device /dev/mcelog (rc: -5)
Yes, it was loud enough to generate a bug report from a user.
FWIW, I just found this from last year calling the same message "scary". It doesn't look like there was a follow up though.
https://lkml.org/lkml/2016/2/9/354
P.
Quoted 1 line
Borislav Petkov
maintainerQuoted 1 line · Prarit Bhargava
Yes, it was loud enough to generate a bug report from a user.
Yeah, because all users are sane and we should do whatever they want - no questions asked. Especially those who boot with "mce=off".
Did you actually ask that user why she/he is even booting with "mce=off"?
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.Quoted 8 lines · Borislav Petkov
Quoted 1 line · Prarit Bhargava
Yes, it was loud enough to generate a bug report from a user.
Yeah, because all users are sane and we should do whatever they want - no questions asked. Especially those who boot with "mce=off".
Did you actually ask that user why she/he is even booting with "mce=off"?
Yes, mce=off is the default for kdump:
KDUMP_COMMANDLINE_APPEND="irqpoll nr_cpus=1 reset_devices cgroup_disable=memory mce=off numa=off udev.children-max=2 panic=10 rootflags=nofail acpi_no_memhotplug transparent_hugepage=never"
There is a race condition between NMI completing on a CPU and the MCE synchronization timing out that results in a kernel panic on the kdump kernel, and a loss of the dump image. There have been a few attempts to fix it over the years. It seems as simple as setting a flag in native_machine_crash_shutdown() and querying it in do_machine_check() to avoid mce & nmi race.
P.
Quoted 1 line
Borislav Petkov
maintainerQuoted 1 line · Prarit Bhargava
Yes, mce=off is the default for kdump:
So fix kdump to disable MCA properly instead of sending me brown paper bags.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.Quoted 5 lines · Borislav Petkov
Quoted 1 line · Prarit Bhargava
Yes, mce=off is the default for kdump:
So fix kdump to disable MCA properly instead of sending me brown paper bags.
Sure, I'll take a shot at it.
Thanks,
P.
Quoted 1 line
Quoted 5 lines · Borislav Petkov
Quoted 1 line · Prarit Bhargava
Yes, mce=off is the default for kdump:
So fix kdump to disable MCA properly instead of sending me brown paper bags.
That's one thing, but OTOH we really can avoid the KERN_ERR level print out for the case where mce is not available (for whatever reasons: HW / Virt / command line ....).
The simple 2 liner I posted earlier in this thread does it nicely.
Thanks,
tglx
Borislav Petkov
maintainerQuoted 5 lines · Thomas Gleixner
That's one thing, but OTOH we really can avoid the
KERN_ERRlevel print out for the case where mce is not available (for whatever reasons: HW / Virt / command line ....).The simple 2 liner I posted earlier in this thread does it nicely.
Well, I seem to remember at the time of this:
9c15a24b038f ("x86/mce: Improve mcheck_init_device() error handling")
that I requested to have that error message there so that we *know* when that function fails instead of trying to decipher "Hey, mcelog doesn't start here" and wonder why.
But we can kill it just as well, especially as there's patch deprecating /dev/mcelog floating around. We can reorg error messages in that function later, if deemed necessary.
So sure.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.Quoted 11 lines · Borislav Petkov
Quoted 5 lines · Thomas Gleixner
That's one thing, but OTOH we really can avoid the
KERN_ERRlevel print out for the case where mce is not available (for whatever reasons: HW / Virt / command line ....).The simple 2 liner I posted earlier in this thread does it nicely.
Well, I seem to remember at the time of this:
9c15a24b038f("x86/mce: Improvemcheck_init_device()error handling")
Quoted 7 lines
that I requested to have that error message there so that we *know* when that function fails instead of trying to decipher "Hey, mcelog doesn't start here" and wonder why.
But we can kill it just as well, especially as there's patch deprecating
/dev/mcelogfloating around. We can reorg error messages in that function later, if deemed necessary.
Your decision.
Quoted 5 lines · Prarit Bhargava
When booting kernel with mce=off a loud warning from the mce code is displayed. This causes confusion for end users.
Add a check to see if MCE is available before outputting the warning message.
Sigh.
Quoted 17 lines
Signed-off-by Prarit Bhargava <[email protected]>Cc Tony Luck <[email protected]>Cc Borislav Petkov <[email protected]>Cc Thomas Gleixner <[email protected]>Cc Ingo Molnar <[email protected]>Cc "H. Peter Anvin" <[email protected]>--- arch/x86/kernel/cpu/mcheck/mce.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/x86/kernel/cpu/mcheck/mce.c +0
-0
arch/x86/kernel/cpu/mcheck/mce.c static __init int mcheck_init_device(void) · line 2666
Quoted 6 lines
arch/x86/kernel/cpu/mcheck/mce.c +2
-1@@ -2662,7 +2662,8 @@ static __init int mcheck_init_device(void)free_cpumask_var (mce_device_initialized); err_out: -pr_err ("Unable to init device /dev/mcelog (rc: %d)\n" , err); +if (mce_available (&boot_cpu_data)) +pr_err ("Unable to init device /dev/mcelog (rc: %d)\n" , err);
This is really crap. Why would you check the same condition twice? Just because it's so much more complicated than doing the obvious:
arch/x86/kernel/cpu/mcheck/mce.c +2 -4
@@ -2615,10 +2615,8 @@ static __init int mcheck_init_device(voienum cpuhp_state hp_online;int err; -if (!mce_available (&boot_cpu_data)) { - err = -EIO ; - goto err_out; - } +if (!mce_available (&boot_cpu_data)) +return -EIO ;if (!zalloc_cpumask_var (&mce_device_initialized,GFP_KERNEL )) { err = -ENOMEM ;