Re: [PATCH] cpufreq: Drop the 'initialized' field from struct cpufreq_governor

[Viresh Kumar <viresh.kumar@linaro.org>]

On 18-05-16, 22:59, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The 'initialized' field in struct cpufreq_governor is only used by
> the conservative governor (as a usage counter) and the way that
> happens is far from straightforward and arguably incorrect.
> 
> Namely, the value of 'initialized' is checked by
> cpufreq_dbs_governor_init() and cpufreq_dbs_governor_exit() and
> the results of those checks are passed (as the second argument) to
> the ->init() and ->exit() callbacks in struct dbs_governor.  Those
> callbacks are only implemented by the ondemand and conservative
> governors and ondemand doesn't use their second argument at all.
> In turn, the conservative governor uses it to decide whether or not
> to either register or unregister a transition notifier.
> 
> That whole mechanism is not only unnecessarily convoluted, but also
> racy, because the 'initialized' field of struct cpufreq_governor is
> updated in cpufreq_init_governor() and cpufreq_exit_governor() under
> policy->rwsem which doesn't help if one of these functions is run
> twice in parallel for different policies (which isn't impossible in
> principle), for example.
> 
> Instead of it, add a proper usage counter to the conservative
> governor and update it from cs_init() and cs_exit() which is
> guaranteed to be non-racy, as those functions are only called
> under gov_dbs_data_mutex which is global.
> 
> With that in place, drop the 'initialized' field from struct
> cpufreq_governor as it is not used any more.
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> 
> This is on top of https://patchwork.kernel.org/patch/9094021/
> 
> ---
>  drivers/cpufreq/cpufreq.c              |    3 --
>  drivers/cpufreq/cpufreq_conservative.c |   44 +++++++++++++++++++++------------
>  drivers/cpufreq/cpufreq_governor.c     |    6 ++--
>  drivers/cpufreq/cpufreq_governor.h     |    4 +--
>  drivers/cpufreq/cpufreq_ondemand.c     |    4 +--
>  include/linux/cpufreq.h                |    1 
>  6 files changed, 36 insertions(+), 26 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh