On Tue, Oct 6, 2009 at 1:34 PM, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 30 Sep 2009 23:21:33 +0200
> "Rafael J. Wysocki" <rjw@xxxxxxx> wrote:
>
>> On Wednesday 30 September 2009, Kevin Hilman wrote:
>> > In the case where cpuidle_idle_call() returns before changing state
>> > due to a need_resched(), it was returning with IRQs disabled.
>> >
>> > This patch ensures IRQs are (re)enabled before returning.
>>
>> Venki, any comments on this?
>
> Rigor mortis is setting in on this one.
>
> Venki's most recent linux-acpi email was on July 31.
>
>> > Reported-by: Hemanth V <hemanthv@xxxxxx>
>> > Signed-off-by: Kevin Hilman <khilman@xxxxxxxxxxxxxxxxxxx>
>> > ---
>> > drivers/cpuidle/cpuidle.c | 5 ++++-
>> > 1 files changed, 4 insertions(+), 1 deletions(-)
>> >
>> > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> > index ad41f19..12fdd39 100644
>> > --- a/drivers/cpuidle/cpuidle.c
>> > +++ b/drivers/cpuidle/cpuidle.c
>> > @@ -76,8 +76,11 @@ static void cpuidle_idle_call(void)
>> > #endif
>> > /* ask the governor for the next state */
>> > next_state = cpuidle_curr_governor->select(dev);
>> > - if (need_resched())
>> > + if (need_resched()) {
>> > + local_irq_enable();
>> > return;
>> > + }
>> > +
>> > target_state = &dev->states[next_state];
>> >
>> > /* enter the state and update stats */
>
> The patch seems correct to me. The code is hopelessly poorly
> documented as per usual, but other paths in that function, including
> the call to target_state->enter() (which devolves to default_idle) also
> enable interrupts.
>
> Kevin, the changelog is not good. It tells us what was wrong with the
> code but does not describe the user-visible effects of the bug.
The idle path assumes that the platform specific idle code returns
with interrupts enabled (although this too is undocumented AFAICT) and
on ARM we have a WARN_ON(!(irqs_disabled()) when returning from the
idle loop, so the user-visible effects were only a warning since
interrupts were eventually re-enabled later.
> I'm unable to find any bug report from Hemanth so I'm all in the dark.
>
> Your cc to linux-omap makes me suspect that <whatever the problem was>
> was exhibited on a non-x86 platform, under some circumstances. Perhaps
> that explains (for unknown reasons) why <whatever the problem was> was
> not observed on x86. But I'm totally in the dark and grasping for
> clues and have no way of determining (for example) whether we should
> backport the fix to earlier kernels.
>
> Please send along the additional information?
>
On x86, this same problem exists, but there is no WARN_ON() to detect
it. As on ARM, the interrupts are eventually re-enabled, so I'm not
sure of any actual bugs triggered by this. It's primarily a
correctness/consistency fix.
Thanks,
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
