| Message ID | 1373384217-26307-1-git-send-email-sboyd@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/09, Stephen Boyd wrote: > If one process calls sys_reboot and that process then stops other > CPUs while those CPUs are within a spin_lock() region we can > potentially encounter a deadlock scenario like below. > > CPU 0 CPU 1 > ----- ----- > spin_lock(my_lock) > smp_send_stop() > <send IPI> handle_IPI() > disable_preemption/irqs > while(1); > <PREEMPT> > spin_lock(my_lock) <--- Waits forever > > We shouldn't attempt to run any other tasks after we send a stop > IPI to a CPU so disable preemption so that this task runs to > completion. > > Reported-by: Sundarajan Srinivasan <sundaraj@codeaurora.com> > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> > --- > > Resending this patch now that the context has changed. Ping? Shall I put this in the patch tracker? > > arch/arm/kernel/process.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c > index 7f1efcd..8bc12d7 100644 > --- a/arch/arm/kernel/process.c > +++ b/arch/arm/kernel/process.c > @@ -206,6 +206,7 @@ void machine_shutdown(void) > */ > void machine_halt(void) > { > + preempt_disable(); > smp_send_stop(); > > local_irq_disable(); > @@ -220,6 +221,7 @@ void machine_halt(void) > */ > void machine_power_off(void) > { > + preempt_disable(); > smp_send_stop(); > > if (pm_power_off) > @@ -239,6 +241,7 @@ void machine_power_off(void) > */ > void machine_restart(char *cmd) > { > + preempt_disable(); > smp_send_stop(); > > arm_pm_restart(reboot_mode, cmd);
On Wed, Jul 24, 2013 at 11:56:18AM -0700, Stephen Boyd wrote: > On 07/09, Stephen Boyd wrote: > > If one process calls sys_reboot and that process then stops other > > CPUs while those CPUs are within a spin_lock() region we can > > potentially encounter a deadlock scenario like below. > > > > CPU 0 CPU 1 > > ----- ----- > > spin_lock(my_lock) > > smp_send_stop() > > <send IPI> handle_IPI() > > disable_preemption/irqs > > while(1); > > <PREEMPT> > > spin_lock(my_lock) <--- Waits forever > > > > We shouldn't attempt to run any other tasks after we send a stop > > IPI to a CPU so disable preemption so that this task runs to > > completion. > > > > Reported-by: Sundarajan Srinivasan <sundaraj@codeaurora.com> > > Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> > > --- > > > > Resending this patch now that the context has changed. > > Ping? Shall I put this in the patch tracker? Well, looking at x86, they use local_irq_disable() before sending the stop, so I think we should do the same for cross-arch consistency.
On 07/24/13 13:21, Russell King - ARM Linux wrote: > On Wed, Jul 24, 2013 at 11:56:18AM -0700, Stephen Boyd wrote: >> On 07/09, Stephen Boyd wrote: >>> If one process calls sys_reboot and that process then stops other >>> CPUs while those CPUs are within a spin_lock() region we can >>> potentially encounter a deadlock scenario like below. >>> >>> CPU 0 CPU 1 >>> ----- ----- >>> spin_lock(my_lock) >>> smp_send_stop() >>> <send IPI> handle_IPI() >>> disable_preemption/irqs >>> while(1); >>> <PREEMPT> >>> spin_lock(my_lock) <--- Waits forever >>> >>> We shouldn't attempt to run any other tasks after we send a stop >>> IPI to a CPU so disable preemption so that this task runs to >>> completion. >>> >>> Reported-by: Sundarajan Srinivasan <sundaraj@codeaurora.com> >>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> >>> --- >>> >>> Resending this patch now that the context has changed. >> Ping? Shall I put this in the patch tracker? > Well, looking at x86, they use local_irq_disable() before sending the > stop, so I think we should do the same for cross-arch consistency. Fair enough. I'll send v2 with that.
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 7f1efcd..8bc12d7 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -206,6 +206,7 @@ void machine_shutdown(void) */ void machine_halt(void) { + preempt_disable(); smp_send_stop(); local_irq_disable(); @@ -220,6 +221,7 @@ void machine_halt(void) */ void machine_power_off(void) { + preempt_disable(); smp_send_stop(); if (pm_power_off) @@ -239,6 +241,7 @@ void machine_power_off(void) */ void machine_restart(char *cmd) { + preempt_disable(); smp_send_stop(); arm_pm_restart(reboot_mode, cmd);
If one process calls sys_reboot and that process then stops other CPUs while those CPUs are within a spin_lock() region we can potentially encounter a deadlock scenario like below. CPU 0 CPU 1 ----- ----- spin_lock(my_lock) smp_send_stop() <send IPI> handle_IPI() disable_preemption/irqs while(1); <PREEMPT> spin_lock(my_lock) <--- Waits forever We shouldn't attempt to run any other tasks after we send a stop IPI to a CPU so disable preemption so that this task runs to completion. Reported-by: Sundarajan Srinivasan <sundaraj@codeaurora.com> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> --- Resending this patch now that the context has changed. arch/arm/kernel/process.c | 3 +++ 1 file changed, 3 insertions(+)