Re: [PATCH] cpuidle: coupled: Enable interrupts when early returning on invalid driver
From: Daniel Lezcano
Date: Thu Jul 24 2014 - 06:27:39 EST
On 07/24/2014 11:46 AM, Krzysztof Kozlowski wrote:
On czw, 2014-07-24 at 11:35 +0200, Daniel Lezcano wrote:
On 07/24/2014 11:26 AM, Krzysztof Kozlowski wrote:
cpuidle_enter_state is expected to return with interrupts enabled.
However cpuidle_enter_state_coupled returned with interrupts disabled if
the cpuidle driver was registered without mask of coupled cpus.
Hi Krzysztof,
may be your patch is right but I would like to clarify one thing. This
is the cpuidle driver for exynos. AFAICT, the coupled states shouldn't
be enabled for this driver, so how did you face this issue ?
I faced this on cpuidle drivers on both Exynos and MSM. But these were
not mainline cpuidle drivers.
Ok, I think the root problem is the FLAG_COUPLED is set for the state
but without the cpumask. I don't think we should handle that in the
enter_couple function but in the registering of the device and fail to
register the driver/device if the informations are not properly set.
In this case, we know the sanity checks are done one time at the
registering time and we don't need to do that each time when entering in
the enter coupled function. Hence the "if (!coupled)" disappear and no
need to re-enable the irq.
This could be observed as a warning:
[ 1.613132] ------------[ cut here ]------------
[ 1.613244] WARNING: CPU: 0 PID: 0 at kernel/sched/idle.c:175 cpu_idle_loop+0x2dc/0x6d0()
[ 1.620268] Modules linked in:
[ 1.623311] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.16.0-rc4-00102-g7669ccdbf8af-dirty #81
[ 1.623619] dwmmc_exynos 12510000.mshc: 1 slots initialized
[ 1.624890] logger: created 256K log 'log_main'
[ 1.625483] logger: created 256K log 'log_events'
[ 1.626084] logger: created 256K log 'log_radio'
[ 1.626699] logger: created 256K log 'log_system'
[ 1.655960] [<c00167cc>] (unwind_backtrace) from [<c0012c28>] (show_stack+0x10/0x14)
[ 1.663679] [<c0012c28>] (show_stack) from [<c050ab00>] (dump_stack+0x70/0xbc)
[ 1.670883] [<c050ab00>] (dump_stack) from [<c0023ac8>] (warn_slowpath_common+0x68/0x8c)
[ 1.678954] [<c0023ac8>] (warn_slowpath_common) from [<c0023b08>] (warn_slowpath_null+0x1c/0x24)
[ 1.687720] [<c0023b08>] (warn_slowpath_null) from [<c006ddd4>] (cpu_idle_loop+0x2dc/0x6d0)
[ 1.696052] [<c006ddd4>] (cpu_idle_loop) from [<c006e1d4>] (cpupri_find+0x0/0xd4)
[ 1.703518] [<c006e1d4>] (cpupri_find) from [<c07cdd14>] (processor_id+0x0/0x2c)
[ 1.710917] ---[ end trace a85327313857296e ]---
Enable the interrupts also when early returning from
cpuidle_enter_state_coupled due to invalid coupled configuration.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
Fixes: 4126c0197bc8 ("cpuidle: add support for states that affect multiple cpus")
Cc: <stable@xxxxxxxxxxxxxxx>
---
drivers/cpuidle/coupled.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
index 73fe2f8d7f96..2b57c267e855 100644
--- a/drivers/cpuidle/coupled.c
+++ b/drivers/cpuidle/coupled.c
@@ -463,8 +463,10 @@ int cpuidle_enter_state_coupled(struct cpuidle_device *dev,
struct cpuidle_coupled *coupled = dev->coupled;
int w;
- if (!coupled)
+ if (!coupled) {
+ local_irq_enable();
return -EINVAL;
+ }
while (coupled->prevent) {
cpuidle_coupled_clear_pokes(dev->cpu);
--
<http://www.linaro.org/> Linaro.org â Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
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/