Re: [PATCH] cpuidle: coupled: Enable interrupts when early returning on invalid driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]