Re: [PATCH V4 4/7] cpufreq: governor: Move common sysfs tunables to cpufreq_governor.c

[Rafael J. Wysocki]

On Tue, Feb 9, 2016 at 4:46 AM, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
> We have got five common sysfs tunables between ondemand and conservative
> governors, move their callbacks to cpufreq_governor.c to get rid of
> redundant code.
>
> Because of minor differences in the implementation of the callbacks,
> some more per-governor callbacks are introduced in order to not
> introduce any more "governor == ONDEMAND/CONSERVATIVE" like checks.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@xxxxxxxxxx>
> Tested-by: Juri Lelli <juri.lelli@xxxxxxx>
> Tested-by: Shilpasri G Bhat <shilpa.bhat@xxxxxxxxxxxxxxxxxx>

To me, the benefit from this patch is marginal and the cost is quite
substantial.

The code is really only duplicate if both governors are non-modular or
their modules are both loaded at the same time and it's not worth
adding the new governor callbacks IMO.

If the implementation of the given show/store pair is different enough
that you need an extra callback to move them to _governor.c, I won't
bother doing that.

Thanks,
Rafael