ARM: Disable preemption along with interrupts during shutdown

ARM: Disable preemption along with interrupts during shutdown

[arunks.linux at gmail.com]

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()

Signed-off-by: Arun KS <getarunks@gmail.com>
---
 arch/arm/kernel/process.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

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);
-- 
1.7.6

ARM: Disable preemption along with interrupts during shutdown

[Russell King - ARM Linux]

On Thu, May 08, 2014 at 06:53:31PM +0530, arunks.linux at 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;

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

ARM: Disable preemption along with interrupts during shutdown

[Arun KS]

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 at 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.

ARM: Disable preemption along with interrupts during shutdown

[Russell King - ARM Linux]

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.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

ARM: Disable preemption along with interrupts during shutdown

[Arun KS]

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.