linux-kernel - Re: [PATCH] cpufreq/cppc: changing highest_perf to nominal_perf in cppc_cpufreq_cpu

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] [day] [month] [year] [list]

Message-ID: <20240613084051.6e7o32nmwim4wj5d@vireshk-i7>
Date: Thu, 13 Jun 2024 14:10:51 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: Ionela Voinescu <ionela.voinescu@....com>
Cc: "liwei (JK)" <liwei728@...wei.com>,
	Beata Michalska <beata.michalska@....com>,
	Vanshidhar Konda <vanshikonda@...amperecomputing.com>,
	rafael@...nel.org, al.stone@...aro.org, ashwin.chaugule@...aro.org,
	linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
	liwei391@...wei.com, liaoyu15@...wei.com
Subject: Re: [PATCH] cpufreq/cppc: changing highest_perf to nominal_perf in
 cppc_cpufreq_cpu_init()

On 11-06-24, 11:29, Ionela Voinescu wrote:
> Yes, I mean we should only update policy->cur to a requested frequency
> from a governor, after we start it (cpufreq_start_governor()).

Hmm, I went through the code one more time and there is more to it I guess.

During the earlier days (when the cpufreq core was taking shape), I think the
basic idea behind keeping the policy->cur field was (which is true today as
well):

- Know current frequency of a CPU, quickly and so we cached it.

- Avoid a frequency change, and not waste time, if we know that we aren't going
  to change the frequency eventually.

- Ideally we can just remove the field, if the get() operation has zero
  overhead. But it doesn't and so we have this field.


What the core does now is this:

- Once the frequency is changed, set policy->cur to the frequency that the
  software believes the hardware is running at. This can be different from the
  requested frequency for example.

- If we end up getting the real frequency from the hardware somehow, update
  policy->cur to match and so to avoid wasting of time later.

- And I think this is the right thing to do even today.

> But currently policy->cur gets updated to the .get() returned value in
> multiple places, via cpufreq_verify_current_freq() (for example from 
> show_cpuinfo_cur_freq() or cpufreq_get().

Which makes sense based on above..

> .get() is meant to return the current frequency of the hardware and that
> can opportunistically be different from the last request made.

Right, and so if we have called get() for some other reason, like the user
requesting it, we opportunistically fix policy->cur as well.

> (+ we probably should force the first request from a governor to go
> through to the driver to make sure the policy->cur obtained before,
> via .get(), did not just happen to coincide with the governor request,
> therefore making the request no longer go through to the driver: see
> __cpufreq_driver_target)

Not sure why would we want to do that.

> > needs to be up to date all the times, it is an important part of the
> > entire working of the cpufreq core..
> 
> When you say that "policy->cur must be kept up to date at all times",
> I suppose you mean that it should be kept up to date with any frequency
> change requests not with any changes happening in hardware?

Yes and no. It should be up to date with the best knowledge that the software
has, without any extra overheads.

-- 
viresh