| Message ID | 1399555411-15330-1-git-send-email-arunks.linux@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 08, 2014 at 06:53:31PM +0530, arunks.linux@gmail.com wrote: > From: Arun KS <arunks.linux@gmail.com> > > 'irqs disabled' is funtamentally unsafe way of disabling preemption. > Any spin_unlock() decreasing the preemption count to 0 might trigger > a reschedule. A simple printk() might trigger a reschedule. > > To be on safe side disable preemption as well using preempt_disable() NAK. asmlinkage void __sched notrace preempt_schedule(void) { /* * If there is a non-zero preempt_count or interrupts are disabled, * we do not want to preempt the current task. Just return.. */ if (likely(!preemptible())) return;
On Thu, May 8, 2014 at 7:19 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, May 08, 2014 at 06:53:31PM +0530, arunks.linux@gmail.com wrote: >> From: Arun KS <arunks.linux@gmail.com> >> >> 'irqs disabled' is funtamentally unsafe way of disabling preemption. >> Any spin_unlock() decreasing the preemption count to 0 might trigger >> a reschedule. A simple printk() might trigger a reschedule. >> >> To be on safe side disable preemption as well using preempt_disable() > > NAK. > > asmlinkage void __sched notrace preempt_schedule(void) > { > /* > * If there is a non-zero preempt_count or interrupts are disabled, > * we do not want to preempt the current task. Just return.. > */ > if (likely(!preemptible())) > return; What if we fall in a path which calls spin_unlock_irq(). Interrupts will get enabled and only preempt_count will help to bail out early from preempt_schedule(). I recently hit a similar situation. Thanks, > > -- > FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly > improving, and getting towards what was expected from it.
On Thu, May 08, 2014 at 07:46:21PM +0530, Arun KS wrote: > What if we fall in a path which calls spin_unlock_irq(). > Interrupts will get enabled and only preempt_count will help to bail > out early from preempt_schedule(). > I recently hit a similar situation. Use of spin_lock_irq()..spin_unlock_irq() in code which was entered with interrupts already disabled is a bug in itself. Papering over it with preempt_disable() is not an option. /Always/ fix the real problem, never paper over stuff like this.
On Thu, May 8, 2014 at 8:52 PM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Thu, May 08, 2014 at 07:46:21PM +0530, Arun KS wrote: >> What if we fall in a path which calls spin_unlock_irq(). >> Interrupts will get enabled and only preempt_count will help to bail >> out early from preempt_schedule(). >> I recently hit a similar situation. > > Use of spin_lock_irq()..spin_unlock_irq() in code which was entered with > interrupts already disabled is a bug in itself. > > Papering over it with preempt_disable() is not an option. /Always/ fix > the real problem, never paper over stuff like this. Thanks Russell for your explanation. During power onoff test, we hit the below scenario, #0 [<c072df48>] (__schedule) from [<c072e3a0>] #1 [<c072e3a0>] (preempt_schedule) from [<c072f244>] #2 [<c072f244>] (_raw_spin_unlock_irq) from [<c0394468>] #3 [<c0394468>] (__rpm_callback) from [<c03944b0>] #4 [<c03944b0>] (rpm_callback) from [<c03957bc>] #5 [<c03957bc>] (rpm_resume) from [<c0395d18>] #6 [<c0395d18>] (__pm_runtime_resume) from [<c04675b4>] #7 [<c04675b4>] (sh_mobile_i2c_xfer) from [<c0460390>] #8 [<c0460390>] (__i2c_transfer) from [<c0462094>] #9 [<c0462094>] (i2c_transfer) from [<c03b2738>] #10 [<c03b2738>] (d2153_i2c_write_device) from [<c03b08b0>] #11 [<c03b08b0>] (d2153_write) from [<c03b1574>] #12 [<c03b1574>] (d2153_reg_write) from [<c03b2490>] #13 [<c03b2490>] (d2153_system_poweroff) from [<c0011278>] #14 [<c0011278>] (machine_power_off) from [<c0047d58>] #15 [<c0047d58>] (kernel_power_off) from [<c0048420>] #16 [<c0048420>] (sys_reboot) from [<c0010240>] I can fix this by adding pm_runtime_irq_safe(&dev->dev); Thanks, Arun > > -- > FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly > improving, and getting towards what was expected from it.
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 81ef686..9ecb7b5 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -193,6 +193,7 @@ void machine_shutdown(void) void machine_halt(void) { local_irq_disable(); + preempt_disable(); smp_send_stop(); local_irq_disable(); @@ -208,6 +209,7 @@ void machine_halt(void) void machine_power_off(void) { local_irq_disable(); + preempt_disable(); smp_send_stop(); if (pm_power_off) @@ -228,6 +230,7 @@ void machine_power_off(void) void machine_restart(char *cmd) { local_irq_disable(); + preempt_disable(); smp_send_stop(); arm_pm_restart(reboot_mode, cmd);