Re: [PATCH 1/3] cpuidle: coupled: disable interrupts after enteringsafe state

[Colin Cross]

On Wed, Aug 28, 2013 at 11:50 PM, Prashant Gaikwad <pgaikwad@xxxxxxxxxx> wrote:
> On Saturday 24 August 2013 01:15 AM, Colin Cross wrote:
>>
>> Calling cpuidle_enter_state is expected to return with interrupts
>> enabled, but interrupts must be disabled before starting the
>> ready loop synchronization stage.  Call local_irq_disable after
>> each call to cpuidle_enter_state for the safe state.
>>
>> CC: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Colin Cross <ccross@xxxxxxxxxxx>
>> ---
>>   drivers/cpuidle/coupled.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
>> index 2a297f8..db92bcb 100644
>> --- a/drivers/cpuidle/coupled.c
>> +++ b/drivers/cpuidle/coupled.c
>> @@ -460,6 +460,7 @@ int cpuidle_enter_state_coupled(struct cpuidle_device
>> *dev,
>>                 }
>>                 entered_state = cpuidle_enter_state(dev, drv,
>>                         dev->safe_state_index);
>> +               local_irq_disable();
>
>
> Colin,
>
> There is still some window where irq remains enabled after exiting safe
> state. It may introduce some corner case.
> Instead of this can we pass a parameter to cpuidle_enter_state indicating
> that irq has to be enabled or not after exit from idle state, which would be
> false when entering safe state from coupled idle.

It's fine for irqs to be enabled when exiting the safe state, in fact
on further inspection this patch isn't strictly necessary at all - we
always enable interrupts inside cpuidle_coupled_clear_pokes soon after
cpuidle_enter_state returns, and then disable them again.  It's
probably better to disable interrupts right after cpuidle_enter_state,
it makes sure interrupts are consistently disabled when calling
cpuidle_coupled_set_waiting, cpuidle_coupled_cpus_waiting and
cpuidle_coupled_clear_pokes, although that doesn't matter in the
current implementation.

Rafael, feel free to drop the stable annotation from this patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/