Re: [PATCH v3 0/2] Add stop callback to the cpufreq_driver interface.

[Srivatsa S. Bhat]

On 03/18/2014 10:52 PM, dirk.brandewie@xxxxxxxxx wrote:
> From: Dirk Brandewie <dirk.j.brandewie@xxxxxxxxx>
>

I don't mean to nitpick, but generally its easier to deal with
patchsets if you post the subsequent versions in fresh email threads.
Otherwise it can get a bit muddled along with too many other email
discussions in the same thread :-(
 
> Changes: 
> v2->v3
> Changed the calling of the ->stop() callback to be conditional on the
> core being the last core controlled by a given policy.
> 

Wait, why? I'm sorry if I am not catching up with the discussions on
this issue quickly enough, but I don't see why we should make it
conditional on _that_. I thought we agreed that we should make it
conditional in the sense that ->stop() should be invoked only for
->setpolicy drivers, right?

The way I look at it, ->stop() gives you a chance to stop managing
the CPU going offline. As in "stop this CPU". ->exit() is your chance
to cleanup the policy, since all its users have gone offline (or this
is the last CPU belonging to that policy which is going offline).

With this in mind, we should invoke ->stop() every time we take a
CPU offline, and invoke ->exit() only when the last CPU in the policy
goes offline.

What am I missing?

Regards,
Srivatsa S. Bhat

> v1->2
> Change name of callback function added to cpufreq_driver interface.
> 
> Some drivers (intel_pstate) need to modify state on a core before it
> is completely offline.  The ->exit() callback is executed during the
> CPU_POST_DEAD phase of the cpu offline process which is too late to
> change the state of the core.
> 
> Patch 1 adds an optional callback function to the cpufreq_driver
> interface which is called by the core during the CPU_DOWN_PREPARE
> phase of cpu offline in __cpufreq_remove_dev_prepare().
> 
> Patch 2 changes intel_pstate to use the ->stop callback to do
> its cleanup during cpu offline. 
> 
> Dirk Brandewie (2):
>   cpufreq: Add exit_prepare callback to cpufreq_driver interface
>   intel_pstate: Set core to min P state during core offline
> 
>  Documentation/cpu-freq/cpu-drivers.txt |  8 +++++++-
>  drivers/cpufreq/cpufreq.c              |  3 +++
>  drivers/cpufreq/intel_pstate.c         | 20 +++++++++++++-------
>  include/linux/cpufreq.h                |  1 +
>  4 files changed, 24 insertions(+), 8 deletions(-)
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/