mbox series

[V4,00/17] thermal: cpu_cooling: improve interaction with cpufreq core

Message ID cover.1493115651.git.viresh.kumar@linaro.org
Headers show
Series thermal: cpu_cooling: improve interaction with cpufreq core | expand

Message

Viresh Kumar April 25, 2017, 10:27 a.m. UTC
Hi Guys,

The cpu_cooling driver is designed to use CPU frequency scaling to avoid
high thermal states for a platform. But it wasn't glued really well with
cpufreq core. For example clipped-cpus is copied from the policy
structure and its much better to use the policy->cpus (or related_cpus)
fields directly as they may have got updated. Not that things were
broken before this series, but they can be optimized a bit more.

This series tries to improve interactions between cpufreq core and
cpu_cooling driver and does some fixes/cleanups to the cpu_cooling
driver.

I have tested it on ARM 32 (exynos) and 64 bit (hikey) boards (haven't
tested the power specific bits).

Lukasz from ARM has been very generous in testing and finding out few
bugs in the earlier versions and getting those fixed.  He has
successfully tested the new version on his ARM big LITTLE Juno board.

Pushed here as well:

git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git thermal/cooling

V3->V4:
- The pointer to policy in the cpufreq notifier will have a different
  address and so that address can't be used for comparison. Compare
  policy->cpu instead.
- max_level field was used for the newly created power-freq table, but
  few places I have used < max_level instead of <= max_level and that
  caused the trouble.
- Added Tested-by from Lukasz.

V2->V3:
- Additional check to guarantee that policy is valid.
- Initialize freq-table and cpufreq_cdev->policy fields before they are
  used by the power-cooling functionality.
- Thanks Lukasz for testing out and suggesting these changes.

V1->V2:
- Name cpufreq cooling dev as cpufreq_cdev everywhere (Eduardo).

--
viresh

Viresh Kumar (17):
  thermal: cpu_cooling: Avoid accessing potentially freed structures
  thermal: cpu_cooling: rearrange globals
  thermal: cpu_cooling: Name cpufreq cooling devices as cpufreq_cdev
  thermal: cpu_cooling: replace cool_dev with cdev
  thermal: cpu_cooling: remove cpufreq_cooling_get_level()
  thermal: cpu_cooling: get rid of a variable in cpufreq_set_cur_state()
  thermal: cpu_cooling: use cpufreq_policy to register cooling device
  cpufreq: create cpufreq_table_count_valid_entries()
  thermal: cpu_cooling: store cpufreq policy
  thermal: cpu_cooling: OPPs are registered for all CPUs
  thermal: cpu_cooling: get rid of 'allowed_cpus'
  thermal: cpu_cooling: merge frequency and power tables
  thermal: cpu_cooling: create structure for idle time stats
  thermal: cpu_cooling: get_level() can't fail
  thermal: cpu_cooling: don't store cpu_dev in cpufreq_cdev
  thermal: cpu_cooling: 'freq' can't be zero in cpufreq_state2power()
  thermal: cpu_cooling: Rearrange struct cpufreq_cooling_device

 drivers/cpufreq/arm_big_little.c                   |   2 +-
 drivers/cpufreq/cpufreq-dt.c                       |   2 +-
 drivers/cpufreq/cpufreq_stats.c                    |  13 +-
 drivers/cpufreq/dbx500-cpufreq.c                   |   2 +-
 drivers/cpufreq/mt8173-cpufreq.c                   |   4 +-
 drivers/cpufreq/qoriq-cpufreq.c                    |   3 +-
 drivers/thermal/cpu_cooling.c                      | 606 +++++++++------------
 drivers/thermal/imx_thermal.c                      |  22 +-
 drivers/thermal/ti-soc-thermal/ti-thermal-common.c |  22 +-
 include/linux/cpu_cooling.h                        |  32 +-
 include/linux/cpufreq.h                            |  14 +
 11 files changed, 315 insertions(+), 407 deletions(-)

-- 
2.12.0.432.g71c3a4f4ba37

Comments

Lukasz Luba April 26, 2017, 10:41 a.m. UTC | #1
Hi Viresh,

I went through the v4 code and it looks good to me.
Feel free to add for the v4 series
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>


Thank you Viresh!

Regards,
Lukasz


On 25/04/17 11:27, Viresh Kumar wrote:
> Hi Guys,

>

> The cpu_cooling driver is designed to use CPU frequency scaling to avoid

> high thermal states for a platform. But it wasn't glued really well with

> cpufreq core. For example clipped-cpus is copied from the policy

> structure and its much better to use the policy->cpus (or related_cpus)

> fields directly as they may have got updated. Not that things were

> broken before this series, but they can be optimized a bit more.

>

> This series tries to improve interactions between cpufreq core and

> cpu_cooling driver and does some fixes/cleanups to the cpu_cooling

> driver.

>

> I have tested it on ARM 32 (exynos) and 64 bit (hikey) boards (haven't

> tested the power specific bits).

>

> Lukasz from ARM has been very generous in testing and finding out few

> bugs in the earlier versions and getting those fixed.  He has

> successfully tested the new version on his ARM big LITTLE Juno board.

>

> Pushed here as well:

>

> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git thermal/cooling

>

> V3->V4:

> - The pointer to policy in the cpufreq notifier will have a different

>   address and so that address can't be used for comparison. Compare

>   policy->cpu instead.

> - max_level field was used for the newly created power-freq table, but

>   few places I have used < max_level instead of <= max_level and that

>   caused the trouble.

> - Added Tested-by from Lukasz.

>

> V2->V3:

> - Additional check to guarantee that policy is valid.

> - Initialize freq-table and cpufreq_cdev->policy fields before they are

>   used by the power-cooling functionality.

> - Thanks Lukasz for testing out and suggesting these changes.

>

> V1->V2:

> - Name cpufreq cooling dev as cpufreq_cdev everywhere (Eduardo).

>

> --

> viresh

>

> Viresh Kumar (17):

>   thermal: cpu_cooling: Avoid accessing potentially freed structures

>   thermal: cpu_cooling: rearrange globals

>   thermal: cpu_cooling: Name cpufreq cooling devices as cpufreq_cdev

>   thermal: cpu_cooling: replace cool_dev with cdev

>   thermal: cpu_cooling: remove cpufreq_cooling_get_level()

>   thermal: cpu_cooling: get rid of a variable in cpufreq_set_cur_state()

>   thermal: cpu_cooling: use cpufreq_policy to register cooling device

>   cpufreq: create cpufreq_table_count_valid_entries()

>   thermal: cpu_cooling: store cpufreq policy

>   thermal: cpu_cooling: OPPs are registered for all CPUs

>   thermal: cpu_cooling: get rid of 'allowed_cpus'

>   thermal: cpu_cooling: merge frequency and power tables

>   thermal: cpu_cooling: create structure for idle time stats

>   thermal: cpu_cooling: get_level() can't fail

>   thermal: cpu_cooling: don't store cpu_dev in cpufreq_cdev

>   thermal: cpu_cooling: 'freq' can't be zero in cpufreq_state2power()

>   thermal: cpu_cooling: Rearrange struct cpufreq_cooling_device

>

>  drivers/cpufreq/arm_big_little.c                   |   2 +-

>  drivers/cpufreq/cpufreq-dt.c                       |   2 +-

>  drivers/cpufreq/cpufreq_stats.c                    |  13 +-

>  drivers/cpufreq/dbx500-cpufreq.c                   |   2 +-

>  drivers/cpufreq/mt8173-cpufreq.c                   |   4 +-

>  drivers/cpufreq/qoriq-cpufreq.c                    |   3 +-

>  drivers/thermal/cpu_cooling.c                      | 606 +++++++++------------

>  drivers/thermal/imx_thermal.c                      |  22 +-

>  drivers/thermal/ti-soc-thermal/ti-thermal-common.c |  22 +-

>  include/linux/cpu_cooling.h                        |  32 +-

>  include/linux/cpufreq.h                            |  14 +

>  11 files changed, 315 insertions(+), 407 deletions(-)

>
Viresh Kumar April 26, 2017, 10:47 a.m. UTC | #2
On 26-04-17, 11:41, Lukasz Luba wrote:
> Hi Viresh,

> 

> I went through the v4 code and it looks good to me.

> Feel free to add for the v4 series

> Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>


Thanks a lot for testing and reviewing the series.

@Eduardo: Will you be able to pick it for 4.12 ?

-- 
viresh
Eduardo Valentin April 27, 2017, 4:26 p.m. UTC | #3
On Wed, Apr 26, 2017 at 04:17:52PM +0530, Viresh Kumar wrote:
> On 26-04-17, 11:41, Lukasz Luba wrote:

> > Hi Viresh,

> > 

> > I went through the v4 code and it looks good to me.

> > Feel free to add for the v4 series

> > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

> 

> Thanks a lot for testing and reviewing the series.

> 

> @Eduardo: Will you be able to pick it for 4.12 ?


I feel like this is late for 4.12 given that we are already -rc8. 
Besides, it has gone through a couple of cycles of broken interactions
with IPA (now fixed).

So, I would prefer we give this series a bit more of testing time on
linux-next. I can pull it and put into linux-next after the coming
merge window is closed.

> 

> -- 

> viresh


Thanks,

Eduardo
Viresh Kumar April 28, 2017, 4:24 a.m. UTC | #4
On 27-04-17, 09:26, Eduardo Valentin wrote:
> On Wed, Apr 26, 2017 at 04:17:52PM +0530, Viresh Kumar wrote:

> > On 26-04-17, 11:41, Lukasz Luba wrote:

> > > Hi Viresh,

> > > 

> > > I went through the v4 code and it looks good to me.

> > > Feel free to add for the v4 series

> > > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

> > 

> > Thanks a lot for testing and reviewing the series.

> > 

> > @Eduardo: Will you be able to pick it for 4.12 ?

> 

> I feel like this is late for 4.12 given that we are already -rc8. 

> Besides, it has gone through a couple of cycles of broken interactions

> with IPA (now fixed).

> 

> So, I would prefer we give this series a bit more of testing time on

> linux-next. I can pull it and put into linux-next after the coming

> merge window is closed.


Ok, sure. No hurry.

-- 
viresh
Eduardo Valentin May 24, 2017, 2:41 a.m. UTC | #5
Hey,

On Tue, Apr 25, 2017 at 03:57:07PM +0530, Viresh Kumar wrote:
> Hi Guys,

> 

> The cpu_cooling driver is designed to use CPU frequency scaling to avoid

> high thermal states for a platform. But it wasn't glued really well with

> cpufreq core. For example clipped-cpus is copied from the policy

> structure and its much better to use the policy->cpus (or related_cpus)

> fields directly as they may have got updated. Not that things were

> broken before this series, but they can be optimized a bit more.

> 

> This series tries to improve interactions between cpufreq core and

> cpu_cooling driver and does some fixes/cleanups to the cpu_cooling

> driver.

> 

> I have tested it on ARM 32 (exynos) and 64 bit (hikey) boards (haven't

> tested the power specific bits).

> 

> Lukasz from ARM has been very generous in testing and finding out few

> bugs in the earlier versions and getting those fixed.  He has

> successfully tested the new version on his ARM big LITTLE Juno board.

> 

> Pushed here as well:

> 

> git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git thermal/cooling

> 

> V3->V4:



Took this version into my -linus branch for the next merge window.
Including the patch to remove the checkpatch warning. But please send
an extra patch to fix the style pointed on patch 8.
Viresh Kumar May 24, 2017, 3:53 a.m. UTC | #6
On 23-05-17, 19:41, Eduardo Valentin wrote:
> Hey,

> 

> On Tue, Apr 25, 2017 at 03:57:07PM +0530, Viresh Kumar wrote:

> > Hi Guys,

> > 

> > The cpu_cooling driver is designed to use CPU frequency scaling to avoid

> > high thermal states for a platform. But it wasn't glued really well with

> > cpufreq core. For example clipped-cpus is copied from the policy

> > structure and its much better to use the policy->cpus (or related_cpus)

> > fields directly as they may have got updated. Not that things were

> > broken before this series, but they can be optimized a bit more.

> > 

> > This series tries to improve interactions between cpufreq core and

> > cpu_cooling driver and does some fixes/cleanups to the cpu_cooling

> > driver.

> > 

> > I have tested it on ARM 32 (exynos) and 64 bit (hikey) boards (haven't

> > tested the power specific bits).

> > 

> > Lukasz from ARM has been very generous in testing and finding out few

> > bugs in the earlier versions and getting those fixed.  He has

> > successfully tested the new version on his ARM big LITTLE Juno board.

> > 

> > Pushed here as well:

> > 

> > git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git thermal/cooling

> > 

> > V3->V4:

> 

> 

> Took this version into my -linus branch for the next merge window.

> Including the patch to remove the checkpatch warning. But please send

> an extra patch to fix the style pointed on patch 8.


Thanks Eduardo.

But I am not sure what's left there to be fixed :(

There were two warnings with patch 8, s/kmalloc/kmalloc_array and line
over 80 columns and both were fixed by the patch I sent separately.

-- 
viresh
Eduardo Valentin May 24, 2017, 4:03 a.m. UTC | #7
Viresh,

On Wed, May 24, 2017 at 09:23:52AM +0530, Viresh Kumar wrote:
> On 23-05-17, 19:41, Eduardo Valentin wrote:

> > Hey,

> > 

> > On Tue, Apr 25, 2017 at 03:57:07PM +0530, Viresh Kumar wrote:

> > > Hi Guys,

> > > 

> > > The cpu_cooling driver is designed to use CPU frequency scaling to avoid

> > > high thermal states for a platform. But it wasn't glued really well with

> > > cpufreq core. For example clipped-cpus is copied from the policy

> > > structure and its much better to use the policy->cpus (or related_cpus)

> > > fields directly as they may have got updated. Not that things were

> > > broken before this series, but they can be optimized a bit more.

> > > 

> > > This series tries to improve interactions between cpufreq core and

> > > cpu_cooling driver and does some fixes/cleanups to the cpu_cooling

> > > driver.

> > > 

> > > I have tested it on ARM 32 (exynos) and 64 bit (hikey) boards (haven't

> > > tested the power specific bits).

> > > 

> > > Lukasz from ARM has been very generous in testing and finding out few

> > > bugs in the earlier versions and getting those fixed.  He has

> > > successfully tested the new version on his ARM big LITTLE Juno board.

> > > 

> > > Pushed here as well:

> > > 

> > > git://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git thermal/cooling

> > > 

> > > V3->V4:

> > 

> > 

> > Took this version into my -linus branch for the next merge window.

> > Including the patch to remove the checkpatch warning. But please send

> > an extra patch to fix the style pointed on patch 8.

> 

> Thanks Eduardo.

> 

> But I am not sure what's left there to be fixed :(

> 

> There were two warnings with patch 8, s/kmalloc/kmalloc_array and line

> over 80 columns and both were fixed by the patch I sent separately.


You are right, you are done here. This was probably a bug in my script
showing old checkpatch results on patch 8, somehow.

Sorry for the noise.

> 

> -- 

> viresh
Eduardo Valentin May 28, 2017, 12:29 a.m. UTC | #8
On Fri, May 26, 2017 at 10:27:18AM +0530, Viresh Kumar wrote:
> On 26-04-17, 11:41, Lukasz Luba wrote:

> > Hi Viresh,

> > 

> > I went through the v4 code and it looks good to me.

> > Feel free to add for the v4 series

> > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>

> 

> @Eduardo: You missed adding this to the patches.


This is true. I picked your patches before the previous merge window and
they were locally in my tree. Then I pushed them to k.org and missed
this. That is fine, only your patches are for next merge window and it
doesnt hurt too much to rebuild the branch.

Thanks for noticing.

> 

> -- 

> viresh