On 15/06/23 00:29, Sumit Gupta wrote:
On 06/06/23 21:27, Beata Michalska wrote:
External email: Use caution opening links or attachments
With the Frequency Invariance Engine (FIE) being already wired up with
sched tick and making use of relevant (core counter and constant
counter) AMU counters, getting the current frequency for a given CPU
on supported platforms, can be achieved by utilizing the frequency scale
factor which reflects an average CPU frequency for the last tick period
length.
With that at hand, arch_freq_get_on_cpu dedicated implementation
gets enrolled into cpuinfo_cur_freq policy sysfs attribute handler,
which is expected to represent the current frequency of a given CPU,
as obtained by the hardware. This is exactly the type of feedback that
cycle counters provide.
In order to avoid calling arch_freq_get_on_cpu from the scaling_cur_freq
attribute handler for platforms that do provide cpuinfo_cur_freq, and
yet keeping things intact for those platform that do not, its use gets
conditioned on the presence of cpufreq_driver (*get) callback (which also
seems to be the case for creating cpuinfo_cur_freq attribute).
Tested the change with frequency switch stress test but was getting big
delta between set and get freq.
After passing "nohz=off" and commenting "wfi" in "cpu_do_idle()", the
delta is less. This confirms that more delta is due to AMU counters
stopping at "WFI".
+++ b/arch/arm64/kernel/idle.c
@@ -27,7 +27,7 @@ void noinstr cpu_do_idle(void)
arm_cpuidle_save_irq_context(&context);
dsb(sy);
- wfi();
+// wfi();
I am not sure if the expected behavior here is right.
In our tests, we compare the last set frequency against the re-generated
value from counters to confirm that the CPU is actually running at the
requested frequency and the counters are working correct. But that won't
happen with this change.
In [1] and later in the updated patch within [2], we are busy looping
on the target CPU and avoid WFI to get the actual frequency.
Please share what you think is the right expected behavior.
[1] https://lore.kernel.org/lkml/20230418113459.12860-7-sumitg@xxxxxxxxxx/
[2]
https://lore.kernel.org/lkml/cde1d8a9-3a21-e82b-7895-40603a14d898@xxxxxxxxxx/T/#mb898a75fd0c72d166b26b04da3ad162afe068a82
Observed another issue where CPUFREQ is coming too high when the
performance governor is set as default.
Below change solves that by using the new API arch_freq_get_on_cpu()
if present over the existing one, while verifying the currently set
frequency.
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 62face349fd2..2c74e70f701e 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1761,9 +1761,12 @@ static unsigned int
cpufreq_verify_current_freq(struct cpufreq_policy *policy, b
{
unsigned int new_freq;
- new_freq = cpufreq_driver->get(policy->cpu);
- if (!new_freq)
- return 0;
+ new_freq = arch_freq_get_on_cpu(policy->cpu);
+ if (!new_freq) {
+ new_freq = cpufreq_driver->get(policy->cpu);
+ if (!new_freq)
+ return 0;
+ }
Best Regards,
Sumit Gupta
