Potential cpufreq backports for v3.10 LTS

Potential cpufreq backports for v3.10 LTS

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2366 bytes --]

Hi,

The power management team at Linaro has had a look at the cpufreq
updates since the v3.10 LTS and identified a few patches that look
like they should be in there.  The most recent of these is from v3.12 so
they won't apply to any of the more recent stable kernels.

 - 59a6342203a7a cpufreq: Fix governor start/stop race condition

   This looks like a straight race condition fix.

 - 19c763031acb8 cpufreq: serialize calls to __cpufreq_governor()

   The commit message here outlines a scenario where this change fixes a
   crash which seems like stable material to me.  There is a context
   change that needs a fixup but it's fairly trivial:

diff --cc drivers/cpufreq/cpufreq.c
index c34d4423f298,7e6baa58a7f2..000000000000
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@@ -1586,8 -1692,10 +1586,9 @@@ static int __cpufreq_governor(struct cp
  						policy->cpu, event);
  
  	mutex_lock(&cpufreq_governor_lock);
- 	if ((!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) ||
 -	if (policy->governor_busy
 -	    || (policy->governor_enabled && event == CPUFREQ_GOV_START)
 -	    || (!policy->governor_enabled
 -	    && (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) {
++	if (policy->governor_busy ||
++	    (!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) ||
 +	    (policy->governor_enabled && (event == CPUFREQ_GOV_START))) {
  		mutex_unlock(&cpufreq_governor_lock);
  		return -EBUSY;
  	}

 - a857c0b9e24e3 cpufreq: Fix wrong time unit conversion

   As noted in the changelog this is a bugfix for non-tick based
   systems like full dynticks ones.

There's also this:

 - dfa5bb622555d cpufreq: ondemand: Change the calculation of target frequency

   This is a small patch which delivers a useful performance improvement
   but it's not a bug fix and it does touch the cpufreq core so it
   doesn't seem to really fit.  On the other hand I've got user reports
   that it fixes problems with some multicluster systems that can scale
   cluster frequencies independently (the additional stability in the
   frequencies selected avoids poor interaction with scheduler as cpufreq
   ramps the frequency while the scheduler moves load away from the
   cluster), it's simple and it's quite well isolated so I wanted to
   mention it.

Thanks,
Mark

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Potential cpufreq backports for v3.10 LTS

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2622 bytes --]

On Tue, Oct 07, 2014 at 12:48:44PM +0100, Mark Brown wrote:
> Hi,
> 
> The power management team at Linaro has had a look at the cpufreq
> updates since the v3.10 LTS and identified a few patches that look
> like they should be in there.  The most recent of these is from v3.12 so
> they won't apply to any of the more recent stable kernels.

CCing in the actual stable list rather than the nonexistant linux-stable.

>  - 59a6342203a7a cpufreq: Fix governor start/stop race condition
> 
>    This looks like a straight race condition fix.
> 
>  - 19c763031acb8 cpufreq: serialize calls to __cpufreq_governor()
> 
>    The commit message here outlines a scenario where this change fixes a
>    crash which seems like stable material to me.  There is a context
>    change that needs a fixup but it's fairly trivial:
> 
> diff --cc drivers/cpufreq/cpufreq.c
> index c34d4423f298,7e6baa58a7f2..000000000000
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@@ -1586,8 -1692,10 +1586,9 @@@ static int __cpufreq_governor(struct cp
>   						policy->cpu, event);
>   
>   	mutex_lock(&cpufreq_governor_lock);
> - 	if ((!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) ||
>  -	if (policy->governor_busy
>  -	    || (policy->governor_enabled && event == CPUFREQ_GOV_START)
>  -	    || (!policy->governor_enabled
>  -	    && (event == CPUFREQ_GOV_LIMITS || event == CPUFREQ_GOV_STOP))) {
> ++	if (policy->governor_busy ||
> ++	    (!policy->governor_enabled && (event == CPUFREQ_GOV_STOP)) ||
>  +	    (policy->governor_enabled && (event == CPUFREQ_GOV_START))) {
>   		mutex_unlock(&cpufreq_governor_lock);
>   		return -EBUSY;
>   	}
> 
>  - a857c0b9e24e3 cpufreq: Fix wrong time unit conversion
> 
>    As noted in the changelog this is a bugfix for non-tick based
>    systems like full dynticks ones.
> 
> There's also this:
> 
>  - dfa5bb622555d cpufreq: ondemand: Change the calculation of target frequency
> 
>    This is a small patch which delivers a useful performance improvement
>    but it's not a bug fix and it does touch the cpufreq core so it
>    doesn't seem to really fit.  On the other hand I've got user reports
>    that it fixes problems with some multicluster systems that can scale
>    cluster frequencies independently (the additional stability in the
>    frequencies selected avoids poor interaction with scheduler as cpufreq
>    ramps the frequency while the scheduler moves load away from the
>    cluster), it's simple and it's quite well isolated so I wanted to
>    mention it.
> 
> Thanks,
> Mark



[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Potential cpufreq backports for v3.10 LTS

[Greg Kroah-Hartman]

On Tue, Oct 07, 2014 at 12:48:44PM +0100, Mark Brown wrote:
> Hi,
> 
> The power management team at Linaro has had a look at the cpufreq
> updates since the v3.10 LTS and identified a few patches that look
> like they should be in there.  The most recent of these is from v3.12 so
> they won't apply to any of the more recent stable kernels.
> 
>  - 59a6342203a7a cpufreq: Fix governor start/stop race condition
> 
>    This looks like a straight race condition fix.

That is not a commit id in Linus's tree :(

Re: Potential cpufreq backports for v3.10 LTS

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

On Tue, Oct 07, 2014 at 09:46:43AM -0700, Greg Kroah-Hartman wrote:
> On Tue, Oct 07, 2014 at 12:48:44PM +0100, Mark Brown wrote:

> > The power management team at Linaro has had a look at the cpufreq
> > updates since the v3.10 LTS and identified a few patches that look
> > like they should be in there.  The most recent of these is from v3.12 so
> > they won't apply to any of the more recent stable kernels.

> >  - 59a6342203a7a cpufreq: Fix governor start/stop race condition

> >    This looks like a straight race condition fix.

> That is not a commit id in Linus's tree :(

Interesting, I was able to cherry-pick it...  looks like the upstream
version is 95731eb.  In any case at roughly the same time you sent your
mail it's been drawn to my attention off-list that this was subsequently
reverted so please ignore this one.  The others should be fine though.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Potential cpufreq backports for v3.10 LTS

[Greg Kroah-Hartman]

On Tue, Oct 07, 2014 at 06:08:00PM +0100, Mark Brown wrote:
> On Tue, Oct 07, 2014 at 09:46:43AM -0700, Greg Kroah-Hartman wrote:
> > On Tue, Oct 07, 2014 at 12:48:44PM +0100, Mark Brown wrote:
> 
> > > The power management team at Linaro has had a look at the cpufreq
> > > updates since the v3.10 LTS and identified a few patches that look
> > > like they should be in there.  The most recent of these is from v3.12 so
> > > they won't apply to any of the more recent stable kernels.
> 
> > >  - 59a6342203a7a cpufreq: Fix governor start/stop race condition
> 
> > >    This looks like a straight race condition fix.
> 
> > That is not a commit id in Linus's tree :(
> 
> Interesting, I was able to cherry-pick it...  looks like the upstream
> version is 95731eb.  In any case at roughly the same time you sent your
> mail it's been drawn to my attention off-list that this was subsequently
> reverted so please ignore this one.  The others should be fine though.

Ok, I've queued the others up now.

thanks,

greg k-h

Re: Potential cpufreq backports for v3.10 LTS

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 999 bytes --]

On Tue, Oct 07, 2014 at 01:04:58PM -0700, Greg Kroah-Hartman wrote:
> On Tue, Oct 07, 2014 at 06:08:00PM +0100, Mark Brown wrote:

> > > >  - 59a6342203a7a cpufreq: Fix governor start/stop race condition

> > > >    This looks like a straight race condition fix.

> > > That is not a commit id in Linus's tree :(

> > Interesting, I was able to cherry-pick it...  looks like the upstream
> > version is 95731eb.  In any case at roughly the same time you sent your
> > mail it's been drawn to my attention off-list that this was subsequently
> > reverted so please ignore this one.  The others should be fine though.

> Ok, I've queued the others up now.

Actually I thinkoed here, as I said in reply to the stable mail the
problematic patch was "19c763031acb8 cpufreq: serialize calls to
__cpufreq_governor()" which was reverted in "56d07db cpufreq: Remove
temporary fix for race between CPU hotplug and sysfs-writes".  The above
commit ID (95731eb) should be good.

Sorry about the confusion here.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Potential cpufreq backports for v3.10 LTS

[Greg Kroah-Hartman]

On Tue, Oct 07, 2014 at 09:45:05PM +0100, Mark Brown wrote:
> On Tue, Oct 07, 2014 at 01:04:58PM -0700, Greg Kroah-Hartman wrote:
> > On Tue, Oct 07, 2014 at 06:08:00PM +0100, Mark Brown wrote:
> 
> > > > >  - 59a6342203a7a cpufreq: Fix governor start/stop race condition
> 
> > > > >    This looks like a straight race condition fix.
> 
> > > > That is not a commit id in Linus's tree :(
> 
> > > Interesting, I was able to cherry-pick it...  looks like the upstream
> > > version is 95731eb.  In any case at roughly the same time you sent your
> > > mail it's been drawn to my attention off-list that this was subsequently
> > > reverted so please ignore this one.  The others should be fine though.
> 
> > Ok, I've queued the others up now.
> 
> Actually I thinkoed here, as I said in reply to the stable mail the
> problematic patch was "19c763031acb8 cpufreq: serialize calls to
> __cpufreq_governor()" which was reverted in "56d07db cpufreq: Remove
> temporary fix for race between CPU hotplug and sysfs-writes".  The above
> commit ID (95731eb) should be good.
> 
> Sorry about the confusion here.

Ok, I'm still confused.  I've applied 3 patches to the 3.10-stable
queue, in this order:
	19c763031acb831a5ab9c1a701b7fedda073eb3f	cpufreq: serialize calls to __cpufreq_governor()
	a857c0b9e24e39fe5be82451b65377795f9538d8	cpufreq: Fix wrong time unit conversion
	dfa5bb622555d9da0df21b50f46ebdeef390041b	cpufreq: ondemand: Change the calculation of target frequency

Are those correct?  Is there anything else I need to apply?

Note, 95731eb does not apply to 3.10-stable as it is already in there,
it showed up in 3.10.37.

thanks,

greg k-h

Re: Potential cpufreq backports for v3.10 LTS

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

On Tue, Oct 07, 2014 at 02:58:07PM -0700, Greg Kroah-Hartman wrote:
> On Tue, Oct 07, 2014 at 09:45:05PM +0100, Mark Brown wrote:

> > Actually I thinkoed here, as I said in reply to the stable mail the
> > problematic patch was "19c763031acb8 cpufreq: serialize calls to
> > __cpufreq_governor()" which was reverted in "56d07db cpufreq: Remove
> > temporary fix for race between CPU hotplug and sysfs-writes".  The above
> > commit ID (95731eb) should be good.

> > Sorry about the confusion here.

> Ok, I'm still confused.  I've applied 3 patches to the 3.10-stable
> queue, in this order:

To the best of my ability to assess:

> 	19c763031acb831a5ab9c1a701b7fedda073eb3f	cpufreq: serialize calls to __cpufreq_governor()

This one is bad.

> 	a857c0b9e24e39fe5be82451b65377795f9538d8	cpufreq: Fix wrong time unit conversion
> 	dfa5bb622555d9da0df21b50f46ebdeef390041b	cpufreq: ondemand: Change the calculation of target frequency

These two should be good.

> Are those correct?  Is there anything else I need to apply?

> Note, 95731eb does not apply to 3.10-stable as it is already in there,
> it showed up in 3.10.37.

Ah, good - I think whatever made my cut'n'paste of the commit ID go
wrong will have been responsible for making my search for it fail.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: Potential cpufreq backports for v3.10 LTS

[Greg Kroah-Hartman]

On Tue, Oct 07, 2014 at 11:50:06PM +0100, Mark Brown wrote:
> On Tue, Oct 07, 2014 at 02:58:07PM -0700, Greg Kroah-Hartman wrote:
> > On Tue, Oct 07, 2014 at 09:45:05PM +0100, Mark Brown wrote:
> 
> > > Actually I thinkoed here, as I said in reply to the stable mail the
> > > problematic patch was "19c763031acb8 cpufreq: serialize calls to
> > > __cpufreq_governor()" which was reverted in "56d07db cpufreq: Remove
> > > temporary fix for race between CPU hotplug and sysfs-writes".  The above
> > > commit ID (95731eb) should be good.
> 
> > > Sorry about the confusion here.
> 
> > Ok, I'm still confused.  I've applied 3 patches to the 3.10-stable
> > queue, in this order:
> 
> To the best of my ability to assess:
> 
> > 	19c763031acb831a5ab9c1a701b7fedda073eb3f	cpufreq: serialize calls to __cpufreq_governor()
> 
> This one is bad.

Ok, now deleted, thanks for letting me know.

greg k-h

Re: Potential cpufreq backports for v3.10 LTS

[Stratos Karafotis]

Hi,

On 07/10/2014 11:04 μμ, Greg Kroah-Hartman wrote:
> On Tue, Oct 07, 2014 at 06:08:00PM +0100, Mark Brown wrote:
>> On Tue, Oct 07, 2014 at 09:46:43AM -0700, Greg Kroah-Hartman wrote:
>>> On Tue, Oct 07, 2014 at 12:48:44PM +0100, Mark Brown wrote:
>>
>>>> The power management team at Linaro has had a look at the cpufreq
>>>> updates since the v3.10 LTS and identified a few patches that look
>>>> like they should be in there.  The most recent of these is from v3.12 so
>>>> they won't apply to any of the more recent stable kernels.
>>
>>>>  - 59a6342203a7a cpufreq: Fix governor start/stop race condition
>>
>>>>    This looks like a straight race condition fix.
>>
>>> That is not a commit id in Linus's tree :(
>>
>> Interesting, I was able to cherry-pick it...  looks like the upstream
>> version is 95731eb.  In any case at roughly the same time you sent your
>> mail it's been drawn to my attention off-list that this was subsequently
>> reverted so please ignore this one.  The others should be fine though.
> 
> Ok, I've queued the others up now.
> 

For your information, commits:

- 61c63e5ed3b9 cpufreq: Remove unused APERF/MPERF support
- cffe4e0e7413 cpufreq: Remove unused function __cpufreq_driver_getavg()

remove unused code (no other functional changes) as a result of
commit dfa5bb622555. They all were part of the same patch set.

I'm not familiar with the procedure of patch inclusion in stable trees,
so please forgive me if this mail is just noise.


Thanks,
Stratos

Re: Potential cpufreq backports for v3.10 LTS

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

On Wed, Oct 08, 2014 at 12:36:59AM +0300, Stratos Karafotis wrote:

> For your information, commits:

> - 61c63e5ed3b9 cpufreq: Remove unused APERF/MPERF support
> - cffe4e0e7413 cpufreq: Remove unused function __cpufreq_driver_getavg()

> remove unused code (no other functional changes) as a result of
> commit dfa5bb622555. They all were part of the same patch set.

> I'm not familiar with the procedure of patch inclusion in stable trees,
> so please forgive me if this mail is just noise.

If they're just cleanups and not producing a functional change then the
standard thing would be to leave them out of stable.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]