[PATCH] kernel/sys.c: call disable_nonboot_cpus in kernel_restart

[PATCH] kernel/sys.c: call disable_nonboot_cpus in kernel_restart

[Shawn Guo]

Like kernel_power_off calls disable_nonboot_cpus, we may want to have
kernel_restart call disable_nonboot_cpus as well.  Doing so can help
the machines that require boot cpu be the last alive cpu during reboot
to survive with kernel restart.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
 kernel/sys.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index 241507f..6fab59a 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -368,6 +368,7 @@ EXPORT_SYMBOL(unregister_reboot_notifier);
 void kernel_restart(char *cmd)
 {
 	kernel_restart_prepare(cmd);
+	disable_nonboot_cpus();
 	if (!cmd)
 		printk(KERN_EMERG "Restarting system.\n");
 	else
-- 
1.7.5.4

Re: [PATCH] kernel/sys.c: call disable_nonboot_cpus in kernel_restart

[Andrew Morton]

On Mon,  3 Sep 2012 22:24:58 +0800
Shawn Guo <shawn.guo@linaro.org> wrote:

> Like kernel_power_off calls disable_nonboot_cpus, we may want to have
> kernel_restart call disable_nonboot_cpus as well.  Doing so can help
> the machines that require boot cpu be the last alive cpu during reboot
> to survive with kernel restart.

That does sound logical.  But the changelog is very vague and fluffy. 
Does this patch actually fix any known problem on any known machine?

> ...
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -368,6 +368,7 @@ EXPORT_SYMBOL(unregister_reboot_notifier);
>  void kernel_restart(char *cmd)
>  {
>  	kernel_restart_prepare(cmd);
> +	disable_nonboot_cpus();
>  	if (!cmd)
>  		printk(KERN_EMERG "Restarting system.\n");
>  	else

Re: [PATCH] kernel/sys.c: call disable_nonboot_cpus in kernel_restart

[Stephen Boyd]

On 09/04/12 15:36, Andrew Morton wrote:
> On Mon,  3 Sep 2012 22:24:58 +0800
> Shawn Guo <shawn.guo@linaro.org> wrote:
>
>> Like kernel_power_off calls disable_nonboot_cpus, we may want to have
>> kernel_restart call disable_nonboot_cpus as well.  Doing so can help
>> the machines that require boot cpu be the last alive cpu during reboot
>> to survive with kernel restart.
> That does sound logical.  But the changelog is very vague and fluffy. 
> Does this patch actually fix any known problem on any known machine?
>

Not to hijack this thread but I have a similar problem that would be
partially solved by patch. Basically reboot races with cpu_up and causes
a BUG_ON to hit in stop machine. So this patch would fix my problem
except for in the case where restart is called from panic (i.e.
emergency restart).

https://lkml.org/lkml/2012/8/22/3

Here's the commit text:

Nothing stops a process from hotplugging in a CPU concurrently
with a sys_reboot() call. In such a situation we could have
ipi_cpu_stop() mark a cpu as 'offline' and _cpu_up() ignore the
fact that the CPU is not really offline and call the
CPU_UP_PREPARE notifier. When this happens stop_machine code will
complain that the cpu thread already exists and BUG_ON().

CPU0                      CPU1

sys_reboot()
 kernel_restart()
  machine_restart()
   machine_shutdown()
    smp_send_stop()
    ...                   ipi_cpu_stop()
                           set_cpu_online(1, false)
                            local_irq_disable()
                             while(1)
    <PREEMPT>
cpu_up()
 _cpu_up()
   if (!cpu_online(1))
    __cpu_notify(CPU_UP_PREPARE...)
cpu_stop_cpu_callback()
  BUG_ON(stopper->thread)

This is easily reproducible by hotplugging in and out in a tight
loop while also rebooting.

Since the CPU is not really offline and hasn't gone through the
proper steps to be marked as such, let's mark the CPU as inactive.
This is just as easily testable as online and avoids any possibility
of _cpu_up() trying to bring the CPU back online when it never was
offline to begin with.


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] kernel/sys.c: call disable_nonboot_cpus in kernel_restart

[Shawn Guo]

On Tue, Sep 04, 2012 at 03:36:24PM -0700, Andrew Morton wrote:
> On Mon,  3 Sep 2012 22:24:58 +0800
> Shawn Guo <shawn.guo@linaro.org> wrote:
> 
> > Like kernel_power_off calls disable_nonboot_cpus, we may want to have
> > kernel_restart call disable_nonboot_cpus as well.  Doing so can help
> > the machines that require boot cpu be the last alive cpu during reboot
> > to survive with kernel restart.
> 
> That does sound logical.  But the changelog is very vague and fluffy. 
> Does this patch actually fix any known problem on any known machine?
> 
Yes, it fixes one reboot issue seen on imx6q (Cortex-A9 Quad).  The
machine requires the restart routine run on primary cpu than secondary
ones.  Otherwise, the secondary core running restart routine will fail
to come to online after reboot.

-- 
Regards,
Shawn