Re: [PATCH v4 1/7] topology: add a new arch_scale_freq_reference

Vincent Guittot
2023-10-27T15:19:21Z

On Fri, 27 Oct 2023 at 11:31, Ingo Molnar <mingo@kernel.org> wrote:
>
>
> * Vincent Guittot <vincent.guittot@linaro.org> wrote:
>
> > --- a/include/linux/arch_topology.h
> > +++ b/include/linux/arch_topology.h
> > @@ -27,6 +27,13 @@ static inline unsigned long topology_get_cpu_scale(int cpu)
> >
> > void topology_set_cpu_scale(unsigned int cpu, unsigned long capacity);
> >
> > +DECLARE_PER_CPU(unsigned long, capacity_ref_freq);
> > +
> > +static inline unsigned long topology_get_freq_ref(int cpu)
> > +{
> > + return per_cpu(capacity_ref_freq, cpu);
> > +}
> > +
> > DECLARE_PER_CPU(unsigned long, arch_freq_scale);
>
> So mind naming it in a way that expresses that this is indeed a maximum
> frequency? arch_scale_ref_freq_max or so?

This does not always have to be the max frequency of the CPU but the
frequency that has been used to compute the capacity of the CPU so
it's the "reference" frequency used for computing the capacity and
that must be used to compute the capacity for a given frequency and
the other way. That 's why I have intentionally not used freq_max

Then other similar functions start with arch_scale_freq_something

>
> Also, is there any particular concept behind naming one new symbol
> 'freq_ref', the other 'ref_freq'? If not then please standardize on one
> variant.

no particular reason. ref_freq comes from "reference frequency" but
interfaces follows arch_scale_freq_something or
topology_*_freq_something

so we should keep freq_ref to keep interfaces aligned

>
> Thanks,
>
> Ingo