linux-kernel - Re: [PATCH v7 1/4] cpufreq: Introduce an optional cpuinfo_avg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0i2KUPXfeAKhkimetOMpx+5opgt26URJF8cstnZsaeZwA@mail.gmail.com>
Date: Tue, 29 Oct 2024 12:31:11 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: Beata Michalska <beata.michalska@....com>, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-pm@...r.kernel.org, 
	ionela.voinescu@....com, sudeep.holla@....com, will@...nel.org, 
	catalin.marinas@....com, rafael@...nel.org, sumitg@...dia.com, 
	yang@...amperecomputing.com, vanshikonda@...amperecomputing.com, 
	lihuisong@...wei.com, zhanjie9@...ilicon.com
Subject: Re: [PATCH v7 1/4] cpufreq: Introduce an optional cpuinfo_avg_freq
 sysfs entry

On Tue, Oct 29, 2024 at 8:04 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> Apologies for the delay from my side. September was mostly holidays
> for me and then I was stuck with other stuff plus email backlog and
> this series was always a painful point to return to :(
>
> On 13-09-24, 14:29, Beata Michalska wrote:
> > Currently the CPUFreq core exposes two sysfs attributes that can be used
> > to query current frequency of a given CPU(s): namely cpuinfo_cur_freq
> > and scaling_cur_freq. Both provide slightly different view on the
> > subject and they do come with their own drawbacks.
> >
> > cpuinfo_cur_freq provides higher precision though at a cost of being
> > rather expensive. Moreover, the information retrieved via this attribute
> > is somewhat short lived as frequency can change at any point of time
> > making it difficult to reason from.
> >
> > scaling_cur_freq, on the other hand, tends to be less accurate but then
> > the actual level of precision (and source of information) varies between
> > architectures making it a bit ambiguous.
> >
> > The new attribute, cpuinfo_avg_freq, is intended to provide more stable,
> > distinct interface, exposing an average frequency of a given CPU(s), as
> > reported by the hardware, over a time frame spanning no more than a few
> > milliseconds. As it requires appropriate hardware support, this
> > interface is optional.
>
> From what I recall, the plan is to:
> - keep cpuinfo_cur_freq as it is, not expose for x86 and call ->get()
>   for ARM.

Yes.

> - introduce cpuinfo_avg_freq() and make it return frequency from hw
>   counters for both ARM and Intel and others who provide the API.

Yes.

> - update scaling_cur_freq() to only return the requested frequency or
>   error in case of X86

Yes.

Preferably, -ENOTSUPP for "setpolicy" drivers without the .get() callback.

>   and update documentation to reflect the same.
>   Right now or after some time ? How much time ?

After some time, I think at least two cycles, so people have the time
to switch over, but much more may be necessary if someone is stuck
with RHEL or similar user space.

Anyway, x86 will be the only one affected and there may be a Kconfig
option even to allow it to be changed at the kernel build time.

The documentation for cpuinfo_avg_freq() needs to be added along with it.

> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 04fc786dd2c0..3493e5a9500d 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -752,6 +752,16 @@ __weak unsigned int arch_freq_get_on_cpu(int cpu)
> >       return 0;
> >  }
> >
> > +__weak int arch_freq_avg_get_on_cpu(int cpu)
> > +{
> > +     return -EOPNOTSUPP;
> > +}
> > +
> > +static inline bool cpufreq_avg_freq_supported(struct cpufreq_policy *policy)
> > +{
> > +     return arch_freq_avg_get_on_cpu(policy->cpu) >= 0;
> > +}
>
> And why aren't we simply reusing arch_freq_get_on_cpu() here ?
>
> --
> viresh