Re: [PATCH] sched/fair: Only apply cpufreq pressure where frequency is invariant

[Jianyong Wu <jianyong.wu@outlook.com>]

Hi Vincent, Hongyan,

Thanks for your comments.

My original commit message did not clearly describe the concrete issue
being fixed, and its explanation based on frequency invariance was not
correct. After looking into this further, I found that the issue I
observed has a different cause: the cpuinfo.max_freq fallback added by
d2d5c129d07e ("cpufreq: Make cpufreq_update_pressure() fall back to
cpuinfo.max_freq").

The commit message says:

  However, in the absence of arch_scale_freq_ref(), it is reasonable
  to assume that cpuinfo.max_freq is the maximum sustainable frequency
  for the given cpufreq policy.

That assumption does not always hold.

On an x86 server using acpi-cpufreq, cpuinfo.max_freq includes the
autonomous boost frequency, while policy->max is resolved to the
highest selectable _PSS state. With boost enabled and policy->max
unchanged at that state, the measured CPU frequency can still exceed
policy->max. Thus, policy->max does not represent an effective hardware
maximum-frequency cap in this case.

Nevertheless, the cpuinfo.max_freq fallback makes
cpufreq_update_pressure() calculate positive pressure for every policy,
although no effective maximum-frequency restriction has been applied.

The underlying issue is that cpuinfo.max_freq is the maximum possible
operating frequency and may include an autonomous boost frequency,
whereas policy->max may represent the highest selectable _PSS state.
Consequently, policy->max < cpuinfo.max_freq does not necessarily mean
that the available CPU capacity has been capped.


Therefore, this patch checks the wrong condition and is not the right fix. I will drop it.

Instead, I am investigating a fix for the reference-frequency fallback
in the cpufreq subsystem. One possible approach is to use the highest
non-boost frequency-table entry when arch_scale_freq_ref() is
unavailable, and only fall back to cpuinfo.max_freq for drivers without
such an entry.

Does that approach sound reasonable?

Thanks
Jianyong

>