* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-02-26 17:20 ` Gregory CLEMENT
0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-02-26 17:20 UTC (permalink / raw)
To: Daniel Lezcano, Rafael J. Wysocki, linux-pm
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Gregory CLEMENT, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
that cpu_pm_enter is not called twice on the same CPU before
cpu_pm_exit is called.". In the current code in case of failure when
calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
called whereas cpu_pm_enter() was called just before.
This patch moves the cpu_pm_exit() in order to balance the
cpu_pm_enter() calls.
Reported-by: Fulvio Benini <fbf@libero.it>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
index 38e68618513a..cefa07438ae1 100644
--- a/drivers/cpuidle/cpuidle-mvebu-v7.c
+++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
@@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
deepidle = true;
ret = mvebu_v7_cpu_suspend(deepidle);
+ cpu_pm_exit();
+
if (ret)
return ret;
- cpu_pm_exit();
-
return index;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-02-26 17:20 ` Gregory CLEMENT
0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-02-26 17:20 UTC (permalink / raw)
To: linux-arm-kernel
As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
that cpu_pm_enter is not called twice on the same CPU before
cpu_pm_exit is called.". In the current code in case of failure when
calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
called whereas cpu_pm_enter() was called just before.
This patch moves the cpu_pm_exit() in order to balance the
cpu_pm_enter() calls.
Reported-by: Fulvio Benini <fbf@libero.it>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
index 38e68618513a..cefa07438ae1 100644
--- a/drivers/cpuidle/cpuidle-mvebu-v7.c
+++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
@@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
deepidle = true;
ret = mvebu_v7_cpu_suspend(deepidle);
+ cpu_pm_exit();
+
if (ret)
return ret;
- cpu_pm_exit();
-
return index;
}
--
2.1.0
^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-02-26 17:20 ` Gregory CLEMENT
@ 2015-02-26 21:55 ` Rafael J. Wysocki
-1 siblings, 0 replies; 30+ messages in thread
From: Rafael J. Wysocki @ 2015-02-26 21:55 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Daniel Lezcano, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
> that cpu_pm_enter is not called twice on the same CPU before
> cpu_pm_exit is called.". In the current code in case of failure when
> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
> called whereas cpu_pm_enter() was called just before.
>
> This patch moves the cpu_pm_exit() in order to balance the
> cpu_pm_enter() calls.
>
> Reported-by: Fulvio Benini <fbf@libero.it>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Should that go to "stable" too? Which "stable" series it should go to if so?
> ---
> drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
> index 38e68618513a..cefa07438ae1 100644
> --- a/drivers/cpuidle/cpuidle-mvebu-v7.c
> +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
> @@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
> deepidle = true;
>
> ret = mvebu_v7_cpu_suspend(deepidle);
> + cpu_pm_exit();
> +
> if (ret)
> return ret;
>
> - cpu_pm_exit();
> -
> return index;
> }
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-02-26 21:55 ` Rafael J. Wysocki
0 siblings, 0 replies; 30+ messages in thread
From: Rafael J. Wysocki @ 2015-02-26 21:55 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
> that cpu_pm_enter is not called twice on the same CPU before
> cpu_pm_exit is called.". In the current code in case of failure when
> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
> called whereas cpu_pm_enter() was called just before.
>
> This patch moves the cpu_pm_exit() in order to balance the
> cpu_pm_enter() calls.
>
> Reported-by: Fulvio Benini <fbf@libero.it>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Should that go to "stable" too? Which "stable" series it should go to if so?
> ---
> drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
> index 38e68618513a..cefa07438ae1 100644
> --- a/drivers/cpuidle/cpuidle-mvebu-v7.c
> +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
> @@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
> deepidle = true;
>
> ret = mvebu_v7_cpu_suspend(deepidle);
> + cpu_pm_exit();
> +
> if (ret)
> return ret;
>
> - cpu_pm_exit();
> -
> return index;
> }
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-02-26 21:55 ` Rafael J. Wysocki
@ 2015-02-27 9:39 ` Gregory CLEMENT
-1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-02-27 9:39 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Daniel Lezcano, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
Hi Rafael,
On 26/02/2015 22:55, Rafael J. Wysocki wrote:
> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>> that cpu_pm_enter is not called twice on the same CPU before
>> cpu_pm_exit is called.". In the current code in case of failure when
>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>> called whereas cpu_pm_enter() was called just before.
>>
>> This patch moves the cpu_pm_exit() in order to balance the
>> cpu_pm_enter() calls.
>>
>> Reported-by: Fulvio Benini <fbf@libero.it>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> Should that go to "stable" too? Which "stable" series it should go to if so?
Yes as it fixes a potential issue, you're right it should go
to "stable". The bug was here since the introduction of the driver
in 3.16.
Thanks,
Gregory
>
>> ---
>> drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
>> index 38e68618513a..cefa07438ae1 100644
>> --- a/drivers/cpuidle/cpuidle-mvebu-v7.c
>> +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
>> @@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
>> deepidle = true;
>>
>> ret = mvebu_v7_cpu_suspend(deepidle);
>> + cpu_pm_exit();
>> +
>> if (ret)
>> return ret;
>>
>> - cpu_pm_exit();
>> -
>> return index;
>> }
>>
>>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-02-27 9:39 ` Gregory CLEMENT
0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-02-27 9:39 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rafael,
On 26/02/2015 22:55, Rafael J. Wysocki wrote:
> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>> that cpu_pm_enter is not called twice on the same CPU before
>> cpu_pm_exit is called.". In the current code in case of failure when
>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>> called whereas cpu_pm_enter() was called just before.
>>
>> This patch moves the cpu_pm_exit() in order to balance the
>> cpu_pm_enter() calls.
>>
>> Reported-by: Fulvio Benini <fbf@libero.it>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> Should that go to "stable" too? Which "stable" series it should go to if so?
Yes as it fixes a potential issue, you're right it should go
to "stable". The bug was here since the introduction of the driver
in 3.16.
Thanks,
Gregory
>
>> ---
>> drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
>> index 38e68618513a..cefa07438ae1 100644
>> --- a/drivers/cpuidle/cpuidle-mvebu-v7.c
>> +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
>> @@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
>> deepidle = true;
>>
>> ret = mvebu_v7_cpu_suspend(deepidle);
>> + cpu_pm_exit();
>> +
>> if (ret)
>> return ret;
>>
>> - cpu_pm_exit();
>> -
>> return index;
>> }
>>
>>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-02-26 21:55 ` Rafael J. Wysocki
@ 2015-02-27 16:50 ` Daniel Lezcano
-1 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-02-27 16:50 UTC (permalink / raw)
To: Rafael J. Wysocki, Gregory CLEMENT
Cc: linux-pm, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Thomas Petazzoni, Ezequiel Garcia, linux-arm-kernel,
Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk,
Nadav Haklai
On 02/26/2015 10:55 PM, Rafael J. Wysocki wrote:
> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>> that cpu_pm_enter is not called twice on the same CPU before
>> cpu_pm_exit is called.". In the current code in case of failure when
>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>> called whereas cpu_pm_enter() was called just before.
>>
>> This patch moves the cpu_pm_exit() in order to balance the
>> cpu_pm_enter() calls.
>>
>> Reported-by: Fulvio Benini <fbf@libero.it>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> Should that go to "stable" too? Which "stable" series it should go to if so?
Hi Rafael,
do you mind if I take this fix in my tree ? There is the same issue for
cpuidle-arm64.c I fixed.
--
<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] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-02-27 16:50 ` Daniel Lezcano
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-02-27 16:50 UTC (permalink / raw)
To: linux-arm-kernel
On 02/26/2015 10:55 PM, Rafael J. Wysocki wrote:
> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>> that cpu_pm_enter is not called twice on the same CPU before
>> cpu_pm_exit is called.". In the current code in case of failure when
>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>> called whereas cpu_pm_enter() was called just before.
>>
>> This patch moves the cpu_pm_exit() in order to balance the
>> cpu_pm_enter() calls.
>>
>> Reported-by: Fulvio Benini <fbf@libero.it>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> Should that go to "stable" too? Which "stable" series it should go to if so?
Hi Rafael,
do you mind if I take this fix in my tree ? There is the same issue for
cpuidle-arm64.c I fixed.
--
<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] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-02-27 16:50 ` Daniel Lezcano
@ 2015-02-27 22:18 ` Rafael J. Wysocki
-1 siblings, 0 replies; 30+ messages in thread
From: Rafael J. Wysocki @ 2015-02-27 22:18 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Gregory CLEMENT, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
On Friday, February 27, 2015 05:50:23 PM Daniel Lezcano wrote:
> On 02/26/2015 10:55 PM, Rafael J. Wysocki wrote:
> > On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
> >> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
> >> that cpu_pm_enter is not called twice on the same CPU before
> >> cpu_pm_exit is called.". In the current code in case of failure when
> >> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
> >> called whereas cpu_pm_enter() was called just before.
> >>
> >> This patch moves the cpu_pm_exit() in order to balance the
> >> cpu_pm_enter() calls.
> >>
> >> Reported-by: Fulvio Benini <fbf@libero.it>
> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >
> > Should that go to "stable" too? Which "stable" series it should go to if so?
>
> Hi Rafael,
>
> do you mind if I take this fix in my tree ?
Not at all, please go ahead.
> There is the same issue for cpuidle-arm64.c I fixed.
OK
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-02-27 22:18 ` Rafael J. Wysocki
0 siblings, 0 replies; 30+ messages in thread
From: Rafael J. Wysocki @ 2015-02-27 22:18 UTC (permalink / raw)
To: linux-arm-kernel
On Friday, February 27, 2015 05:50:23 PM Daniel Lezcano wrote:
> On 02/26/2015 10:55 PM, Rafael J. Wysocki wrote:
> > On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
> >> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
> >> that cpu_pm_enter is not called twice on the same CPU before
> >> cpu_pm_exit is called.". In the current code in case of failure when
> >> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
> >> called whereas cpu_pm_enter() was called just before.
> >>
> >> This patch moves the cpu_pm_exit() in order to balance the
> >> cpu_pm_enter() calls.
> >>
> >> Reported-by: Fulvio Benini <fbf@libero.it>
> >> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> >
> > Should that go to "stable" too? Which "stable" series it should go to if so?
>
> Hi Rafael,
>
> do you mind if I take this fix in my tree ?
Not at all, please go ahead.
> There is the same issue for cpuidle-arm64.c I fixed.
OK
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-02-27 9:39 ` Gregory CLEMENT
@ 2015-03-03 10:30 ` Daniel Lezcano
-1 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-03 10:30 UTC (permalink / raw)
To: Gregory CLEMENT, Rafael J. Wysocki
Cc: linux-pm, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
Thomas Petazzoni, Ezequiel Garcia, linux-arm-kernel,
Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk,
Nadav Haklai, fbf
On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
> Hi Rafael,
>
> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>> that cpu_pm_enter is not called twice on the same CPU before
>>> cpu_pm_exit is called.". In the current code in case of failure when
>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>> called whereas cpu_pm_enter() was called just before.
>>>
>>> This patch moves the cpu_pm_exit() in order to balance the
>>> cpu_pm_enter() calls.
>>>
>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>> Should that go to "stable" too? Which "stable" series it should go to if so?
>
> Yes as it fixes a potential issue, you're right it should go
> to "stable". The bug was here since the introduction of the driver
> in 3.16.
Hi Gregory,
actually the 'stable' rules state clearly:
"- It must fix a real bug that bothers people (not a, "This could be a
problem..." type thing)."
You say "it fixes a potential issue", so no bug has been raise yet, right ?
>>> ---
>>> drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
>>> index 38e68618513a..cefa07438ae1 100644
>>> --- a/drivers/cpuidle/cpuidle-mvebu-v7.c
>>> +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
>>> @@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
>>> deepidle = true;
>>>
>>> ret = mvebu_v7_cpu_suspend(deepidle);
>>> + cpu_pm_exit();
>>> +
>>> if (ret)
>>> return ret;
>>>
>>> - cpu_pm_exit();
>>> -
>>> return index;
>>> }
>>>
>>>
>>
>
>
--
<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] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-03 10:30 ` Daniel Lezcano
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-03 10:30 UTC (permalink / raw)
To: linux-arm-kernel
On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
> Hi Rafael,
>
> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>> that cpu_pm_enter is not called twice on the same CPU before
>>> cpu_pm_exit is called.". In the current code in case of failure when
>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>> called whereas cpu_pm_enter() was called just before.
>>>
>>> This patch moves the cpu_pm_exit() in order to balance the
>>> cpu_pm_enter() calls.
>>>
>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>
>> Should that go to "stable" too? Which "stable" series it should go to if so?
>
> Yes as it fixes a potential issue, you're right it should go
> to "stable". The bug was here since the introduction of the driver
> in 3.16.
Hi Gregory,
actually the 'stable' rules state clearly:
"- It must fix a real bug that bothers people (not a, "This could be a
problem..." type thing)."
You say "it fixes a potential issue", so no bug has been raise yet, right ?
>>> ---
>>> drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
>>> index 38e68618513a..cefa07438ae1 100644
>>> --- a/drivers/cpuidle/cpuidle-mvebu-v7.c
>>> +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
>>> @@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
>>> deepidle = true;
>>>
>>> ret = mvebu_v7_cpu_suspend(deepidle);
>>> + cpu_pm_exit();
>>> +
>>> if (ret)
>>> return ret;
>>>
>>> - cpu_pm_exit();
>>> -
>>> return index;
>>> }
>>>
>>>
>>
>
>
--
<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] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-03 10:30 ` Daniel Lezcano
@ 2015-03-03 10:34 ` Gregory CLEMENT
-1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-03-03 10:34 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Daniel Lezcano, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai, fbf
Hi Rafael,
On 03/03/2015 11:30, Daniel Lezcano wrote:
> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>> Hi Rafael,
>>
>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>> called whereas cpu_pm_enter() was called just before.
>>>>
>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>> cpu_pm_enter() calls.
>>>>
>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>
>>> Should that go to "stable" too? Which "stable" series it should go to if so?
>>
>> Yes as it fixes a potential issue, you're right it should go
>> to "stable". The bug was here since the introduction of the driver
>> in 3.16.
>
> Hi Gregory,
>
> actually the 'stable' rules state clearly:
>
> "- It must fix a real bug that bothers people (not a, "This could be a
> problem..." type thing)."
>
> You say "it fixes a potential issue", so no bug has been raise yet, right ?
Indeed nobody claimed yet having a bug related to this issue.
Gregory
>
>>>> ---
>>>> drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
>>>> index 38e68618513a..cefa07438ae1 100644
>>>> --- a/drivers/cpuidle/cpuidle-mvebu-v7.c
>>>> +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
>>>> @@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
>>>> deepidle = true;
>>>>
>>>> ret = mvebu_v7_cpu_suspend(deepidle);
>>>> + cpu_pm_exit();
>>>> +
>>>> if (ret)
>>>> return ret;
>>>>
>>>> - cpu_pm_exit();
>>>> -
>>>> return index;
>>>> }
>>>>
>>>>
>>>
>>
>>
>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-03 10:34 ` Gregory CLEMENT
0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-03-03 10:34 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rafael,
On 03/03/2015 11:30, Daniel Lezcano wrote:
> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>> Hi Rafael,
>>
>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>> called whereas cpu_pm_enter() was called just before.
>>>>
>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>> cpu_pm_enter() calls.
>>>>
>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>
>>> Should that go to "stable" too? Which "stable" series it should go to if so?
>>
>> Yes as it fixes a potential issue, you're right it should go
>> to "stable". The bug was here since the introduction of the driver
>> in 3.16.
>
> Hi Gregory,
>
> actually the 'stable' rules state clearly:
>
> "- It must fix a real bug that bothers people (not a, "This could be a
> problem..." type thing)."
>
> You say "it fixes a potential issue", so no bug has been raise yet, right ?
Indeed nobody claimed yet having a bug related to this issue.
Gregory
>
>>>> ---
>>>> drivers/cpuidle/cpuidle-mvebu-v7.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/cpuidle/cpuidle-mvebu-v7.c b/drivers/cpuidle/cpuidle-mvebu-v7.c
>>>> index 38e68618513a..cefa07438ae1 100644
>>>> --- a/drivers/cpuidle/cpuidle-mvebu-v7.c
>>>> +++ b/drivers/cpuidle/cpuidle-mvebu-v7.c
>>>> @@ -37,11 +37,11 @@ static int mvebu_v7_enter_idle(struct cpuidle_device *dev,
>>>> deepidle = true;
>>>>
>>>> ret = mvebu_v7_cpu_suspend(deepidle);
>>>> + cpu_pm_exit();
>>>> +
>>>> if (ret)
>>>> return ret;
>>>>
>>>> - cpu_pm_exit();
>>>> -
>>>> return index;
>>>> }
>>>>
>>>>
>>>
>>
>>
>
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-03 10:34 ` Gregory CLEMENT
@ 2015-03-03 10:52 ` Fulvio
-1 siblings, 0 replies; 30+ messages in thread
From: Fulvio @ 2015-03-03 10:52 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Thomas Petazzoni, Andrew Lunn, Jason Cooper, Tawfik Bayouk,
linux-pm, Boris BREZILLON, Daniel Lezcano, Rafael J. Wysocki,
Nadav Haklai, Lior Amsalem, Ezequiel Garcia, Maxime Ripard,
linux-arm-kernel, Sebastian Hesselbarth
Gregory CLEMENT wrote:
> Hi Rafael,
>
>
>
> On 03/03/2015 11:30, Daniel Lezcano wrote:
>
>> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>>
>>> Hi Rafael,
>>>
>>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>>
>>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>>
>>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>>> called whereas cpu_pm_enter() was called just before.
>>>>>
>>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>>> cpu_pm_enter() calls.
>>>>>
>>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>>
>>>> Should that go to "stable" too? Which "stable" series it should go to if so?
>>>>
>>> Yes as it fixes a potential issue, you're right it should go
>>> to "stable". The bug was here since the introduction of the driver
>>> in 3.16.
>>>
>> Hi Gregory,
>>
>> actually the 'stable' rules state clearly:
>>
>> "- It must fix a real bug that bothers people (not a, "This could be a
>> problem..." type thing)."
>>
>> You say "it fixes a potential issue", so no bug has been raise yet, right ?
>>
>
> Indeed nobody claimed yet having a bug related to this issue.
>
> Gregory
>
>
I reported the issue, but i cannot say if it's a real bug.
I had random kernel panics with a Netgear ReadyNAS RN102 (armada 370 cpu):
http://www.readynas.com/forum/viewtopic.php?f=20&t=78697&sid=14747617286d55ac27296cdee7a3f420&start=210#p452214
All i can say is that the system use the "armadaxp_idle" driver and
works fine when running "stress --cpu 8" in background.
I asked Netgear to provide a firmware without the idle driver to confirm
if it's the cause of the problem, but they did not answered.
Bye,
Fulvio
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-03 10:52 ` Fulvio
0 siblings, 0 replies; 30+ messages in thread
From: Fulvio @ 2015-03-03 10:52 UTC (permalink / raw)
To: linux-arm-kernel
Gregory CLEMENT wrote:
> Hi Rafael,
>
>
>
> On 03/03/2015 11:30, Daniel Lezcano wrote:
>
>> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>>
>>> Hi Rafael,
>>>
>>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>>
>>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>>
>>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>>> called whereas cpu_pm_enter() was called just before.
>>>>>
>>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>>> cpu_pm_enter() calls.
>>>>>
>>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>>
>>>> Should that go to "stable" too? Which "stable" series it should go to if so?
>>>>
>>> Yes as it fixes a potential issue, you're right it should go
>>> to "stable". The bug was here since the introduction of the driver
>>> in 3.16.
>>>
>> Hi Gregory,
>>
>> actually the 'stable' rules state clearly:
>>
>> "- It must fix a real bug that bothers people (not a, "This could be a
>> problem..." type thing)."
>>
>> You say "it fixes a potential issue", so no bug has been raise yet, right ?
>>
>
> Indeed nobody claimed yet having a bug related to this issue.
>
> Gregory
>
>
I reported the issue, but i cannot say if it's a real bug.
I had random kernel panics with a Netgear ReadyNAS RN102 (armada 370 cpu):
http://www.readynas.com/forum/viewtopic.php?f=20&t=78697&sid=14747617286d55ac27296cdee7a3f420&start=210#p452214
All i can say is that the system use the "armadaxp_idle" driver and
works fine when running "stress --cpu 8" in background.
I asked Netgear to provide a firmware without the idle driver to confirm
if it's the cause of the problem, but they did not answered.
Bye,
Fulvio
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-03 10:52 ` Fulvio
@ 2015-03-03 11:12 ` Daniel Lezcano
-1 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-03 11:12 UTC (permalink / raw)
To: Fulvio, Gregory CLEMENT
Cc: Rafael J. Wysocki, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
On 03/03/2015 11:52 AM, Fulvio wrote:
> Gregory CLEMENT wrote:
>> Hi Rafael,
>>
>>
>>
>> On 03/03/2015 11:30, Daniel Lezcano wrote:
>>> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>>>> Hi Rafael,
>>>>
>>>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>>>> called whereas cpu_pm_enter() was called just before.
>>>>>>
>>>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>>>> cpu_pm_enter() calls.
>>>>>>
>>>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>> Should that go to "stable" too? Which "stable" series it should go
>>>>> to if so?
>>>> Yes as it fixes a potential issue, you're right it should go
>>>> to "stable". The bug was here since the introduction of the driver
>>>> in 3.16.
>>> Hi Gregory,
>>>
>>> actually the 'stable' rules state clearly:
>>>
>>> "- It must fix a real bug that bothers people (not a, "This could be
>>> a problem..." type thing)."
>>>
>>> You say "it fixes a potential issue", so no bug has been raise yet,
>>> right ?
>>
>> Indeed nobody claimed yet having a bug related to this issue.
>>
>> Gregory
>>
> I reported the issue, but i cannot say if it's a real bug.
> I had random kernel panics with a Netgear ReadyNAS RN102 (armada 370 cpu):
> http://www.readynas.com/forum/viewtopic.php?f=20&t=78697&sid=14747617286d55ac27296cdee7a3f420&start=210#p452214
>
>
> All i can say is that the system use the "armadaxp_idle" driver and
> works fine when running "stress --cpu 8" in background.
Hi Fulvio,
so IIUC, you suggest the stress test prevent the system to use cpuidle
because of the busy cycles, right ?
Is it possible to have the kernel panic stack ?
Are you able to reproduce the kernel panic ? and test a new kernel ?
Thanks
-- Daniel
> I asked Netgear to provide a firmware without the idle driver to confirm
> if it's the cause of the problem, but they did not answered.
> Bye,
> Fulvio
--
<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] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-03 11:12 ` Daniel Lezcano
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-03 11:12 UTC (permalink / raw)
To: linux-arm-kernel
On 03/03/2015 11:52 AM, Fulvio wrote:
> Gregory CLEMENT wrote:
>> Hi Rafael,
>>
>>
>>
>> On 03/03/2015 11:30, Daniel Lezcano wrote:
>>> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>>>> Hi Rafael,
>>>>
>>>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>>>> called whereas cpu_pm_enter() was called just before.
>>>>>>
>>>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>>>> cpu_pm_enter() calls.
>>>>>>
>>>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>> Should that go to "stable" too? Which "stable" series it should go
>>>>> to if so?
>>>> Yes as it fixes a potential issue, you're right it should go
>>>> to "stable". The bug was here since the introduction of the driver
>>>> in 3.16.
>>> Hi Gregory,
>>>
>>> actually the 'stable' rules state clearly:
>>>
>>> "- It must fix a real bug that bothers people (not a, "This could be
>>> a problem..." type thing)."
>>>
>>> You say "it fixes a potential issue", so no bug has been raise yet,
>>> right ?
>>
>> Indeed nobody claimed yet having a bug related to this issue.
>>
>> Gregory
>>
> I reported the issue, but i cannot say if it's a real bug.
> I had random kernel panics with a Netgear ReadyNAS RN102 (armada 370 cpu):
> http://www.readynas.com/forum/viewtopic.php?f=20&t=78697&sid=14747617286d55ac27296cdee7a3f420&start=210#p452214
>
>
> All i can say is that the system use the "armadaxp_idle" driver and
> works fine when running "stress --cpu 8" in background.
Hi Fulvio,
so IIUC, you suggest the stress test prevent the system to use cpuidle
because of the busy cycles, right ?
Is it possible to have the kernel panic stack ?
Are you able to reproduce the kernel panic ? and test a new kernel ?
Thanks
-- Daniel
> I asked Netgear to provide a firmware without the idle driver to confirm
> if it's the cause of the problem, but they did not answered.
> Bye,
> Fulvio
--
<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] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-03 10:52 ` Fulvio
@ 2015-03-03 12:51 ` Gregory CLEMENT
-1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-03-03 12:51 UTC (permalink / raw)
To: Fulvio
Cc: Rafael J. Wysocki, Daniel Lezcano, linux-pm, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Thomas Petazzoni,
Ezequiel Garcia, linux-arm-kernel, Maxime Ripard,
Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai
Hi Fulvio,
On 03/03/2015 11:52, Fulvio wrote:
> Gregory CLEMENT wrote:
>> Hi Rafael,
>>
>>
>>
>> On 03/03/2015 11:30, Daniel Lezcano wrote:
>>
>>> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>>>
>>>> Hi Rafael,
>>>>
>>>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>>>
>>>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>>>
>>>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>>>> called whereas cpu_pm_enter() was called just before.
>>>>>>
>>>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>>>> cpu_pm_enter() calls.
>>>>>>
>>>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>>>
>>>>> Should that go to "stable" too? Which "stable" series it should go to if so?
>>>>>
>>>> Yes as it fixes a potential issue, you're right it should go
>>>> to "stable". The bug was here since the introduction of the driver
>>>> in 3.16.
>>>>
>>> Hi Gregory,
>>>
>>> actually the 'stable' rules state clearly:
>>>
>>> "- It must fix a real bug that bothers people (not a, "This could be a
>>> problem..." type thing)."
>>>
>>> You say "it fixes a potential issue", so no bug has been raise yet, right ?
>>>
>>
>> Indeed nobody claimed yet having a bug related to this issue.
>>
>> Gregory
>>
>>
> I reported the issue, but i cannot say if it's a real bug.
> I had random kernel panics with a Netgear ReadyNAS RN102 (armada 370 cpu):
> http://www.readynas.com/forum/viewtopic.php?f=20&t=78697&sid=14747617286d55ac27296cdee7a3f420&start=210#p452214
I didn't know you experimented random kernel panics and that you thought
it was related to the CPU Idle driver.
>
> All i can say is that the system use the "armadaxp_idle" driver and
> works fine when running "stress --cpu 8" in background.
> I asked Netgear to provide a firmware without the idle driver to confirm
> if it's the cause of the problem, but they did not answered.
I think that if you disable all the state using by doing an
echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
where NUM is the nuymbert of the state. Then it should disable the
cpuidle on the fly.
Daniel, is it correct?
Or if you can modify your bootargs, then just add cpuidle.off=1 to the kernel
command line.
Thanks,
Gregory
> Bye,
> Fulvio
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-03 12:51 ` Gregory CLEMENT
0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-03-03 12:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi Fulvio,
On 03/03/2015 11:52, Fulvio wrote:
> Gregory CLEMENT wrote:
>> Hi Rafael,
>>
>>
>>
>> On 03/03/2015 11:30, Daniel Lezcano wrote:
>>
>>> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>>>
>>>> Hi Rafael,
>>>>
>>>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>>>
>>>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>>>
>>>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>>>> called whereas cpu_pm_enter() was called just before.
>>>>>>
>>>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>>>> cpu_pm_enter() calls.
>>>>>>
>>>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>>>
>>>>> Should that go to "stable" too? Which "stable" series it should go to if so?
>>>>>
>>>> Yes as it fixes a potential issue, you're right it should go
>>>> to "stable". The bug was here since the introduction of the driver
>>>> in 3.16.
>>>>
>>> Hi Gregory,
>>>
>>> actually the 'stable' rules state clearly:
>>>
>>> "- It must fix a real bug that bothers people (not a, "This could be a
>>> problem..." type thing)."
>>>
>>> You say "it fixes a potential issue", so no bug has been raise yet, right ?
>>>
>>
>> Indeed nobody claimed yet having a bug related to this issue.
>>
>> Gregory
>>
>>
> I reported the issue, but i cannot say if it's a real bug.
> I had random kernel panics with a Netgear ReadyNAS RN102 (armada 370 cpu):
> http://www.readynas.com/forum/viewtopic.php?f=20&t=78697&sid=14747617286d55ac27296cdee7a3f420&start=210#p452214
I didn't know you experimented random kernel panics and that you thought
it was related to the CPU Idle driver.
>
> All i can say is that the system use the "armadaxp_idle" driver and
> works fine when running "stress --cpu 8" in background.
> I asked Netgear to provide a firmware without the idle driver to confirm
> if it's the cause of the problem, but they did not answered.
I think that if you disable all the state using by doing an
echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
where NUM is the nuymbert of the state. Then it should disable the
cpuidle on the fly.
Daniel, is it correct?
Or if you can modify your bootargs, then just add cpuidle.off=1 to the kernel
command line.
Thanks,
Gregory
> Bye,
> Fulvio
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-03 12:51 ` Gregory CLEMENT
@ 2015-03-03 13:00 ` Daniel Lezcano
-1 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-03 13:00 UTC (permalink / raw)
To: Gregory CLEMENT, Fulvio
Cc: Rafael J. Wysocki, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
On 03/03/2015 01:51 PM, Gregory CLEMENT wrote:
> Hi Fulvio,
>
> On 03/03/2015 11:52, Fulvio wrote:
>> Gregory CLEMENT wrote:
>>> Hi Rafael,
>>>
>>>
>>>
>>> On 03/03/2015 11:30, Daniel Lezcano wrote:
>>>
>>>> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>>>>
>>>>> Hi Rafael,
>>>>>
>>>>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>>>>
>>>>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>>>>
>>>>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>>>>> called whereas cpu_pm_enter() was called just before.
>>>>>>>
>>>>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>>>>> cpu_pm_enter() calls.
>>>>>>>
>>>>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>>>>
>>>>>> Should that go to "stable" too? Which "stable" series it should go to if so?
>>>>>>
>>>>> Yes as it fixes a potential issue, you're right it should go
>>>>> to "stable". The bug was here since the introduction of the driver
>>>>> in 3.16.
>>>>>
>>>> Hi Gregory,
>>>>
>>>> actually the 'stable' rules state clearly:
>>>>
>>>> "- It must fix a real bug that bothers people (not a, "This could be a
>>>> problem..." type thing)."
>>>>
>>>> You say "it fixes a potential issue", so no bug has been raise yet, right ?
>>>>
>>>
>>> Indeed nobody claimed yet having a bug related to this issue.
>>>
>>> Gregory
>>>
>>>
>> I reported the issue, but i cannot say if it's a real bug.
>> I had random kernel panics with a Netgear ReadyNAS RN102 (armada 370 cpu):
>> http://www.readynas.com/forum/viewtopic.php?f=20&t=78697&sid=14747617286d55ac27296cdee7a3f420&start=210#p452214
>
> I didn't know you experimented random kernel panics and that you thought
> it was related to the CPU Idle driver.
>
>>
>> All i can say is that the system use the "armadaxp_idle" driver and
>> works fine when running "stress --cpu 8" in background.
>> I asked Netgear to provide a firmware without the idle driver to confirm
>> if it's the cause of the problem, but they did not answered.
>
> I think that if you disable all the state using by doing an
>
> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
>
> where NUM is the nuymbert of the state. Then it should disable the
> cpuidle on the fly.
>
> Daniel, is it correct?
Yes, it is correct.
Make sure it is done on all cpus.
> Or if you can modify your bootargs, then just add cpuidle.off=1 to the kernel
> command line.
>
> Thanks,
>
> Gregory
>
>> Bye,
>> Fulvio
>>
>
>
--
<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] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-03 13:00 ` Daniel Lezcano
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-03 13:00 UTC (permalink / raw)
To: linux-arm-kernel
On 03/03/2015 01:51 PM, Gregory CLEMENT wrote:
> Hi Fulvio,
>
> On 03/03/2015 11:52, Fulvio wrote:
>> Gregory CLEMENT wrote:
>>> Hi Rafael,
>>>
>>>
>>>
>>> On 03/03/2015 11:30, Daniel Lezcano wrote:
>>>
>>>> On 02/27/2015 10:39 AM, Gregory CLEMENT wrote:
>>>>
>>>>> Hi Rafael,
>>>>>
>>>>> On 26/02/2015 22:55, Rafael J. Wysocki wrote:
>>>>>
>>>>>> On Thursday, February 26, 2015 06:20:48 PM Gregory CLEMENT wrote:
>>>>>>
>>>>>>> As stated in kernel/cpu_pm.c, "Platform is responsible for ensuring
>>>>>>> that cpu_pm_enter is not called twice on the same CPU before
>>>>>>> cpu_pm_exit is called.". In the current code in case of failure when
>>>>>>> calling mvebu_v7_cpu_suspend, the function cpu_pm_exit() is never
>>>>>>> called whereas cpu_pm_enter() was called just before.
>>>>>>>
>>>>>>> This patch moves the cpu_pm_exit() in order to balance the
>>>>>>> cpu_pm_enter() calls.
>>>>>>>
>>>>>>> Reported-by: Fulvio Benini <fbf@libero.it>
>>>>>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>>>>>>>
>>>>>> Should that go to "stable" too? Which "stable" series it should go to if so?
>>>>>>
>>>>> Yes as it fixes a potential issue, you're right it should go
>>>>> to "stable". The bug was here since the introduction of the driver
>>>>> in 3.16.
>>>>>
>>>> Hi Gregory,
>>>>
>>>> actually the 'stable' rules state clearly:
>>>>
>>>> "- It must fix a real bug that bothers people (not a, "This could be a
>>>> problem..." type thing)."
>>>>
>>>> You say "it fixes a potential issue", so no bug has been raise yet, right ?
>>>>
>>>
>>> Indeed nobody claimed yet having a bug related to this issue.
>>>
>>> Gregory
>>>
>>>
>> I reported the issue, but i cannot say if it's a real bug.
>> I had random kernel panics with a Netgear ReadyNAS RN102 (armada 370 cpu):
>> http://www.readynas.com/forum/viewtopic.php?f=20&t=78697&sid=14747617286d55ac27296cdee7a3f420&start=210#p452214
>
> I didn't know you experimented random kernel panics and that you thought
> it was related to the CPU Idle driver.
>
>>
>> All i can say is that the system use the "armadaxp_idle" driver and
>> works fine when running "stress --cpu 8" in background.
>> I asked Netgear to provide a firmware without the idle driver to confirm
>> if it's the cause of the problem, but they did not answered.
>
> I think that if you disable all the state using by doing an
>
> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
>
> where NUM is the nuymbert of the state. Then it should disable the
> cpuidle on the fly.
>
> Daniel, is it correct?
Yes, it is correct.
Make sure it is done on all cpus.
> Or if you can modify your bootargs, then just add cpuidle.off=1 to the kernel
> command line.
>
> Thanks,
>
> Gregory
>
>> Bye,
>> Fulvio
>>
>
>
--
<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] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-03 12:51 ` Gregory CLEMENT
@ 2015-03-03 14:58 ` Fulvio
-1 siblings, 0 replies; 30+ messages in thread
From: Fulvio @ 2015-03-03 14:58 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Rafael J. Wysocki, Daniel Lezcano, linux-pm, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Thomas Petazzoni,
Ezequiel Garcia, linux-arm-kernel, Maxime Ripard,
Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai
> I didn't know you experimented random kernel panics and that you thought
> it was related to the CPU Idle driver.
>
>
>> All i can say is that the system use the "armadaxp_idle" driver and
>> works fine when running "stress --cpu 8" in background.
>> I asked Netgear to provide a firmware without the idle driver to confirm
>> if it's the cause of the problem, but they did not answered.
>>
>
> I think that if you disable all the state using by doing an
>
> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
>
> where NUM is the nuymbert of the state. Then it should disable the
> cpuidle on the fly.
>
Thanks, i'll try that as soon as possible (i gave back the unit to my
client) and report back.
However, the description of the cpu_pm_enter function state:
"Must be called on the affected CPU with interrupts disabled. Platform
is responsible for ensuring that cpu_pm_enter is not called twice on the
same CPU before cpu_pm_exit is called. Notified drivers can include VFP
co-processor, interrupt controller and its PM extensions, local CPU
timers context save/restore which shouldn't be interrupted. Hence it
must be called with interrupts disabled."
and the point is: it that an invariant? Do current code and future code
safely assume that cpu_pm_enter is not called twice?
For example if cpu_pm_enter do "context save" and cpu_pm_exit do
"context restore", calling twice cpu_pm_enter will overwrite the
previous saved context: is that safe in all circumstances?
I assume the rule " It must fix a real bug that bothers people (not a,
"This could be a problem..." type thing)." is to avoid committing
useless changes that may introduce new bugs, but i do not think that
apply to this case: a bug report from an unknown user (me) should change
nothing.
Bye,
Fulvio
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-03 14:58 ` Fulvio
0 siblings, 0 replies; 30+ messages in thread
From: Fulvio @ 2015-03-03 14:58 UTC (permalink / raw)
To: linux-arm-kernel
> I didn't know you experimented random kernel panics and that you thought
> it was related to the CPU Idle driver.
>
>
>> All i can say is that the system use the "armadaxp_idle" driver and
>> works fine when running "stress --cpu 8" in background.
>> I asked Netgear to provide a firmware without the idle driver to confirm
>> if it's the cause of the problem, but they did not answered.
>>
>
> I think that if you disable all the state using by doing an
>
> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
>
> where NUM is the nuymbert of the state. Then it should disable the
> cpuidle on the fly.
>
Thanks, i'll try that as soon as possible (i gave back the unit to my
client) and report back.
However, the description of the cpu_pm_enter function state:
"Must be called on the affected CPU with interrupts disabled. Platform
is responsible for ensuring that cpu_pm_enter is not called twice on the
same CPU before cpu_pm_exit is called. Notified drivers can include VFP
co-processor, interrupt controller and its PM extensions, local CPU
timers context save/restore which shouldn't be interrupted. Hence it
must be called with interrupts disabled."
and the point is: it that an invariant? Do current code and future code
safely assume that cpu_pm_enter is not called twice?
For example if cpu_pm_enter do "context save" and cpu_pm_exit do
"context restore", calling twice cpu_pm_enter will overwrite the
previous saved context: is that safe in all circumstances?
I assume the rule " It must fix a real bug that bothers people (not a,
"This could be a problem..." type thing)." is to avoid committing
useless changes that may introduce new bugs, but i do not think that
apply to this case: a bug report from an unknown user (me) should change
nothing.
Bye,
Fulvio
^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-03 14:58 ` Fulvio
@ 2015-03-03 15:20 ` Daniel Lezcano
-1 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-03 15:20 UTC (permalink / raw)
To: Fulvio, Gregory CLEMENT
Cc: Rafael J. Wysocki, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
On 03/03/2015 03:58 PM, Fulvio wrote:
>
>> I didn't know you experimented random kernel panics and that you thought
>> it was related to the CPU Idle driver.
>>
>>> All i can say is that the system use the "armadaxp_idle" driver and
>>> works fine when running "stress --cpu 8" in background.
>>> I asked Netgear to provide a firmware without the idle driver to
>>> confirm if it's the cause of the problem, but they did not answered.
>>
>> I think that if you disable all the state using by doing an
>>
>> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
>>
>> where NUM is the nuymbert of the state. Then it should disable the
>> cpuidle on the fly.
> Thanks, i'll try that as soon as possible (i gave back the unit to my
> client) and report back.
>
> However, the description of the cpu_pm_enter function state:
> "Must be called on the affected CPU with interrupts disabled. Platform
> is responsible for ensuring that cpu_pm_enter is not called twice on the
> same CPU before cpu_pm_exit is called. Notified drivers can include VFP
> co-processor, interrupt controller and its PM extensions, local CPU
> timers context save/restore which shouldn't be interrupted. Hence it
> must be called with interrupts disabled."
>
> and the point is: it that an invariant? Do current code and future code
> safely assume that cpu_pm_enter is not called twice?
The fix is correct. The cpu_pm_enter/exit symmetry must be kept because
we don't know what the notifier clients are doing.
The point is : can we send it to stable@ as a bug fix or not ?
> For example if cpu_pm_enter do "context save" and cpu_pm_exit do
> "context restore", calling twice cpu_pm_enter will overwrite the
> previous saved context: is that safe in all circumstances?
That is the drawback of the notifiers: the kernel provides a service and
everyone plug something on it. The cpu_pm notifier are very low level
functions, so the answer of your question is not obvious. I already
checked all the cpuidle drivers if the potential bug you reported is
there or not but apparently everything else is fine, cpu_pm_exit is
always called after cpu_pm_enter.
As you stated, the API description implies cpu_pm_exit must be called
after cpu_pm_enter. So the fix is right.
> I assume the rule " It must fix a real bug that bothers people (not a,
> "This could be a problem..." type thing)." is to avoid committing
> useless changes that may introduce new bugs, but i do not think that
> apply to this case: a bug report from an unknown user (me) should change
> nothing.
It would be perfect if we can succeed to reproduce the bug you are
facing and check the patch fixes it. In this case, it goes to stable@
automatically.
--
<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] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-03 15:20 ` Daniel Lezcano
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-03 15:20 UTC (permalink / raw)
To: linux-arm-kernel
On 03/03/2015 03:58 PM, Fulvio wrote:
>
>> I didn't know you experimented random kernel panics and that you thought
>> it was related to the CPU Idle driver.
>>
>>> All i can say is that the system use the "armadaxp_idle" driver and
>>> works fine when running "stress --cpu 8" in background.
>>> I asked Netgear to provide a firmware without the idle driver to
>>> confirm if it's the cause of the problem, but they did not answered.
>>
>> I think that if you disable all the state using by doing an
>>
>> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
>>
>> where NUM is the nuymbert of the state. Then it should disable the
>> cpuidle on the fly.
> Thanks, i'll try that as soon as possible (i gave back the unit to my
> client) and report back.
>
> However, the description of the cpu_pm_enter function state:
> "Must be called on the affected CPU with interrupts disabled. Platform
> is responsible for ensuring that cpu_pm_enter is not called twice on the
> same CPU before cpu_pm_exit is called. Notified drivers can include VFP
> co-processor, interrupt controller and its PM extensions, local CPU
> timers context save/restore which shouldn't be interrupted. Hence it
> must be called with interrupts disabled."
>
> and the point is: it that an invariant? Do current code and future code
> safely assume that cpu_pm_enter is not called twice?
The fix is correct. The cpu_pm_enter/exit symmetry must be kept because
we don't know what the notifier clients are doing.
The point is : can we send it to stable@ as a bug fix or not ?
> For example if cpu_pm_enter do "context save" and cpu_pm_exit do
> "context restore", calling twice cpu_pm_enter will overwrite the
> previous saved context: is that safe in all circumstances?
That is the drawback of the notifiers: the kernel provides a service and
everyone plug something on it. The cpu_pm notifier are very low level
functions, so the answer of your question is not obvious. I already
checked all the cpuidle drivers if the potential bug you reported is
there or not but apparently everything else is fine, cpu_pm_exit is
always called after cpu_pm_enter.
As you stated, the API description implies cpu_pm_exit must be called
after cpu_pm_enter. So the fix is right.
> I assume the rule " It must fix a real bug that bothers people (not a,
> "This could be a problem..." type thing)." is to avoid committing
> useless changes that may introduce new bugs, but i do not think that
> apply to this case: a bug report from an unknown user (me) should change
> nothing.
It would be perfect if we can succeed to reproduce the bug you are
facing and check the patch fixes it. In this case, it goes to stable@
automatically.
--
<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] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-04 14:53 ` Rafael J. Wysocki
@ 2015-03-04 14:34 ` Daniel Lezcano
-1 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-04 14:34 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Fulvio, Gregory CLEMENT, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
On 03/04/2015 03:53 PM, Rafael J. Wysocki wrote:
> On Tuesday, March 03, 2015 04:20:26 PM Daniel Lezcano wrote:
>> On 03/03/2015 03:58 PM, Fulvio wrote:
[ ... ]
>> The fix is correct. The cpu_pm_enter/exit symmetry must be kept because
>> we don't know what the notifier clients are doing.
>>
>> The point is : can we send it to stable@ as a bug fix or not ?
>
> Yes, we can, unless there's a risk of breaking things that cannot be
> ignored.
Ok, I added the stable@ tag.
Thanks
-- Daniel
--
<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] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-04 14:34 ` Daniel Lezcano
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Lezcano @ 2015-03-04 14:34 UTC (permalink / raw)
To: linux-arm-kernel
On 03/04/2015 03:53 PM, Rafael J. Wysocki wrote:
> On Tuesday, March 03, 2015 04:20:26 PM Daniel Lezcano wrote:
>> On 03/03/2015 03:58 PM, Fulvio wrote:
[ ... ]
>> The fix is correct. The cpu_pm_enter/exit symmetry must be kept because
>> we don't know what the notifier clients are doing.
>>
>> The point is : can we send it to stable@ as a bug fix or not ?
>
> Yes, we can, unless there's a risk of breaking things that cannot be
> ignored.
Ok, I added the stable@ tag.
Thanks
-- Daniel
--
<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] 30+ messages in thread
* Re: [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
2015-03-03 15:20 ` Daniel Lezcano
@ 2015-03-04 14:53 ` Rafael J. Wysocki
-1 siblings, 0 replies; 30+ messages in thread
From: Rafael J. Wysocki @ 2015-03-04 14:53 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Fulvio, Gregory CLEMENT, linux-pm, Jason Cooper, Andrew Lunn,
Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia,
linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem,
Tawfik Bayouk, Nadav Haklai
On Tuesday, March 03, 2015 04:20:26 PM Daniel Lezcano wrote:
> On 03/03/2015 03:58 PM, Fulvio wrote:
> >
> >> I didn't know you experimented random kernel panics and that you thought
> >> it was related to the CPU Idle driver.
> >>
> >>> All i can say is that the system use the "armadaxp_idle" driver and
> >>> works fine when running "stress --cpu 8" in background.
> >>> I asked Netgear to provide a firmware without the idle driver to
> >>> confirm if it's the cause of the problem, but they did not answered.
> >>
> >> I think that if you disable all the state using by doing an
> >>
> >> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
> >>
> >> where NUM is the nuymbert of the state. Then it should disable the
> >> cpuidle on the fly.
> > Thanks, i'll try that as soon as possible (i gave back the unit to my
> > client) and report back.
> >
> > However, the description of the cpu_pm_enter function state:
> > "Must be called on the affected CPU with interrupts disabled. Platform
> > is responsible for ensuring that cpu_pm_enter is not called twice on the
> > same CPU before cpu_pm_exit is called. Notified drivers can include VFP
> > co-processor, interrupt controller and its PM extensions, local CPU
> > timers context save/restore which shouldn't be interrupted. Hence it
> > must be called with interrupts disabled."
> >
> > and the point is: it that an invariant? Do current code and future code
> > safely assume that cpu_pm_enter is not called twice?
>
> The fix is correct. The cpu_pm_enter/exit symmetry must be kept because
> we don't know what the notifier clients are doing.
>
> The point is : can we send it to stable@ as a bug fix or not ?
Yes, we can, unless there's a risk of breaking things that cannot be
ignored.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage
@ 2015-03-04 14:53 ` Rafael J. Wysocki
0 siblings, 0 replies; 30+ messages in thread
From: Rafael J. Wysocki @ 2015-03-04 14:53 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday, March 03, 2015 04:20:26 PM Daniel Lezcano wrote:
> On 03/03/2015 03:58 PM, Fulvio wrote:
> >
> >> I didn't know you experimented random kernel panics and that you thought
> >> it was related to the CPU Idle driver.
> >>
> >>> All i can say is that the system use the "armadaxp_idle" driver and
> >>> works fine when running "stress --cpu 8" in background.
> >>> I asked Netgear to provide a firmware without the idle driver to
> >>> confirm if it's the cause of the problem, but they did not answered.
> >>
> >> I think that if you disable all the state using by doing an
> >>
> >> echo 1 > /sys/devices/system/cpu/cpu0/cpuidle/stateNUM/disable
> >>
> >> where NUM is the nuymbert of the state. Then it should disable the
> >> cpuidle on the fly.
> > Thanks, i'll try that as soon as possible (i gave back the unit to my
> > client) and report back.
> >
> > However, the description of the cpu_pm_enter function state:
> > "Must be called on the affected CPU with interrupts disabled. Platform
> > is responsible for ensuring that cpu_pm_enter is not called twice on the
> > same CPU before cpu_pm_exit is called. Notified drivers can include VFP
> > co-processor, interrupt controller and its PM extensions, local CPU
> > timers context save/restore which shouldn't be interrupted. Hence it
> > must be called with interrupts disabled."
> >
> > and the point is: it that an invariant? Do current code and future code
> > safely assume that cpu_pm_enter is not called twice?
>
> The fix is correct. The cpu_pm_enter/exit symmetry must be kept because
> we don't know what the notifier clients are doing.
>
> The point is : can we send it to stable@ as a bug fix or not ?
Yes, we can, unless there's a risk of breaking things that cannot be
ignored.
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2015-03-04 14:53 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-26 17:20 [PATCH] cpuidle: mvebu: Fix the CPU PM notifier usage Gregory CLEMENT
2015-02-26 17:20 ` Gregory CLEMENT
2015-02-26 21:55 ` Rafael J. Wysocki
2015-02-26 21:55 ` Rafael J. Wysocki
2015-02-27 9:39 ` Gregory CLEMENT
2015-02-27 9:39 ` Gregory CLEMENT
2015-03-03 10:30 ` Daniel Lezcano
2015-03-03 10:30 ` Daniel Lezcano
2015-03-03 10:34 ` Gregory CLEMENT
2015-03-03 10:34 ` Gregory CLEMENT
2015-03-03 10:52 ` Fulvio
2015-03-03 10:52 ` Fulvio
2015-03-03 11:12 ` Daniel Lezcano
2015-03-03 11:12 ` Daniel Lezcano
2015-03-03 12:51 ` Gregory CLEMENT
2015-03-03 12:51 ` Gregory CLEMENT
2015-03-03 13:00 ` Daniel Lezcano
2015-03-03 13:00 ` Daniel Lezcano
2015-03-03 14:58 ` Fulvio
2015-03-03 14:58 ` Fulvio
2015-03-03 15:20 ` Daniel Lezcano
2015-03-03 15:20 ` Daniel Lezcano
2015-03-04 14:53 ` Rafael J. Wysocki
2015-03-04 14:53 ` Rafael J. Wysocki
2015-03-04 14:34 ` Daniel Lezcano
2015-03-04 14:34 ` Daniel Lezcano
2015-02-27 16:50 ` Daniel Lezcano
2015-02-27 16:50 ` Daniel Lezcano
2015-02-27 22:18 ` Rafael J. Wysocki
2015-02-27 22:18 ` Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.