| Message ID | 20231027080400.56703-8-vincent.guittot@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | consolidate and cleanup CPU capacity | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-for-next-PR | fail | PR summary |
| conchuod/patch-7-test-1 | fail | .github/scripts/patches/build_rv32_defconfig.sh |
| conchuod/patch-7-test-2 | fail | .github/scripts/patches/build_rv64_clang_allmodconfig.sh |
| conchuod/patch-7-test-3 | fail | .github/scripts/patches/build_rv64_gcc_allmodconfig.sh |
| conchuod/patch-7-test-4 | fail | .github/scripts/patches/build_rv64_nommu_k210_defconfig.sh |
| conchuod/patch-7-test-5 | fail | .github/scripts/patches/build_rv64_nommu_virt_defconfig.sh |
| conchuod/patch-7-test-6 | warning | .github/scripts/patches/checkpatch.sh |
| conchuod/patch-7-test-7 | success | .github/scripts/patches/dtb_warn_rv64.sh |
| conchuod/patch-7-test-8 | success | .github/scripts/patches/header_inline.sh |
| conchuod/patch-7-test-9 | success | .github/scripts/patches/kdoc.sh |
| conchuod/patch-7-test-10 | success | .github/scripts/patches/module_param.sh |
| conchuod/patch-7-test-11 | success | .github/scripts/patches/verify_fixes.sh |
| conchuod/patch-7-test-12 | success | .github/scripts/patches/verify_signedoff.sh |
On Fri, Oct 27, 2023 at 10:04:00AM +0200, Vincent Guittot wrote: > Use the new capacity_ref_freq to set the ratio that is used by AMU for > computing the arch_scale_freq_capacity(). > This helps to keep everything aligned using the same reference for > computing CPUs capacity. > > The default value of the ratio (stored in per_cpu(arch_max_freq_scale) > ensures that arch_scale_freq_capacity() returns max capacity until it is > set to its correct value with the cpu capacity and capacity_ref_freq. > > Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> > --- > arch/arm64/kernel/topology.c | 26 ++++++++++++++------------ > drivers/base/arch_topology.c | 12 +++++++++++- > include/linux/arch_topology.h | 1 + > 3 files changed, 26 insertions(+), 13 deletions(-) > > diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c > index 817d788cd866..626031076b75 100644 > --- a/arch/arm64/kernel/topology.c > +++ b/arch/arm64/kernel/topology.c > @@ -82,7 +82,12 @@ int __init parse_acpi_topology(void) > #undef pr_fmt > #define pr_fmt(fmt) "AMU: " fmt > > -static DEFINE_PER_CPU_READ_MOSTLY(unsigned long, arch_max_freq_scale); > +/* > + * Ensure that amu_scale_freq_tick() will return SCHED_CAPACITY_SCALE until > + * the CPU capacity and its assosciated frequency have been correctly > + * initialized. > + */ > +static DEFINE_PER_CPU_READ_MOSTLY(unsigned long, arch_max_freq_scale) = 1UL << (2 * SCHED_CAPACITY_SHIFT); > static DEFINE_PER_CPU(u64, arch_const_cycles_prev); > static DEFINE_PER_CPU(u64, arch_core_cycles_prev); > static cpumask_var_t amu_fie_cpus; > @@ -112,14 +117,14 @@ static inline bool freq_counters_valid(int cpu) > return true; > } > > -static int freq_inv_set_max_ratio(int cpu, u64 max_rate, u64 ref_rate) > +void freq_inv_set_max_ratio(int cpu, u64 max_rate) > { > - u64 ratio; > + u64 ratio, ref_rate = arch_timer_get_rate(); > > if (unlikely(!max_rate || !ref_rate)) { > - pr_debug("CPU%d: invalid maximum or reference frequency.\n", > + WARN_ONCE(1, "CPU%d: invalid maximum or reference frequency.\n", > cpu); > - return -EINVAL; > + return; > } > > /* > @@ -139,12 +144,12 @@ static int freq_inv_set_max_ratio(int cpu, u64 max_rate, u64 ref_rate) > ratio = div64_u64(ratio, max_rate); > if (!ratio) { > WARN_ONCE(1, "Reference frequency too low.\n"); > - return -EINVAL; > + return; > } > > - per_cpu(arch_max_freq_scale, cpu) = (unsigned long)ratio; > + WRITE_ONCE(per_cpu(arch_max_freq_scale, cpu), (unsigned long)ratio); > > - return 0; > + return; > } > > static void amu_scale_freq_tick(void) > @@ -195,10 +200,7 @@ static void amu_fie_setup(const struct cpumask *cpus) > return; > > for_each_cpu(cpu, cpus) { > - if (!freq_counters_valid(cpu) || > - freq_inv_set_max_ratio(cpu, > - cpufreq_get_hw_max_freq(cpu) * 1000ULL, > - arch_timer_get_rate())) > + if (!freq_counters_valid(cpu)) > return; > } > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index d4bef370feb3..3cba7dc753b3 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -344,6 +344,10 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) > return !ret; > } > > +void __weak freq_inv_set_max_ratio(int cpu, u64 max_rate) > +{ > +} > + > #ifdef CONFIG_ACPI_CPPC_LIB > #include <acpi/cppc_acpi.h> > > @@ -381,6 +385,9 @@ void topology_init_cpu_capacity_cppc(void) > } > > for_each_possible_cpu(cpu) { > + freq_inv_set_max_ratio(cpu, > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > + The capacity_ref_freq set earlier will still lead to units mismatch, as at the point of calling topology_init_cpu_capacity_cppc the lowest & nominal frequencies will be provided in MHz (unless I have missed smth). This means that use of both, the capacity_ref_freq and the arch_freq_scale will generate unexpected results, so I guess this should get amended in the preceding patch (?) [1] --- BR B. [1] https://lore.kernel.org/linux-arm-kernel/20231027080400.56703-4-vincent.guittot@linaro.org/T/#m42daa167097edc190b1cfc05382c385ed801d909 > capacity = raw_capacity[cpu]; > capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, > capacity_scale); > @@ -422,8 +429,11 @@ init_cpu_capacity_callback(struct notifier_block *nb, > > cpumask_andnot(cpus_to_visit, cpus_to_visit, policy->related_cpus); > > - for_each_cpu(cpu, policy->related_cpus) > + for_each_cpu(cpu, policy->related_cpus) { > per_cpu(capacity_ref_freq, cpu) = policy->cpuinfo.max_freq; > + freq_inv_set_max_ratio(cpu, > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > + } > > if (cpumask_empty(cpus_to_visit)) { > topology_normalize_cpu_scale(); > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > index 38ca6c76af56..ffdf0b7c55fa 100644 > --- a/include/linux/arch_topology.h > +++ b/include/linux/arch_topology.h > @@ -99,6 +99,7 @@ void update_siblings_masks(unsigned int cpu); > void remove_cpu_topology(unsigned int cpuid); > void reset_cpu_topology(void); > int parse_acpi_topology(void); > +void freq_inv_set_max_ratio(int cpu, u64 max_rate); > #endif > > #endif /* _LINUX_ARCH_TOPOLOGY_H_ */ > -- > 2.34.1 >
Hi Beata, On Wed, 1 Nov 2023 at 00:59, Beata Michalska <beata.michalska@arm.com> wrote: > > On Fri, Oct 27, 2023 at 10:04:00AM +0200, Vincent Guittot wrote: > > Use the new capacity_ref_freq to set the ratio that is used by AMU for > > computing the arch_scale_freq_capacity(). > > This helps to keep everything aligned using the same reference for > > computing CPUs capacity. > > [..] > > @@ -381,6 +385,9 @@ void topology_init_cpu_capacity_cppc(void) > > } > > > > for_each_possible_cpu(cpu) { > > + freq_inv_set_max_ratio(cpu, > > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > > + > The capacity_ref_freq set earlier will still lead to units mismatch, > as at the point of calling topology_init_cpu_capacity_cppc the lowest & nominal > frequencies will be provided in MHz (unless I have missed smth). I don't get your point: the unit of per_cpu(capacity_freq_ref, cpu) is Khz For cppc, we have per_cpu(capacity_freq_ref, cpu) = cppc_perf_to_khz(&perf_caps, raw_capacity[cpu]); freq_inv_set_max_ratio() uses arch_timer_get_rate() which returns a freq in Hz and per_cpu(capacity_freq_ref, cpu) * HZ_PER_KHZ. to get a freq in Hz too. > This means that use of both, the capacity_ref_freq and the arch_freq_scale > will generate unexpected results, so I guess this should get amended in the > preceding patch (?) [1] > > --- > BR > B. > > [1] https://lore.kernel.org/linux-arm-kernel/20231027080400.56703-4-vincent.guittot@linaro.org/T/#m42daa167097edc190b1cfc05382c385ed801d909 > > > capacity = raw_capacity[cpu]; > > capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, > > capacity_scale); > > @@ -422,8 +429,11 @@ init_cpu_capacity_callback(struct notifier_block *nb, > > > > cpumask_andnot(cpus_to_visit, cpus_to_visit, policy->related_cpus); > > > > - for_each_cpu(cpu, policy->related_cpus) > > + for_each_cpu(cpu, policy->related_cpus) { > > per_cpu(capacity_ref_freq, cpu) = policy->cpuinfo.max_freq; > > + freq_inv_set_max_ratio(cpu, > > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > > + } > > > > if (cpumask_empty(cpus_to_visit)) { > > topology_normalize_cpu_scale(); > > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > > index 38ca6c76af56..ffdf0b7c55fa 100644 > > --- a/include/linux/arch_topology.h > > +++ b/include/linux/arch_topology.h > > @@ -99,6 +99,7 @@ void update_siblings_masks(unsigned int cpu); > > void remove_cpu_topology(unsigned int cpuid); > > void reset_cpu_topology(void); > > int parse_acpi_topology(void); > > +void freq_inv_set_max_ratio(int cpu, u64 max_rate); > > #endif > > > > #endif /* _LINUX_ARCH_TOPOLOGY_H_ */ > > -- > > 2.34.1 > >
Hi Vincent, On Wed, Nov 01, 2023 at 10:04:27AM +0100, Vincent Guittot wrote: > Hi Beata, > > On Wed, 1 Nov 2023 at 00:59, Beata Michalska <beata.michalska@arm.com> wrote: > > > > On Fri, Oct 27, 2023 at 10:04:00AM +0200, Vincent Guittot wrote: > > > Use the new capacity_ref_freq to set the ratio that is used by AMU for > > > computing the arch_scale_freq_capacity(). > > > This helps to keep everything aligned using the same reference for > > > computing CPUs capacity. > > > > > [..] > > > > @@ -381,6 +385,9 @@ void topology_init_cpu_capacity_cppc(void) > > > } > > > > > > for_each_possible_cpu(cpu) { > > > + freq_inv_set_max_ratio(cpu, > > > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > > > + > > The capacity_ref_freq set earlier will still lead to units mismatch, > > as at the point of calling topology_init_cpu_capacity_cppc the lowest & nominal > > frequencies will be provided in MHz (unless I have missed smth). > > I don't get your point: > the unit of per_cpu(capacity_freq_ref, cpu) is Khz > > For cppc, we have > per_cpu(capacity_freq_ref, cpu) = cppc_perf_to_khz(&perf_caps, > raw_capacity[cpu]); > > freq_inv_set_max_ratio() uses > arch_timer_get_rate() which returns a freq in Hz > and > per_cpu(capacity_freq_ref, cpu) * HZ_PER_KHZ. to get a freq in Hz too. > Apologies, I should have been more verbose here. Before the change made in [1] the cppc_perf_to_khz was indeed operating on lowest & nominal frequency values expressed in kHZ, as those were appropriately amended: cppc_cpufreq_cpu_init |__> cppc_cpufreq_get_cpu_data: |__> ... /* Convert the lowest and nominal freq from MHz to KHz */ cpu_data->perf_caps.lowest_freq *= 1000; cpu_data->perf_caps.nominal_freq *= 1000; So far cppc_perf_to_khz (previously cppc_cpufreq_perf_to_khz) was being called with the post-processed CPC data (through cppc_cpufreq_get_cpu_data) and thus guaranteed to be operating on values in kHz. With the new changes the cppc_perf_to_khz will operate on raw CPC data, which are expressed in MHz.(as per acpi spec) --- Best Regards B. > > This means that use of both, the capacity_ref_freq and the arch_freq_scale > > will generate unexpected results, so I guess this should get amended in the > > preceding patch (?) [1] > > > > --- > > BR > > B. > > > > [1] https://lore.kernel.org/linux-arm-kernel/20231027080400.56703-4-vincent.guittot@linaro.org/T/#m42daa167097edc190b1cfc05382c385ed801d909 > > > > > capacity = raw_capacity[cpu]; > > > capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, > > > capacity_scale); > > > @@ -422,8 +429,11 @@ init_cpu_capacity_callback(struct notifier_block *nb, > > > > > > cpumask_andnot(cpus_to_visit, cpus_to_visit, policy->related_cpus); > > > > > > - for_each_cpu(cpu, policy->related_cpus) > > > + for_each_cpu(cpu, policy->related_cpus) { > > > per_cpu(capacity_ref_freq, cpu) = policy->cpuinfo.max_freq; > > > + freq_inv_set_max_ratio(cpu, > > > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > > > + } > > > > > > if (cpumask_empty(cpus_to_visit)) { > > > topology_normalize_cpu_scale(); > > > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > > > index 38ca6c76af56..ffdf0b7c55fa 100644 > > > --- a/include/linux/arch_topology.h > > > +++ b/include/linux/arch_topology.h > > > @@ -99,6 +99,7 @@ void update_siblings_masks(unsigned int cpu); > > > void remove_cpu_topology(unsigned int cpuid); > > > void reset_cpu_topology(void); > > > int parse_acpi_topology(void); > > > +void freq_inv_set_max_ratio(int cpu, u64 max_rate); > > > #endif > > > > > > #endif /* _LINUX_ARCH_TOPOLOGY_H_ */ > > > -- > > > 2.34.1 > > >
On Thu, 2 Nov 2023 at 11:31, Beata Michalska <beata.michalska@arm.com> wrote: > > Hi Vincent, > > On Wed, Nov 01, 2023 at 10:04:27AM +0100, Vincent Guittot wrote: > > Hi Beata, > > > > On Wed, 1 Nov 2023 at 00:59, Beata Michalska <beata.michalska@arm.com> wrote: > > > > > > On Fri, Oct 27, 2023 at 10:04:00AM +0200, Vincent Guittot wrote: > > > > Use the new capacity_ref_freq to set the ratio that is used by AMU for > > > > computing the arch_scale_freq_capacity(). > > > > This helps to keep everything aligned using the same reference for > > > > computing CPUs capacity. > > > > > > > > [..] > > > > > > @@ -381,6 +385,9 @@ void topology_init_cpu_capacity_cppc(void) > > > > } > > > > > > > > for_each_possible_cpu(cpu) { > > > > + freq_inv_set_max_ratio(cpu, > > > > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > > > > + > > > The capacity_ref_freq set earlier will still lead to units mismatch, > > > as at the point of calling topology_init_cpu_capacity_cppc the lowest & nominal > > > frequencies will be provided in MHz (unless I have missed smth). > > > > I don't get your point: > > the unit of per_cpu(capacity_freq_ref, cpu) is Khz > > > > For cppc, we have > > per_cpu(capacity_freq_ref, cpu) = cppc_perf_to_khz(&perf_caps, > > raw_capacity[cpu]); > > > > freq_inv_set_max_ratio() uses > > arch_timer_get_rate() which returns a freq in Hz > > and > > per_cpu(capacity_freq_ref, cpu) * HZ_PER_KHZ. to get a freq in Hz too. > > > Apologies, I should have been more verbose here. > Before the change made in [1] the cppc_perf_to_khz was indeed operating on > lowest & nominal frequency values expressed in kHZ, as those were appropriately > amended: > cppc_cpufreq_cpu_init > |__> cppc_cpufreq_get_cpu_data: > |__> ... > /* Convert the lowest and nominal freq from MHz to KHz */ > cpu_data->perf_caps.lowest_freq *= 1000; > cpu_data->perf_caps.nominal_freq *= 1000; > > So far cppc_perf_to_khz (previously cppc_cpufreq_perf_to_khz) was being called > with the post-processed CPC data (through cppc_cpufreq_get_cpu_data) and thus > guaranteed to be operating on values in kHz. > With the new changes the cppc_perf_to_khz will operate on raw CPC data, > which are expressed in MHz.(as per acpi spec) Ok, thanks for the explanation. I haven't noticed that cppc_cpufreq was processing the raw CPC data (*1000) before using them. I'm going to fix that. > > --- > Best Regards > B. > > > This means that use of both, the capacity_ref_freq and the arch_freq_scale > > > will generate unexpected results, so I guess this should get amended in the > > > preceding patch (?) [1] > > > > > > --- > > > BR > > > B. > > > > > > [1] https://lore.kernel.org/linux-arm-kernel/20231027080400.56703-4-vincent.guittot@linaro.org/T/#m42daa167097edc190b1cfc05382c385ed801d909 > > > > > > > capacity = raw_capacity[cpu]; > > > > capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, > > > > capacity_scale); > > > > @@ -422,8 +429,11 @@ init_cpu_capacity_callback(struct notifier_block *nb, > > > > > > > > cpumask_andnot(cpus_to_visit, cpus_to_visit, policy->related_cpus); > > > > > > > > - for_each_cpu(cpu, policy->related_cpus) > > > > + for_each_cpu(cpu, policy->related_cpus) { > > > > per_cpu(capacity_ref_freq, cpu) = policy->cpuinfo.max_freq; > > > > + freq_inv_set_max_ratio(cpu, > > > > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > > > > + } > > > > > > > > if (cpumask_empty(cpus_to_visit)) { > > > > topology_normalize_cpu_scale(); > > > > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > > > > index 38ca6c76af56..ffdf0b7c55fa 100644 > > > > --- a/include/linux/arch_topology.h > > > > +++ b/include/linux/arch_topology.h > > > > @@ -99,6 +99,7 @@ void update_siblings_masks(unsigned int cpu); > > > > void remove_cpu_topology(unsigned int cpuid); > > > > void reset_cpu_topology(void); > > > > int parse_acpi_topology(void); > > > > +void freq_inv_set_max_ratio(int cpu, u64 max_rate); > > > > #endif > > > > > > > > #endif /* _LINUX_ARCH_TOPOLOGY_H_ */ > > > > -- > > > > 2.34.1 > > > >
On Thu, 2 Nov 2023 at 11:40, Vincent Guittot <vincent.guittot@linaro.org> wrote: > > On Thu, 2 Nov 2023 at 11:31, Beata Michalska <beata.michalska@arm.com> wrote: > > > > Hi Vincent, > > > > On Wed, Nov 01, 2023 at 10:04:27AM +0100, Vincent Guittot wrote: > > > Hi Beata, > > > > > > On Wed, 1 Nov 2023 at 00:59, Beata Michalska <beata.michalska@arm.com> wrote: > > > > > > > > On Fri, Oct 27, 2023 at 10:04:00AM +0200, Vincent Guittot wrote: > > > > > Use the new capacity_ref_freq to set the ratio that is used by AMU for > > > > > computing the arch_scale_freq_capacity(). > > > > > This helps to keep everything aligned using the same reference for > > > > > computing CPUs capacity. > > > > > > > > > > > [..] > > > > > > > > @@ -381,6 +385,9 @@ void topology_init_cpu_capacity_cppc(void) > > > > > } > > > > > > > > > > for_each_possible_cpu(cpu) { > > > > > + freq_inv_set_max_ratio(cpu, > > > > > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > > > > > + > > > > The capacity_ref_freq set earlier will still lead to units mismatch, > > > > as at the point of calling topology_init_cpu_capacity_cppc the lowest & nominal > > > > frequencies will be provided in MHz (unless I have missed smth). > > > > > > I don't get your point: > > > the unit of per_cpu(capacity_freq_ref, cpu) is Khz > > > > > > For cppc, we have > > > per_cpu(capacity_freq_ref, cpu) = cppc_perf_to_khz(&perf_caps, > > > raw_capacity[cpu]); > > > > > > freq_inv_set_max_ratio() uses > > > arch_timer_get_rate() which returns a freq in Hz > > > and > > > per_cpu(capacity_freq_ref, cpu) * HZ_PER_KHZ. to get a freq in Hz too. > > > > > Apologies, I should have been more verbose here. > > Before the change made in [1] the cppc_perf_to_khz was indeed operating on > > lowest & nominal frequency values expressed in kHZ, as those were appropriately > > amended: > > cppc_cpufreq_cpu_init > > |__> cppc_cpufreq_get_cpu_data: > > |__> ... > > /* Convert the lowest and nominal freq from MHz to KHz */ > > cpu_data->perf_caps.lowest_freq *= 1000; > > cpu_data->perf_caps.nominal_freq *= 1000; > > > > So far cppc_perf_to_khz (previously cppc_cpufreq_perf_to_khz) was being called > > with the post-processed CPC data (through cppc_cpufreq_get_cpu_data) and thus > > guaranteed to be operating on values in kHz. > > With the new changes the cppc_perf_to_khz will operate on raw CPC data, > > which are expressed in MHz.(as per acpi spec) > > Ok, thanks for the explanation. I haven't noticed that cppc_cpufreq > was processing the raw CPC data (*1000) before using them. I'm going > to fix that. With cppc_perf_to_khz() being moved into the generic cppc_acpi.c, we can't change perf_caps.nominal_freq and perf_caps.lowest_freq directly. So, I'm planning to add the conversion from MHz to kHz in the functions cppc_perf_to_khz() and cppc_khz_to_perf() and remove the 2 conversions above in cppc_cpufreq_get_cpu_data(). > > > > > > --- > > Best Regards > > B. > > > > This means that use of both, the capacity_ref_freq and the arch_freq_scale > > > > will generate unexpected results, so I guess this should get amended in the > > > > preceding patch (?) [1] > > > > > > > > --- > > > > BR > > > > B. > > > > > > > > [1] https://lore.kernel.org/linux-arm-kernel/20231027080400.56703-4-vincent.guittot@linaro.org/T/#m42daa167097edc190b1cfc05382c385ed801d909 > > > > > > > > > capacity = raw_capacity[cpu]; > > > > > capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, > > > > > capacity_scale); > > > > > @@ -422,8 +429,11 @@ init_cpu_capacity_callback(struct notifier_block *nb, > > > > > > > > > > cpumask_andnot(cpus_to_visit, cpus_to_visit, policy->related_cpus); > > > > > > > > > > - for_each_cpu(cpu, policy->related_cpus) > > > > > + for_each_cpu(cpu, policy->related_cpus) { > > > > > per_cpu(capacity_ref_freq, cpu) = policy->cpuinfo.max_freq; > > > > > + freq_inv_set_max_ratio(cpu, > > > > > + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); > > > > > + } > > > > > > > > > > if (cpumask_empty(cpus_to_visit)) { > > > > > topology_normalize_cpu_scale(); > > > > > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > > > > > index 38ca6c76af56..ffdf0b7c55fa 100644 > > > > > --- a/include/linux/arch_topology.h > > > > > +++ b/include/linux/arch_topology.h > > > > > @@ -99,6 +99,7 @@ void update_siblings_masks(unsigned int cpu); > > > > > void remove_cpu_topology(unsigned int cpuid); > > > > > void reset_cpu_topology(void); > > > > > int parse_acpi_topology(void); > > > > > +void freq_inv_set_max_ratio(int cpu, u64 max_rate); > > > > > #endif > > > > > > > > > > #endif /* _LINUX_ARCH_TOPOLOGY_H_ */ > > > > > -- > > > > > 2.34.1 > > > > >
diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c index 817d788cd866..626031076b75 100644 --- a/arch/arm64/kernel/topology.c +++ b/arch/arm64/kernel/topology.c @@ -82,7 +82,12 @@ int __init parse_acpi_topology(void) #undef pr_fmt #define pr_fmt(fmt) "AMU: " fmt -static DEFINE_PER_CPU_READ_MOSTLY(unsigned long, arch_max_freq_scale); +/* + * Ensure that amu_scale_freq_tick() will return SCHED_CAPACITY_SCALE until + * the CPU capacity and its assosciated frequency have been correctly + * initialized. + */ +static DEFINE_PER_CPU_READ_MOSTLY(unsigned long, arch_max_freq_scale) = 1UL << (2 * SCHED_CAPACITY_SHIFT); static DEFINE_PER_CPU(u64, arch_const_cycles_prev); static DEFINE_PER_CPU(u64, arch_core_cycles_prev); static cpumask_var_t amu_fie_cpus; @@ -112,14 +117,14 @@ static inline bool freq_counters_valid(int cpu) return true; } -static int freq_inv_set_max_ratio(int cpu, u64 max_rate, u64 ref_rate) +void freq_inv_set_max_ratio(int cpu, u64 max_rate) { - u64 ratio; + u64 ratio, ref_rate = arch_timer_get_rate(); if (unlikely(!max_rate || !ref_rate)) { - pr_debug("CPU%d: invalid maximum or reference frequency.\n", + WARN_ONCE(1, "CPU%d: invalid maximum or reference frequency.\n", cpu); - return -EINVAL; + return; } /* @@ -139,12 +144,12 @@ static int freq_inv_set_max_ratio(int cpu, u64 max_rate, u64 ref_rate) ratio = div64_u64(ratio, max_rate); if (!ratio) { WARN_ONCE(1, "Reference frequency too low.\n"); - return -EINVAL; + return; } - per_cpu(arch_max_freq_scale, cpu) = (unsigned long)ratio; + WRITE_ONCE(per_cpu(arch_max_freq_scale, cpu), (unsigned long)ratio); - return 0; + return; } static void amu_scale_freq_tick(void) @@ -195,10 +200,7 @@ static void amu_fie_setup(const struct cpumask *cpus) return; for_each_cpu(cpu, cpus) { - if (!freq_counters_valid(cpu) || - freq_inv_set_max_ratio(cpu, - cpufreq_get_hw_max_freq(cpu) * 1000ULL, - arch_timer_get_rate())) + if (!freq_counters_valid(cpu)) return; } diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index d4bef370feb3..3cba7dc753b3 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -344,6 +344,10 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) return !ret; } +void __weak freq_inv_set_max_ratio(int cpu, u64 max_rate) +{ +} + #ifdef CONFIG_ACPI_CPPC_LIB #include <acpi/cppc_acpi.h> @@ -381,6 +385,9 @@ void topology_init_cpu_capacity_cppc(void) } for_each_possible_cpu(cpu) { + freq_inv_set_max_ratio(cpu, + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); + capacity = raw_capacity[cpu]; capacity = div64_u64(capacity << SCHED_CAPACITY_SHIFT, capacity_scale); @@ -422,8 +429,11 @@ init_cpu_capacity_callback(struct notifier_block *nb, cpumask_andnot(cpus_to_visit, cpus_to_visit, policy->related_cpus); - for_each_cpu(cpu, policy->related_cpus) + for_each_cpu(cpu, policy->related_cpus) { per_cpu(capacity_ref_freq, cpu) = policy->cpuinfo.max_freq; + freq_inv_set_max_ratio(cpu, + per_cpu(capacity_ref_freq, cpu) * HZ_PER_KHZ); + } if (cpumask_empty(cpus_to_visit)) { topology_normalize_cpu_scale(); diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h index 38ca6c76af56..ffdf0b7c55fa 100644 --- a/include/linux/arch_topology.h +++ b/include/linux/arch_topology.h @@ -99,6 +99,7 @@ void update_siblings_masks(unsigned int cpu); void remove_cpu_topology(unsigned int cpuid); void reset_cpu_topology(void); int parse_acpi_topology(void); +void freq_inv_set_max_ratio(int cpu, u64 max_rate); #endif #endif /* _LINUX_ARCH_TOPOLOGY_H_ */
Use the new capacity_ref_freq to set the ratio that is used by AMU for computing the arch_scale_freq_capacity(). This helps to keep everything aligned using the same reference for computing CPUs capacity. The default value of the ratio (stored in per_cpu(arch_max_freq_scale) ensures that arch_scale_freq_capacity() returns max capacity until it is set to its correct value with the cpu capacity and capacity_ref_freq. Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org> --- arch/arm64/kernel/topology.c | 26 ++++++++++++++------------ drivers/base/arch_topology.c | 12 +++++++++++- include/linux/arch_topology.h | 1 + 3 files changed, 26 insertions(+), 13 deletions(-)