Skip to contentSkip to conversation

x86/mce: Don't unregister CPU hotplug notifier in error path

PATCH New Boris Ostrovsky 2 participants 3 messages Jun 22, 2014 – Jun 24, 2014 x86

lore
Share & tools
lore

Boris Ostrovsky

author Tested-by
#0 lore raw
Quoted 9 lines · ----- [email protected] wrote:
Quoted 1 line · Boris Ostrovsky

I'll try it later but this doesn't look sufficient to me: we might

not

Quoted 1 line

reach this point if subsys_system_register() or

zalloc_cpumask_var()

Quoted 1 line

fail.

If those fail, I'd say we have a much bigger problem than undeleted timers.

You can add

Tested-by Boris Ostrovsky <[email protected]>

if you prefer to go with that version. I still think it's not 100% reliable (because of what I said above) but at least it fixes the current breakage.

Quoted 5 lines
Quoted 3 lines

We could register the notifier as the first thing in this routine (probably after mce_available() succeeds).

I guess...

Actually that won't work --- we need to register bus at sysfs first.

-boris

maintainer
#1 lore raw
Quoted 7 lines · Boris Ostrovsky

You can add

Tested-by Boris Ostrovsky <[email protected]>

if you prefer to go with that version. I still think it's not 100% reliable (because of what I said above) but at least it fixes the current breakage.

Thanks.

The thing is, I don't like to address hypothetical breakages if they can never be triggered on any configuration.

But I'm going to leave an open mind so if you still can trigger *with* this fix, then we can revisit it then again.

Thanks for testing!

Signature · 5 lines
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--
maintainer
#2 lore raw

PATCH 1/1 x86, MCE: Robustify mcheck_init_device

From: Borislav Petkov <[email protected]>
Subject: [PATCH] x86, MCE: Robustify mcheck_init_device

BorisO reports that misc_register() fails often on xen. The current code unregisters the CPU hotplug notifier in that case. If then a CPU is offlined and onlined back again, we end up with a second timer running on that CPU, leading to soft lockups and system hangs.

So let's leave the hotcpu notifier always registered - even if mce_device_create failed for some cores and never unreg it so that we can deal with the timer handling accordingly.

Reported-and-Tested-by Boris Ostrovsky <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Borislav Petkov <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +6 -4
index bb92f38153b2..9a79c8dbd8e8 100644
@@ -2451,6 +2451,12 @@ static __init int mcheck_init_device(void)
 	for_each_online_cpu(i) {
 		err = mce_device_create(i);
 		if (err) {
+			/*
+			 * Register notifier anyway (and do not unreg it) so
+			 * that we don't leave undeleted timers, see notifier
+			 * callback above.
+			 */
+			__register_hotcpu_notifier(&mce_cpu_notifier);
 			cpu_notifier_register_done();
 			goto err_device_create;
 		}

@@ -2471,10 +2477,6 @@ static __init int mcheck_init_device(void)
 err_register:
 	unregister_syscore_ops(&mce_syscore_ops);
 
-	cpu_notifier_register_begin();
-	__unregister_hotcpu_notifier(&mce_cpu_notifier);
-	cpu_notifier_register_done();
-
 err_device_create:
 	/*
 	 * We didn't keep track of which devices were created above, but
Signature · 8 lines
2.0.0

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--