All of lore.kernel.org
 help / color / mirror / Atom feed
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC/PATCH] ARM: smp: Fix cpu_up() racing with sys_reboot
Date: Wed, 29 Aug 2012 00:24:02 -0700	[thread overview]
Message-ID: <503DC392.4070904@codeaurora.org> (raw)
In-Reply-To: <1345608229-5707-1-git-send-email-sboyd@codeaurora.org>

On 8/21/2012 9:03 PM, Stephen Boyd wrote:
> 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.
>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>
> Perhaps we can take the hotplug lock in the sys_reboot() case but I
> don't think that actually fixes everything. For example, in cases
> where machine_shutdown() is called from emergency_restart() we would
> have to take the hotplug lock which doesn't really seem feasible.

Any comments on this patch?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@codeaurora.org>
To: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC/PATCH] ARM: smp: Fix cpu_up() racing with sys_reboot
Date: Wed, 29 Aug 2012 00:24:02 -0700	[thread overview]
Message-ID: <503DC392.4070904@codeaurora.org> (raw)
In-Reply-To: <1345608229-5707-1-git-send-email-sboyd@codeaurora.org>

On 8/21/2012 9:03 PM, Stephen Boyd wrote:
> 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.
>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>
> Perhaps we can take the hotplug lock in the sys_reboot() case but I
> don't think that actually fixes everything. For example, in cases
> where machine_shutdown() is called from emergency_restart() we would
> have to take the hotplug lock which doesn't really seem feasible.

Any comments on this patch?

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


  reply	other threads:[~2012-08-29  7:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-22  4:03 [RFC/PATCH] ARM: smp: Fix cpu_up() racing with sys_reboot Stephen Boyd
2012-08-22  4:03 ` Stephen Boyd
2012-08-29  7:24 ` Stephen Boyd [this message]
2012-08-29  7:24   ` Stephen Boyd
2012-08-29 23:53 ` Russell King - ARM Linux
2012-08-29 23:53   ` Russell King - ARM Linux
2012-08-30 19:00   ` Stephen Boyd
2012-08-30 19:00     ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503DC392.4070904@codeaurora.org \
    --to=sboyd@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.