* [PATCH] cpuidle: coupled: Enable interrupts when early returning on invalid driver
@ 2014-07-24 9:26 Krzysztof Kozlowski
2014-07-24 9:35 ` Daniel Lezcano
0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Kozlowski @ 2014-07-24 9:26 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, linux-pm, linux-kernel
Cc: Colin Cross, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, Krzysztof Kozlowski, stable
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.
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@samsung.com>
Fixes: 4126c0197bc8 ("cpuidle: add support for states that affect multiple cpus")
Cc: <stable@vger.kernel.org>
---
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);
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] cpuidle: coupled: Enable interrupts when early returning on invalid driver
2014-07-24 9:26 [PATCH] cpuidle: coupled: Enable interrupts when early returning on invalid driver Krzysztof Kozlowski
@ 2014-07-24 9:35 ` Daniel Lezcano
2014-07-24 9:46 ` Krzysztof Kozlowski
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Lezcano @ 2014-07-24 9:35 UTC (permalink / raw)
To: Krzysztof Kozlowski, Rafael J. Wysocki, linux-pm, linux-kernel
Cc: Colin Cross, Kyungmin Park, Marek Szyprowski,
Bartlomiej Zolnierkiewicz, stable
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 ?
> 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@samsung.com>
> Fixes: 4126c0197bc8 ("cpuidle: add support for states that affect multiple cpus")
> Cc: <stable@vger.kernel.org>
> ---
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cpuidle: coupled: Enable interrupts when early returning on invalid driver
2014-07-24 9:35 ` Daniel Lezcano
@ 2014-07-24 9:46 ` Krzysztof Kozlowski
2014-07-24 10:27 ` Daniel Lezcano
0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Kozlowski @ 2014-07-24 9:46 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Colin Cross,
Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
stable
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.
Best regards,
Krzysztof
> > 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@samsung.com>
> > Fixes: 4126c0197bc8 ("cpuidle: add support for states that affect multiple cpus")
> > Cc: <stable@vger.kernel.org>
> > ---
> > 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);
> >
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cpuidle: coupled: Enable interrupts when early returning on invalid driver
2014-07-24 9:46 ` Krzysztof Kozlowski
@ 2014-07-24 10:27 ` Daniel Lezcano
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Lezcano @ 2014-07-24 10:27 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rafael J. Wysocki, linux-pm, linux-kernel, Colin Cross,
Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz,
stable
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@samsung.com>
>>> Fixes: 4126c0197bc8 ("cpuidle: add support for states that affect multiple cpus")
>>> Cc: <stable@vger.kernel.org>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-07-24 10:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-24 9:26 [PATCH] cpuidle: coupled: Enable interrupts when early returning on invalid driver Krzysztof Kozlowski
2014-07-24 9:35 ` Daniel Lezcano
2014-07-24 9:46 ` Krzysztof Kozlowski
2014-07-24 10:27 ` Daniel Lezcano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).