Re: x86/mce: machine check warning during poweroff

[Srivatsa S. Bhat]

On 01/14/2012 05:35 AM, Linus Torvalds wrote:

> On Fri, Jan 13, 2012 at 3:27 PM, Srivatsa S. Bhat
> <srivatsa.bhat@xxxxxxxxxxxxxxxxxx> wrote:
>>
>> # echo 1 > /sys/devices/system/cpu/cpu1/online
>>
>> [   75.476772] Booting Node 0 Processor 1 APIC 0x2
>> [   75.481495] smpboot cpu 1: start_ip = 97000
>> [   75.492927] Calibrating delay loop (skipped) already calibrated this CPU
>> [   75.508449] NMI watchdog enabled, takes one hw-pmu counter.
>> [   75.515402] general protection fault: 0000 [#1] SMP
>> [   75.518940]
>> [   75.518940] Pid: 6631, comm: bash Tainted: G        W    3.2.0-debugkernel-0.0.0.28.36b5ec9-default #4 IBM IBM System x -[7870C4Q]-/68Y8033
>> [   75.518940] RIP: 0010:[<ffffffff81270779>]  [<ffffffff81270779>] kobject_get+0x19/0x60
>> [   75.518940] RSP: 0018:ffff8808c6cc7c18  EFLAGS: 00010206
>> [   75.518940] RAX: 0000000000000000 RBX: 6b6b6b6b6b6b6b7b RCX: 0000000000000006
>> [   75.518940] RDX: ffffffff81e98ae0 RSI: ffff8808ccc93080 RDI: 6b6b6b6b6b6b6b7b
> 
> The magic is the %rdi value. The instruction that oopses is
> 
>     mov    0x38(%rdi),%eax
> 
> and "rdi" is 0x10 + the magic 6b6b6b.. pattern. Which is obviously
> 'poison_free'.
> 
> And the 0x10 is because get_device() does
> 
>     return dev ? to_dev(kobject_get(&dev->kobj)) : NULL;
> 
> and I bet "kobj" is at offset 16 in the device structure. So we had a
> pointer to a "struct device", but it was loaded from memory that was
> free'd, turning the kobject pointer into that 0x6b6b6b6b6b6b6b7b
> 
> So somebody got a pointer from free'd memory. That somebody seems to
> be 'klist_devices_get()' that got it from a 'struct klist_node', so I
> think we have free'd something from the klist_devices list in the bus.
> But I dunno. Odd. I would have expected us to hit that invalid pointer
> long before if the klist entry was bogus.
> 
> I'm not seeign anything obvious in mce.c. But the fact that it's that
> magic per_cpu allocation makes me nervous. It uses that magic
> "mce_device_initialized" bit array etc, and ti clearly must have
> worked before, but it equally clearly does *not* work now.
> 
> Looking more at it, I think that maybe something keeps the mce_device
> around (refcounts that didn't use to exist before?) so when we
> unregister it, it is still in use. And then when we re-register it
> when we bring it up, we do that
> 
>     memset(&dev->kobj, 0, sizeof(struct kobject));
> 
> on the device that is in use. I dunno. It's all scary. Somebody who
> knows the MCE layer should look at it.
> 
>                    Linus
> 


YES!! Finally I have a fix for this whole MCE thing! :-)

The patch below works perfectly for me - I tested multiple CPU hotplug
operations as well as multiple pm_test runs at core level. Please let me
know if this solves the suspend issue as well..

Of course, the warnings at drivers/base/core.c: device_release()
as well as the IPI to offline cpu warnings still appear but are rather
unrelated and harmless to the issue being discussed.
So, with this patch CPU hotplug doesn't crash the system and suspend and
hibernate are expected to work fine.

-------
From: Srivatsa S. Bhat <srivatsa.bhat@xxxxxxxxxxxxxxxxxx>
Subject: [PATCH] x86/mce: Fix CPU hotplug and suspend regression related to MCE

Commit 8a25a2f (cpu: convert 'cpu' and 'machinecheck' sysdev_class
to a regular subsystem) changed how things are dealt with in
the MCE subsystem. Some of the things that got broken due to this
are CPU hotplug and suspend/hibernate.

MCE uses per_cpu allocations of struct device. So, when a CPU goes
offline and comes back online, in order to ensure that we start
from a clean slate with respect to the MCE subsystem, zero out the
entire per_cpu device structure to 0 before using it.

Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@xxxxxxxxxxxxxxxxxx>
---

 arch/x86/kernel/cpu/mcheck/mce.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index f22a9f7..29ba329 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -2011,7 +2011,7 @@ static __cpuinit int mce_device_create(unsigned int cpu)
 	if (!mce_available(&boot_cpu_data))
 		return -EIO;
 
-	memset(&dev->kobj, 0, sizeof(struct kobject));
+	memset(dev, 0, sizeof(struct device));
 	dev->id  = cpu;
 	dev->bus = &mce_subsys;
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/