Re: [PATCH v4 01/15] cpufreq: Allow restricting to internal governors only

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Thu, Jun 15, 2023 at 9:21 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 14.06.2023 20:02, Jason Andryuk wrote:
> > @@ -121,6 +122,12 @@ int __init cpufreq_register_governor(struct 
> > cpufreq_governor *governor)
> >      if (!governor)
> >          return -EINVAL;
> >
> > +    if (cpufreq_governor_internal && !governor->internal)
> > +        return -EINVAL;
> > +
> > +    if (!cpufreq_governor_internal && governor->internal)
> > +        return -EINVAL;
>
> First just a nit: Why not simply
>
>     if (cpufreq_governor_internal != governor->internal)
>         return -EINVAL;
>
> ?

Yes, that would work.

> Yet then I find this approach a little odd anyway. I think the
> registration attempts would better be suppressed, thus also not
> resulting in (apparently) failed init-calls. Especially for the
> userspace governor this would then also mean / allow to avoid
> registering of the CPU notifier.

So are you suggesting that each caller check cpufreq_governor_internal
and potentially skip calling cpufreq_register_governor()?  e.g. the
start of cpufreq_gov_userspace_init() would gain:

    if (cpufreq_governor_internal)
        return 0

?

I put the check in cpufreq_register_governor() since then it is
handled in a single place instead of being spread around.

To leave the check in cpufreq_register_governor(),
cpufreq_gov_userspace_init() could be rearranged to call
cpufreq_register_governor() first and only call
register_cpu_notifier() on success?

Or do you want the whole governor registration to be re-worked to be
more explicit?  Remove initcalls and have governors registered
according to the cpufreq driver?

Regards,
Jason