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

[Zhongqiu Han]

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