| Message ID | 20250220091015.2319901-1-beata.michalska@arm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] arm64: Utilize for_each_cpu_wrap for reference lookup | expand |
On Thu, 20 Feb 2025 09:10:15 +0000, Beata Michalska wrote: > While searching for a reference CPU within a given policy, > arch_freq_get_on_cpu relies on cpumask_next_wrap to iterate over > all available CPUs and to ensure each is verified only once. > Recent changes to cpumask_next_wrap will handle the latter no more, > so switching to for_each_cpu_wrap, which preserves expected behavior > while ensuring compatibility with the updates. > Not to mention that when iterating over each CPU, using a dedicated > iterator is preferable to an open-coded loop. > > [...] Applied to arm64 (for-next/amuv1-avg-freq), thanks! [1/1] arm64: Utilize for_each_cpu_wrap for reference lookup https://git.kernel.org/arm64/c/20711efa91e8
On Thu, Feb 20, 2025 at 09:10:15AM +0000, Beata Michalska wrote: > While searching for a reference CPU within a given policy, > arch_freq_get_on_cpu relies on cpumask_next_wrap to iterate over > all available CPUs and to ensure each is verified only once. > Recent changes to cpumask_next_wrap will handle the latter no more, > so switching to for_each_cpu_wrap, which preserves expected behavior > while ensuring compatibility with the updates. > Not to mention that when iterating over each CPU, using a dedicated > iterator is preferable to an open-coded loop. > > Fixes: 16d1e27475f6 ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu") > Signed-off-by: Beata Michalska <beata.michalska@arm.com> Reviewed-by: Yury Norov [NVIDIA] <yury.norov@gmail.com> > --- > v2: > Updated commit message > > arch/arm64/kernel/topology.c | 16 ++++++++++------ > 1 file changed, 10 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c > index a09b0551ec59..9e3583720668 100644 > --- a/arch/arm64/kernel/topology.c > +++ b/arch/arm64/kernel/topology.c > @@ -254,7 +254,7 @@ int arch_freq_get_on_cpu(int cpu) > if (!housekeeping_cpu(cpu, HK_TYPE_TICK) || > time_is_before_jiffies(last_update + msecs_to_jiffies(AMU_SAMPLE_EXP_MS))) { > struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); > - int ref_cpu = cpu; > + int ref_cpu; > > if (!policy) > return -EINVAL; > @@ -265,11 +265,15 @@ int arch_freq_get_on_cpu(int cpu) > return -EOPNOTSUPP; > } > > - do { > - ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus, > - start_cpu, true); > - > - } while (ref_cpu < nr_cpu_ids && idle_cpu(ref_cpu)); > + for_each_cpu_wrap(ref_cpu, policy->cpus, cpu + 1) { > + if (ref_cpu == start_cpu) { > + /* Prevent verifying same CPU twice */ > + ref_cpu = nr_cpu_ids; > + break; > + } > + if (!idle_cpu(ref_cpu)) > + break; > + } > > cpufreq_cpu_put(policy); > > -- > 2.25.1
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c index a09b0551ec59..9e3583720668 100644 --- a/arch/arm64/kernel/topology.c +++ b/arch/arm64/kernel/topology.c @@ -254,7 +254,7 @@ int arch_freq_get_on_cpu(int cpu) if (!housekeeping_cpu(cpu, HK_TYPE_TICK) || time_is_before_jiffies(last_update + msecs_to_jiffies(AMU_SAMPLE_EXP_MS))) { struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); - int ref_cpu = cpu; + int ref_cpu; if (!policy) return -EINVAL; @@ -265,11 +265,15 @@ int arch_freq_get_on_cpu(int cpu) return -EOPNOTSUPP; } - do { - ref_cpu = cpumask_next_wrap(ref_cpu, policy->cpus, - start_cpu, true); - - } while (ref_cpu < nr_cpu_ids && idle_cpu(ref_cpu)); + for_each_cpu_wrap(ref_cpu, policy->cpus, cpu + 1) { + if (ref_cpu == start_cpu) { + /* Prevent verifying same CPU twice */ + ref_cpu = nr_cpu_ids; + break; + } + if (!idle_cpu(ref_cpu)) + break; + } cpufreq_cpu_put(policy);
While searching for a reference CPU within a given policy, arch_freq_get_on_cpu relies on cpumask_next_wrap to iterate over all available CPUs and to ensure each is verified only once. Recent changes to cpumask_next_wrap will handle the latter no more, so switching to for_each_cpu_wrap, which preserves expected behavior while ensuring compatibility with the updates. Not to mention that when iterating over each CPU, using a dedicated iterator is preferable to an open-coded loop. Fixes: 16d1e27475f6 ("arm64: Provide an AMU-based version of arch_freq_get_on_cpu") Signed-off-by: Beata Michalska <beata.michalska@arm.com> --- v2: Updated commit message arch/arm64/kernel/topology.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)