Re: patch revert-fix-smp-poweroff-hangs.patch queued to -stable tree

Re: patch revert-fix-smp-poweroff-hangs.patch queued to -stable tree

[Dave Jones]

On Thu, Dec 13, 2007 at 08:45:45AM -0800, Greg KH wrote:
 > >> There still is a remaining shutdown problem in 2.6.22 with old APM based
 > >> systems, but this fix is not the correct one
 > > ..
 > >
 > > Eh?  It may not fix APM systems, but it *does* fix several known examples
 > > of ACPI systems.
 > 
 > And I have a large number of real systems that this patch has caused
 > problems with.  So much so that we had to revert it from a shipping SuSE
 > product already, so I figured it was safe to also revert this in
 > mainline as it did cause regressions.

Cheesy, but why not something like..

+	if (acpi_enabled)		// Or whatever the var name is these days
+		disable_nonboot_cpus();

until we find out more?

	Dave

-- 
http://www.codemonkey.org.uk

SMP poweroff hangs: it's baaaack! But on x86_64 this time.

[Mark Lord]

> Subject: Fix SMP poweroff hangs
> From: Mark Lord <lkml@rtr.ca>
> 
> We need to disable all CPUs other than the boot CPU (usually 0) before
> attempting to power-off modern SMP machines.  This fixes the
> hang-on-poweroff issue on my MythTV SMP box, and also on Thomas Gleixner's
> new toybox.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  kernel/sys.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff -puN kernel/sys.c~fix-smp-poweroff-hangs kernel/sys.c
> --- a/kernel/sys.c~fix-smp-poweroff-hangs
> +++ a/kernel/sys.c
> @@ -32,6 +32,7 @@
>  #include <linux/getcpu.h>
>  #include <linux/task_io_accounting_ops.h>
>  #include <linux/seccomp.h>
> +#include <linux/cpu.h>
>  
>  #include <linux/compat.h>
>  #include <linux/syscalls.h>
> @@ -878,6 +879,7 @@ void kernel_power_off(void)
>  	kernel_shutdown_prepare(SYSTEM_POWER_OFF);
>  	if (pm_power_off_prepare)
>  		pm_power_off_prepare();
> +	disable_nonboot_cpus();
>  	sysdev_shutdown();
>  	printk(KERN_EMERG "Power down.\n");
>  	machine_power_off();
..

This bug has returned here now, but on x86_86 this time around.
Same machine as before, just upgraded toa 64-bit kernel/user (2.6.27.9)
from the original 32-bit kernel/user that was originally fixed (above).

One hang at poweroff over the past 10 days.  Not much, but enough
to destroy confidence in "unattended" operation.

I lack opportunity to dig further into the code for now,
but just wanted to flag the problem, in case similar reports
from others might already be out there.

In the meanwhile, I'm experimenting with this simple patch,
garnered from the 32-bit investigations last time around.
We should know in a few weeks whether it has any effect or not.

--- old/kernel/sys.c	2008-10-18 13:57:22.000000000 -0400
+++ linux-2.6.27.9/kernel/sys.c	2008-12-17 09:42:17.000000000 -0500
@@ -303,6 +303,8 @@
 
 static void kernel_shutdown_prepare(enum system_states state)
 {
+	set_cpus_allowed(current, cpumask_of_cpu(first_cpu(cpu_online_map)));
+	disable_nonboot_cpus();
 	blocking_notifier_call_chain(&reboot_notifier_list,
 		(state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
 	system_state = state;
@@ -333,7 +335,6 @@
 	kernel_shutdown_prepare(SYSTEM_POWER_OFF);
 	if (pm_power_off_prepare)
 		pm_power_off_prepare();
-	disable_nonboot_cpus();
 	sysdev_shutdown();
 	printk(KERN_EMERG "Power down.\n");
 	machine_power_off();

Re: SMP poweroff hangs: it's baaaack! But on x86_64 this time.

[Pavel Machek]

On Wed 2008-12-17 10:48:02, Mark Lord wrote:
>> Subject: Fix SMP poweroff hangs
>> From: Mark Lord <lkml@rtr.ca>
>>
>> We need to disable all CPUs other than the boot CPU (usually 0) before
>> attempting to power-off modern SMP machines.  This fixes the
>> hang-on-poweroff issue on my MythTV SMP box, and also on Thomas Gleixner's
>> new toybox.
>>
>> Signed-off-by: Mark Lord <mlord@pobox.com>
>> Acked-by: Thomas Gleixner <tglx@linutronix.de>
>> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>> ---
>>
>>  kernel/sys.c |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff -puN kernel/sys.c~fix-smp-poweroff-hangs kernel/sys.c
>> --- a/kernel/sys.c~fix-smp-poweroff-hangs
>> +++ a/kernel/sys.c
>> @@ -32,6 +32,7 @@
>>  #include <linux/getcpu.h>
>>  #include <linux/task_io_accounting_ops.h>
>>  #include <linux/seccomp.h>
>> +#include <linux/cpu.h>
>>   #include <linux/compat.h>
>>  #include <linux/syscalls.h>
>> @@ -878,6 +879,7 @@ void kernel_power_off(void)
>>  	kernel_shutdown_prepare(SYSTEM_POWER_OFF);
>>  	if (pm_power_off_prepare)
>>  		pm_power_off_prepare();
>> +	disable_nonboot_cpus();
>>  	sysdev_shutdown();
>>  	printk(KERN_EMERG "Power down.\n");
>>  	machine_power_off();
> ..
>
> This bug has returned here now, but on x86_86 this time around.
> Same machine as before, just upgraded toa 64-bit kernel/user (2.6.27.9)
> from the original 32-bit kernel/user that was originally fixed (above).
>
> One hang at poweroff over the past 10 days.  Not much, but enough
> to destroy confidence in "unattended" operation.
>
> I lack opportunity to dig further into the code for now,
> but just wanted to flag the problem, in case similar reports
> from others might already be out there.
>
> In the meanwhile, I'm experimenting with this simple patch,
> garnered from the 32-bit investigations last time around.
> We should know in a few weeks whether it has any effect or not.
>
> --- old/kernel/sys.c	2008-10-18 13:57:22.000000000 -0400
> +++ linux-2.6.27.9/kernel/sys.c	2008-12-17 09:42:17.000000000 -0500
> @@ -303,6 +303,8 @@
>
> static void kernel_shutdown_prepare(enum system_states state)
> {
> +	set_cpus_allowed(current, cpumask_of_cpu(first_cpu(cpu_online_map)));

Is this line neccessary? 

> +	disable_nonboot_cpus();
> 	blocking_notifier_call_chain(&reboot_notifier_list,
> 		(state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
> 	system_state = state;
> @@ -333,7 +335,6 @@
> 	kernel_shutdown_prepare(SYSTEM_POWER_OFF);
> 	if (pm_power_off_prepare)
> 		pm_power_off_prepare();
> -	disable_nonboot_cpus();
> 	sysdev_shutdown();
> 	printk(KERN_EMERG "Power down.\n");
> 	machine_power_off();

Do you have any idea why it helps? BIOS will see us shutting down on
cpu0 anyway, so if this helps there's a linux bug somewhere...

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: SMP poweroff hangs: it's baaaack! But on x86_64 this time.

[Mark Lord]

Pavel Machek wrote:
> On Wed 2008-12-17 10:48:02, Mark Lord wrote:
>>> Subject: Fix SMP poweroff hangs
>>> From: Mark Lord <lkml@rtr.ca>
>>>
>>> We need to disable all CPUs other than the boot CPU (usually 0) before
>>> attempting to power-off modern SMP machines.  This fixes the
>>> hang-on-poweroff issue on my MythTV SMP box, and also on Thomas Gleixner's
>>> new toybox.
>>>
>>> Signed-off-by: Mark Lord <mlord@pobox.com>
>>> Acked-by: Thomas Gleixner <tglx@linutronix.de>
>>> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>> ---
>>>
>>>  kernel/sys.c |    2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff -puN kernel/sys.c~fix-smp-poweroff-hangs kernel/sys.c
>>> --- a/kernel/sys.c~fix-smp-poweroff-hangs
>>> +++ a/kernel/sys.c
>>> @@ -32,6 +32,7 @@
>>>  #include <linux/getcpu.h>
>>>  #include <linux/task_io_accounting_ops.h>
>>>  #include <linux/seccomp.h>
>>> +#include <linux/cpu.h>
>>>   #include <linux/compat.h>
>>>  #include <linux/syscalls.h>
>>> @@ -878,6 +879,7 @@ void kernel_power_off(void)
>>>  	kernel_shutdown_prepare(SYSTEM_POWER_OFF);
>>>  	if (pm_power_off_prepare)
>>>  		pm_power_off_prepare();
>>> +	disable_nonboot_cpus();
>>>  	sysdev_shutdown();
>>>  	printk(KERN_EMERG "Power down.\n");
>>>  	machine_power_off();
>> ..
>>
>> This bug has returned here now, but on x86_86 this time around.
>> Same machine as before, just upgraded toa 64-bit kernel/user (2.6.27.9)
>> from the original 32-bit kernel/user that was originally fixed (above).
>>
>> One hang at poweroff over the past 10 days.  Not much, but enough
>> to destroy confidence in "unattended" operation.
>>
>> I lack opportunity to dig further into the code for now,
>> but just wanted to flag the problem, in case similar reports
>> from others might already be out there.
>>
>> In the meanwhile, I'm experimenting with this simple patch,
>> garnered from the 32-bit investigations last time around.
>> We should know in a few weeks whether it has any effect or not.
>>
>> --- old/kernel/sys.c	2008-10-18 13:57:22.000000000 -0400
>> +++ linux-2.6.27.9/kernel/sys.c	2008-12-17 09:42:17.000000000 -0500
>> @@ -303,6 +303,8 @@
>>
>> static void kernel_shutdown_prepare(enum system_states state)
>> {
>> +	set_cpus_allowed(current, cpumask_of_cpu(first_cpu(cpu_online_map)));
> 
> Is this line neccessary?
..

One or the other is probably redundant, but it's not 100% clear *which*.  :)
The line above was suggested by others back when the original problem
was being worked on for 32-bit.

>> +	disable_nonboot_cpus();
>> 	blocking_notifier_call_chain(&reboot_notifier_list,
>> 		(state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
>> 	system_state = state;
>> @@ -333,7 +335,6 @@
>> 	kernel_shutdown_prepare(SYSTEM_POWER_OFF);
>> 	if (pm_power_off_prepare)
>> 		pm_power_off_prepare();
>> -	disable_nonboot_cpus();
>> 	sysdev_shutdown();
>> 	printk(KERN_EMERG "Power down.\n");
>> 	machine_power_off();
> 
> Do you have any idea why it helps? BIOS will see us shutting down on
> cpu0 anyway, so if this helps there's a linux bug somewhere...
..

I don't know if it helps, yet.  Needs a lot more soak first.
The theory here is, that all BIOS interactions from the kernel
probably should happen only on the "boot CPU" or on "cpu 0",
not just the "halt" sequence.

By setting the CPU restriction earlier in the shutdown sequence,
we should be on the correct CPU for all of the various device-driver
BIOS ineractions that might happen before actually halting.

Could be a bad theory, but I'm open to other possible suggestions.

Back on 32-bit it did seem rather unlikely that "disable_nonboot_cpus()"
would help, but it in fact cured the problem completely there.
On two different brands of motherboards with two different BIOSs.

Cheers

Re: SMP poweroff hangs: it's baaaack! But on x86_64 this time.

[Mark Lord]

Mark Lord wrote:
> Pavel Machek wrote:
>> On Wed 2008-12-17 10:48:02, Mark Lord wrote:
>>>> Subject: Fix SMP poweroff hangs
>>>> From: Mark Lord <lkml@rtr.ca>
>>>>
>>>> We need to disable all CPUs other than the boot CPU (usually 0) before
>>>> attempting to power-off modern SMP machines.  This fixes the
>>>> hang-on-poweroff issue on my MythTV SMP box, and also on Thomas 
>>>> Gleixner's
>>>> new toybox.
>>>>
>>>> Signed-off-by: Mark Lord <mlord@pobox.com>
>>>> Acked-by: Thomas Gleixner <tglx@linutronix.de>
>>>> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
>>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>>> ---
>>>>
>>>>  kernel/sys.c |    2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff -puN kernel/sys.c~fix-smp-poweroff-hangs kernel/sys.c
>>>> --- a/kernel/sys.c~fix-smp-poweroff-hangs
>>>> +++ a/kernel/sys.c
>>>> @@ -32,6 +32,7 @@
>>>>  #include <linux/getcpu.h>
>>>>  #include <linux/task_io_accounting_ops.h>
>>>>  #include <linux/seccomp.h>
>>>> +#include <linux/cpu.h>
>>>>   #include <linux/compat.h>
>>>>  #include <linux/syscalls.h>
>>>> @@ -878,6 +879,7 @@ void kernel_power_off(void)
>>>>      kernel_shutdown_prepare(SYSTEM_POWER_OFF);
>>>>      if (pm_power_off_prepare)
>>>>          pm_power_off_prepare();
>>>> +    disable_nonboot_cpus();
>>>>      sysdev_shutdown();
>>>>      printk(KERN_EMERG "Power down.\n");
>>>>      machine_power_off();
>>> ..
>>>
>>> This bug has returned here now, but on x86_86 this time around.
>>> Same machine as before, just upgraded toa 64-bit kernel/user (2.6.27.9)
>>> from the original 32-bit kernel/user that was originally fixed (above).
>>>
>>> One hang at poweroff over the past 10 days.  Not much, but enough
>>> to destroy confidence in "unattended" operation.
>>>
>>> I lack opportunity to dig further into the code for now,
>>> but just wanted to flag the problem, in case similar reports
>>> from others might already be out there.
>>>
>>> In the meanwhile, I'm experimenting with this simple patch,
>>> garnered from the 32-bit investigations last time around.
>>> We should know in a few weeks whether it has any effect or not.
>>>
>>> --- old/kernel/sys.c    2008-10-18 13:57:22.000000000 -0400
>>> +++ linux-2.6.27.9/kernel/sys.c    2008-12-17 09:42:17.000000000 -0500
>>> @@ -303,6 +303,8 @@
>>>
>>> static void kernel_shutdown_prepare(enum system_states state)
>>> {
>>> +    set_cpus_allowed(current, 
>>> cpumask_of_cpu(first_cpu(cpu_online_map)));
>>
>> Is this line neccessary?
> ..
> 
> One or the other is probably redundant, but it's not 100% clear 
> *which*.  :)
> The line above was suggested by others back when the original problem
> was being worked on for 32-bit.
> 
>>> +    disable_nonboot_cpus();
>>>     blocking_notifier_call_chain(&reboot_notifier_list,
>>>         (state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
>>>     system_state = state;
>>> @@ -333,7 +335,6 @@
>>>     kernel_shutdown_prepare(SYSTEM_POWER_OFF);
>>>     if (pm_power_off_prepare)
>>>         pm_power_off_prepare();
>>> -    disable_nonboot_cpus();
>>>     sysdev_shutdown();
>>>     printk(KERN_EMERG "Power down.\n");
>>>     machine_power_off();
>>
>> Do you have any idea why it helps? BIOS will see us shutting down on
>> cpu0 anyway, so if this helps there's a linux bug somewhere...
> ..
> 
> I don't know if it helps, yet.  Needs a lot more soak first.
> The theory here is, that all BIOS interactions from the kernel
> probably should happen only on the "boot CPU" or on "cpu 0",
> not just the "halt" sequence.
> 
> By setting the CPU restriction earlier in the shutdown sequence,
> we should be on the correct CPU for all of the various device-driver
> BIOS ineractions that might happen before actually halting.
> 
> Could be a bad theory, but I'm open to other possible suggestions.
..

Well, it has hung on poweroff again now, so that patch apparently doesn't
cure the problem.  Darn.  The machine is at remote site, and I won't have
access to it for another week or so to do any further diagnostics.
Perhaps we'll just leave it running for the whole week.

Cheers

Re: SMP poweroff hangs: it's baaaack! But on x86_64 this time.

[Mark Lord]

Mark Lord wrote:
> Mark Lord wrote:
>> Pavel Machek wrote:
>>> On Wed 2008-12-17 10:48:02, Mark Lord wrote:
>>>>> Subject: Fix SMP poweroff hangs
>>>>> From: Mark Lord <lkml@rtr.ca>
>>>>>
>>>>> We need to disable all CPUs other than the boot CPU (usually 0) before
>>>>> attempting to power-off modern SMP machines.  This fixes the
>>>>> hang-on-poweroff issue on my MythTV SMP box, and also on Thomas 
>>>>> Gleixner's
>>>>> new toybox.
>>>>>
>>>>> Signed-off-by: Mark Lord <mlord@pobox.com>
>>>>> Acked-by: Thomas Gleixner <tglx@linutronix.de>
>>>>> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
>>>>> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
>>>>> ---
>>>>>
>>>>>  kernel/sys.c |    2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff -puN kernel/sys.c~fix-smp-poweroff-hangs kernel/sys.c
>>>>> --- a/kernel/sys.c~fix-smp-poweroff-hangs
>>>>> +++ a/kernel/sys.c
>>>>> @@ -32,6 +32,7 @@
>>>>>  #include <linux/getcpu.h>
>>>>>  #include <linux/task_io_accounting_ops.h>
>>>>>  #include <linux/seccomp.h>
>>>>> +#include <linux/cpu.h>
>>>>>   #include <linux/compat.h>
>>>>>  #include <linux/syscalls.h>
>>>>> @@ -878,6 +879,7 @@ void kernel_power_off(void)
>>>>>      kernel_shutdown_prepare(SYSTEM_POWER_OFF);
>>>>>      if (pm_power_off_prepare)
>>>>>          pm_power_off_prepare();
>>>>> +    disable_nonboot_cpus();
>>>>>      sysdev_shutdown();
>>>>>      printk(KERN_EMERG "Power down.\n");
>>>>>      machine_power_off();
>>>> ..
>>>>
>>>> This bug has returned here now, but on x86_86 this time around.
>>>> Same machine as before, just upgraded toa 64-bit kernel/user (2.6.27.9)
>>>> from the original 32-bit kernel/user that was originally fixed (above).
>>>>
>>>> One hang at poweroff over the past 10 days.  Not much, but enough
>>>> to destroy confidence in "unattended" operation.
>>>>
>>>> I lack opportunity to dig further into the code for now,
>>>> but just wanted to flag the problem, in case similar reports
>>>> from others might already be out there.
>>>>
>>>> In the meanwhile, I'm experimenting with this simple patch,
>>>> garnered from the 32-bit investigations last time around.
>>>> We should know in a few weeks whether it has any effect or not.
>>>>
>>>> --- old/kernel/sys.c    2008-10-18 13:57:22.000000000 -0400
>>>> +++ linux-2.6.27.9/kernel/sys.c    2008-12-17 09:42:17.000000000 -0500
>>>> @@ -303,6 +303,8 @@
>>>>
>>>> static void kernel_shutdown_prepare(enum system_states state)
>>>> {
>>>> +    set_cpus_allowed(current, 
>>>> cpumask_of_cpu(first_cpu(cpu_online_map)));
>>>
>>> Is this line neccessary?
>> ..
>>
>> One or the other is probably redundant, but it's not 100% clear 
>> *which*.  :)
>> The line above was suggested by others back when the original problem
>> was being worked on for 32-bit.
>>
>>>> +    disable_nonboot_cpus();
>>>>     blocking_notifier_call_chain(&reboot_notifier_list,
>>>>         (state == SYSTEM_HALT)?SYS_HALT:SYS_POWER_OFF, NULL);
>>>>     system_state = state;
>>>> @@ -333,7 +335,6 @@
>>>>     kernel_shutdown_prepare(SYSTEM_POWER_OFF);
>>>>     if (pm_power_off_prepare)
>>>>         pm_power_off_prepare();
>>>> -    disable_nonboot_cpus();
>>>>     sysdev_shutdown();
>>>>     printk(KERN_EMERG "Power down.\n");
>>>>     machine_power_off();
>>>
>>> Do you have any idea why it helps? BIOS will see us shutting down on
>>> cpu0 anyway, so if this helps there's a linux bug somewhere...
>> ..
>>
>> I don't know if it helps, yet.  Needs a lot more soak first.
>> The theory here is, that all BIOS interactions from the kernel
>> probably should happen only on the "boot CPU" or on "cpu 0",
>> not just the "halt" sequence.
>>
>> By setting the CPU restriction earlier in the shutdown sequence,
>> we should be on the correct CPU for all of the various device-driver
>> BIOS ineractions that might happen before actually halting.
>>
>> Could be a bad theory, but I'm open to other possible suggestions.
> ..
> 
> Well, it has hung on poweroff again now, so that patch apparently doesn't
> cure the problem.  Darn.  The machine is at remote site, and I won't have
> access to it for another week or so to do any further diagnostics.
> Perhaps we'll just leave it running for the whole week.
..

Heh.. ten days ago, I instrumented the halt/poweroff functions in the kernel,
with addtional step-by-step printk's to help figure out where it hangs.

But of course, just having the printk's there changes the timing enough
that the race which would hang it, now never happens.

Heh.. I wonder if a patch full of printk's would ever make it past Linus here.  :)

Cheers