Re: [PATCH] CPUidle: always return with interrupts enabled
* Andrew Morton a...@linux-foundation.org [2009-10-06 13:34]: Rigor mortis is setting in on this one. The patch seems correct to me. Can someone put this patch in now? The problem has also been reported on Marvell's Kirkwood platform (ARM) by a number of users and the patch fixes it. Tested-by: Martin Michlmayr t...@cyrius.com Please CC stable when you commit the patch since it also needs to go in for 2.6.31. Reference with the patch: http://patchwork.kernel.org/patch/50728/ -- Martin Michlmayr http://www.cyrius.com/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CPUidle: always return with interrupts enabled
On Fri, 16 Oct 2009 17:25:04 +0100 Martin Michlmayr t...@cyrius.com wrote: * Andrew Morton a...@linux-foundation.org [2009-10-06 13:34]: Rigor mortis is setting in on this one. The patch seems correct to me. Can someone put this patch in now? The problem has also been reported on Marvell's Kirkwood platform (ARM) by a number of users and the patch fixes it. Tested-by: Martin Michlmayr t...@cyrius.com I have it in my for-2.6.32 queue. Please CC stable when you commit the patch since it also needs to go in for 2.6.31. hm, I didn't know that. So noted, thanks. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CPUidle: always return with interrupts enabled
On Wed, 30 Sep 2009 23:21:33 +0200 Rafael J. Wysocki r...@sisk.pl 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 heman...@ti.com Signed-off-by: Kevin Hilman khil...@deeprootsystems.com --- 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. 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? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CPUidle: always return with interrupts enabled
On Tue, Oct 6, 2009 at 1:34 PM, Andrew Morton a...@linux-foundation.org wrote: On Wed, 30 Sep 2009 23:21:33 +0200 Rafael J. Wysocki r...@sisk.pl 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 heman...@ti.com Signed-off-by: Kevin Hilman khil...@deeprootsystems.com --- 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-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html