Re: [PATCH v3 1/2] arch_topology: seed capacity_freq_ref with highest available frequency

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

On 10/13/2026 2:45 AM, Dietmar Eggemann wrote:
> On 11.10.26 16:16, Zhongqiu Han wrote:
>> On 9/29/2026 7:38 PM, Ananthu C V wrote:
> 
> [...]
> 
>> Besides, could you comment patch v2 Dietmar's comments? It seems that
>> patch 1/2 exposes a pre-existing ordering issue. On DT systems,
>> cpu_scale is normalized only from the CPUFREQ_CREATE_POLICY notification
>> of the last policy, so the cpufreq pressure of every other policy is
>> computed in cpufreq_init_policy() from the raw, not yet normalized,
>> capacity, and stays cached until the next cpufreq_set_policy() for that
>> policy. So far this was harmless because capacity_freq_ref == policy-
>>> max at boot, so the pressure was 0 regardless of the capacity. With
>> 1/2, capacity_freq_ref covers the boost frequencies while boost is
>> disabled at boot, so policy->max < capacity_freq_ref becomes the common
>> case and the stale value becomes visible (102 vs 79 on Juno R0, as
>> reported by Dietmar). Perhaps the limits of all policies could be
>> refreshed once from parsing_done_workfn(), after the capacities have
>> been normalized?
>>
>> --- a/drivers/base/arch_topology.c
>> +++ b/drivers/base/arch_topology.c
>> @@ -454,9 +454,27 @@ core_initcall(register_cpufreq_notifier);
>>
>>   static void parsing_done_workfn(struct work_struct *work)
>>   {
>> +    int cpu;
>> +
>>       cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
>>                        CPUFREQ_POLICY_NOTIFIER);
>>       free_cpumask_var(cpus_to_visit);
>> +
>> +    /*
>> +     * The cpufreq pressure of all policies but the last one has been
>> +     * computed before topology_normalize_cpu_scale(), so refresh it now
>> +     * that cpu_scale is final.
>> +     */
>> +    for_each_possible_cpu(cpu) {
>> +        struct cpufreq_policy *policy __free(put_cpufreq_policy) =
>> +            cpufreq_cpu_get(cpu);
>> +
>> +        if (!policy || cpu != policy->cpu)
>> +            continue;
>> +
>> +        guard(cpufreq_policy_write)(policy);
>> +        refresh_frequency_limits(policy);
>> +    }
> Yes, IMHO, something like this is needed.
> 
> [    1.169158] cpufreq_notifier_max(): current=[kworker/u26:0 46] policy->related_cpus=[]												(1)
> [    1.169209] init_cpu_capacity_callback(): current=[kworker/u26:0 46] policy->related_cpus=[0,3-5]
> [    1.169992] cpufreq_update_pressure(): current=[kworker/u26:0 46] policy->related_cpus=[0,3-5] policy->max=700000 pressure=102 max_capacity=578 capped_freq=700000 max_freq=850000
> [    1.170203] cpufreq_update_pressure(): current=[kworker/2:2 67] policy->related_cpus=[0,3-5] policy->max=700000 pressure=102 max_capacity=578 capped_freq=700000 max_freq=850000 	(2)
> [    1.170835] cpufreq_notifier_max(): current=[kworker/u26:0 46] policy->related_cpus=[]												(1)
> [    1.170872] init_cpu_capacity_callback(): current=[kworker/u26:0 46] policy->related_cpus=[1-2]
> [    1.174516] cpufreq_update_pressure(): current=[kworker/u26:0 46] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000
> [    1.175261] cpufreq_update_pressure(): current=[kworker/2:2 67] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000	(2)
> [    1.187698] cpufreq_update_pressure(): current=[kworker/2:0 28] policy->related_cpus=[0,3-5] policy->max=700000 pressure=79 max_capacity=446 capped_freq=700000 max_freq=850000	(3)
> [    1.187847] cpufreq_update_pressure(): current=[kworker/2:0 28] policy->related_cpus=[1-2] policy->max=800000 pressure=280 max_capacity=1024 capped_freq=800000 max_freq=1100000	(3)
> 
> The work scheduled in (1) also still operates on raw capacity (2).
> 
> (3) shows your extension in parsing_done_workfn() work function which
>      gets scheduled when all CPUs are initialised, so
>      topology_normalize_cpu_scale() had been called.
> 
> Not sure whether we can call cpufreq_update_limits(cpu) there instead
> (maybe extended with a 'only do this for policy->cpu'?). So we won't
> have to expose cpufreq internals in /drivers/base/arch_topology.c.

Thanks for the test verification and the discussion.

Unless I'm missing something, it seems to me that
cpufreq_update_limits() is mainly intended for platform limit changes.
When a driver implements ->update_limits(), it is up to the driver
whether the policy gets refreshed at all (it seems that amd-pstate only
updates the preferred core ranking). Otherwise, it goes through
cpufreq_policy_refresh(), which calls ->get() first and WARNs and skips
the refresh if that returns 0, for example when the policy goes inactive
under us.

In comparison, refresh_frequency_limits() seems to match what we need
here more closely: it simply re-applies the limits and therefore
recomputes the pressure, without going through any of the above.

However, calling refresh_frequency_limits() directly from drivers/base
arch_topology.c does expose some cpufreq internals there. I wonder if it
would make sense to add a wrapper around cpufreq_update_pressure()
directly, something like:

--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -454,9 +454,19 @@ core_initcall(register_cpufreq_notifier);

  static void parsing_done_workfn(struct work_struct *work)
  {
+	int cpu;
+
  	cpufreq_unregister_notifier(&init_cpu_capacity_notifier,
  					 CPUFREQ_POLICY_NOTIFIER);
  	free_cpumask_var(cpus_to_visit);
+
+	/*
+	 * The cpufreq pressure of all policies but the last one has been
+	 * computed before topology_normalize_cpu_scale(), so update it now
+	 * that cpu_scale is final.
+	 */
+	for_each_possible_cpu(cpu)
+		cpufreq_refresh_pressure(cpu);
  }

--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2611,6 +2611,27 @@ static void cpufreq_update_pressure(struct 
cpufreq_policy *policy)
  		WRITE_ONCE(per_cpu(cpufreq_pressure, cpu), pressure);
  }

+/**
+ * cpufreq_refresh_pressure() - Recompute cpufreq pressure for a CPU's 
policy.
+ * @cpu: CPU whose policy's cpufreq pressure is to be recomputed.
+ *
+ * To be called when arch_scale_cpu_capacity() of @cpu has changed while
+ * the limits of its policy have not.
+ *
+ * Takes the policy rwsem, so the caller must not hold it.
+ */
+void cpufreq_refresh_pressure(unsigned int cpu)
+{
+	struct cpufreq_policy *policy __free(put_cpufreq_policy) = 
cpufreq_cpu_get(cpu);
+	if (!policy)
+		return;
+
+	guard(cpufreq_policy_write)(policy);
+
+	if (!policy_is_inactive(policy))
+		cpufreq_update_pressure(policy);
+}
+

--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -251,6 +251,7 @@ u64 get_cpu_idle_time(unsigned int cpu, u64 *wall, 
int io_busy);
  void refresh_frequency_limits(struct cpufreq_policy *policy);
  void cpufreq_update_policy(unsigned int cpu);
  void cpufreq_update_limits(unsigned int cpu);
+void cpufreq_refresh_pressure(unsigned int cpu);
  bool have_governor_per_policy(void);
  bool cpufreq_supports_freq_invariance(void);
  struct kobject *get_governor_parent_kobj(struct cpufreq_policy *policy);


-- 
Thx and BRs,
Zhongqiu Han