* [PATCH v2 0/3] arch_topology, ACPI: populate cpu capacity from CPPC @ 2021-08-24 10:56 Ionela Voinescu 2021-08-24 10:56 ` [PATCH v2 1/3] x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc Ionela Voinescu ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Ionela Voinescu @ 2021-08-24 10:56 UTC (permalink / raw) To: Sudeep Holla, Rafael J . Wysocki, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley Cc: linux-kernel, linux-acpi, linux-arm-kernel Hi all, Apologies for the long delay in posting v2. v1->v2: - v1 can be found at [1] - Changed debug prints to the format used on the DT path - s/init_cpu_capacity_cppc/topology_init_cpu_capacity_cppc Patches are based on v5.14-rc7. The patches have been build tested on x86 and more thoroughly tested on Juno R2 (arm64), which uses the new functionality, with the following results: root@buildroot:~# dmesg | grep cpu_capacity [ 2.191152] cpu_capacity: CPU0 cpu_capacity=38300 (raw). [ 2.196482] cpu_capacity: CPU1 cpu_capacity=38300 (raw). [ 2.201809] cpu_capacity: CPU2 cpu_capacity=38300 (raw). [ 2.207136] cpu_capacity: CPU3 cpu_capacity=38300 (raw). [ 2.212463] cpu_capacity: CPU4 cpu_capacity=102400 (raw). [ 2.217877] cpu_capacity: CPU5 cpu_capacity=102400 (raw). [ 2.223291] cpu_capacity: capacity_scale=102400 [ 2.227834] cpu_capacity: CPU0 cpu_capacity=383 [ 2.232376] cpu_capacity: CPU1 cpu_capacity=383 [ 2.236919] cpu_capacity: CPU2 cpu_capacity=383 [ 2.241462] cpu_capacity: CPU3 cpu_capacity=383 [ 2.246004] cpu_capacity: CPU4 cpu_capacity=1024 [ 2.250634] cpu_capacity: CPU5 cpu_capacity=1024 [ 2.255321] cpu_capacity: cpu_capacity initialization done root@buildroot:~# tail -n +1 /sys/devices/system/cpu/cpu*/cpu_capacity ==> /sys/devices/system/cpu/cpu0/cpu_capacity <== 383 ==> /sys/devices/system/cpu/cpu1/cpu_capacity <== 383 ==> /sys/devices/system/cpu/cpu2/cpu_capacity <== 383 ==> /sys/devices/system/cpu/cpu3/cpu_capacity <== 383 ==> /sys/devices/system/cpu/cpu4/cpu_capacity <== 1024 ==> /sys/devices/system/cpu/cpu5/cpu_capacity <== 1024 [1] https://lore.kernel.org/lkml/20210514095339.12979-1-ionela.voinescu@arm.com/ Thanks, Ionela. Ionela Voinescu (3): x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc arch_topology: obtain cpu capacity using information from CPPC arm64, topology: enable use of init_cpu_capacity_cppc() arch/arm64/include/asm/topology.h | 4 ++++ arch/x86/include/asm/topology.h | 2 +- drivers/acpi/cppc_acpi.c | 6 ++--- drivers/base/arch_topology.c | 37 +++++++++++++++++++++++++++++++ include/linux/arch_topology.h | 4 ++++ 5 files changed, 49 insertions(+), 4 deletions(-) -- 2.29.2.dirty ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc 2021-08-24 10:56 [PATCH v2 0/3] arch_topology, ACPI: populate cpu capacity from CPPC Ionela Voinescu @ 2021-08-24 10:56 ` Ionela Voinescu 2021-08-25 17:48 ` Rafael J. Wysocki 2021-08-24 10:56 ` [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC Ionela Voinescu 2021-08-24 10:56 ` [PATCH v2 3/3] arm64, topology: enable use of init_cpu_capacity_cppc() Ionela Voinescu 2 siblings, 1 reply; 9+ messages in thread From: Ionela Voinescu @ 2021-08-24 10:56 UTC (permalink / raw) To: Sudeep Holla, Rafael J . Wysocki, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley Cc: linux-kernel, linux-acpi, linux-arm-kernel init_freq_invariance_cppc() was called in acpi_cppc_processor_probe(), after CPU performance information and controls were populated from the per-cpu _CPC objects. But these _CPC objects provide information that helps with both CPU (u-arch) and frequency invariance. Therefore, change the function name to a more generic one, while adding the arch_ prefix, as this function is expected to be defined differently by different architectures. Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> Tested-by: Valentin Schneider <valentin.schneider@arm.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: Giovanni Gherdovich <ggherdovich@suse.cz> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> --- arch/x86/include/asm/topology.h | 2 +- drivers/acpi/cppc_acpi.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h index 9239399e5491..61d73013cab8 100644 --- a/arch/x86/include/asm/topology.h +++ b/arch/x86/include/asm/topology.h @@ -220,7 +220,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled) #ifdef CONFIG_ACPI_CPPC_LIB void init_freq_invariance_cppc(void); -#define init_freq_invariance_cppc init_freq_invariance_cppc +#define arch_init_invariance_cppc init_freq_invariance_cppc #endif #endif /* _ASM_X86_TOPOLOGY_H */ diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index a4d4eebba1da..c211d77310e8 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -660,8 +660,8 @@ static bool is_cppc_supported(int revision, int num_ent) * } */ -#ifndef init_freq_invariance_cppc -static inline void init_freq_invariance_cppc(void) { } +#ifndef arch_init_invariance_cppc +static inline void arch_init_invariance_cppc(void) { } #endif /** @@ -826,7 +826,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) goto out_free; } - init_freq_invariance_cppc(); + arch_init_invariance_cppc(); kfree(output.pointer); return 0; -- 2.29.2.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc 2021-08-24 10:56 ` [PATCH v2 1/3] x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc Ionela Voinescu @ 2021-08-25 17:48 ` Rafael J. Wysocki 0 siblings, 0 replies; 9+ messages in thread From: Rafael J. Wysocki @ 2021-08-25 17:48 UTC (permalink / raw) To: Ionela Voinescu Cc: Sudeep Holla, Rafael J . Wysocki, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley, Linux Kernel Mailing List, ACPI Devel Maling List, Linux ARM On Tue, Aug 24, 2021 at 12:57 PM Ionela Voinescu <ionela.voinescu@arm.com> wrote: > > init_freq_invariance_cppc() was called in acpi_cppc_processor_probe(), > after CPU performance information and controls were populated from the > per-cpu _CPC objects. > > But these _CPC objects provide information that helps with both CPU > (u-arch) and frequency invariance. Therefore, change the function name > to a more generic one, while adding the arch_ prefix, as this function > is expected to be defined differently by different architectures. > > Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> > Tested-by: Valentin Schneider <valentin.schneider@arm.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Giovanni Gherdovich <ggherdovich@suse.cz> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Acked-by: Rafael J. Wysocki <rafael@kernel.org> > --- > arch/x86/include/asm/topology.h | 2 +- > drivers/acpi/cppc_acpi.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h > index 9239399e5491..61d73013cab8 100644 > --- a/arch/x86/include/asm/topology.h > +++ b/arch/x86/include/asm/topology.h > @@ -220,7 +220,7 @@ static inline void arch_set_max_freq_ratio(bool turbo_disabled) > > #ifdef CONFIG_ACPI_CPPC_LIB > void init_freq_invariance_cppc(void); > -#define init_freq_invariance_cppc init_freq_invariance_cppc > +#define arch_init_invariance_cppc init_freq_invariance_cppc > #endif > > #endif /* _ASM_X86_TOPOLOGY_H */ > diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c > index a4d4eebba1da..c211d77310e8 100644 > --- a/drivers/acpi/cppc_acpi.c > +++ b/drivers/acpi/cppc_acpi.c > @@ -660,8 +660,8 @@ static bool is_cppc_supported(int revision, int num_ent) > * } > */ > > -#ifndef init_freq_invariance_cppc > -static inline void init_freq_invariance_cppc(void) { } > +#ifndef arch_init_invariance_cppc > +static inline void arch_init_invariance_cppc(void) { } > #endif > > /** > @@ -826,7 +826,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) > goto out_free; > } > > - init_freq_invariance_cppc(); > + arch_init_invariance_cppc(); > > kfree(output.pointer); > return 0; > -- > 2.29.2.dirty > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC 2021-08-24 10:56 [PATCH v2 0/3] arch_topology, ACPI: populate cpu capacity from CPPC Ionela Voinescu 2021-08-24 10:56 ` [PATCH v2 1/3] x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc Ionela Voinescu @ 2021-08-24 10:56 ` Ionela Voinescu 2021-08-25 17:54 ` Rafael J. Wysocki 2021-08-24 10:56 ` [PATCH v2 3/3] arm64, topology: enable use of init_cpu_capacity_cppc() Ionela Voinescu 2 siblings, 1 reply; 9+ messages in thread From: Ionela Voinescu @ 2021-08-24 10:56 UTC (permalink / raw) To: Sudeep Holla, Rafael J . Wysocki, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley Cc: linux-kernel, linux-acpi, linux-arm-kernel Define topology_init_cpu_capacity_cppc() to use highest performance values from _CPC objects to obtain and set maximum capacity information for each CPU. acpi_cppc_processor_probe() is a good point at which to trigger the initialization of CPU (u-arch) capacity values, as at this point the highest performance values can be obtained from each CPU's _CPC objects. Architectures can therefore use this functionality through arch_init_invariance_cppc(). The performance scale used by CPPC is a unified scale for all CPUs in the system. Therefore, by obtaining the raw highest performance values from the _CPC objects, and normalizing them on the [0, 1024] capacity scale, used by the task scheduler, we obtain the CPU capacity of each CPU. While an ACPI Notify(0x85) could alert about a change in the highest performance value, which should in turn retrigger the CPU capacity computations, this notification is not currently handled by the ACPI processor driver. When supported, a call to arch_init_invariance_cppc() would perform the update. Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> Tested-by: Valentin Schneider <valentin.schneider@arm.com> Cc: Sudeep Holla <sudeep.holla@arm.com> --- drivers/base/arch_topology.c | 37 +++++++++++++++++++++++++++++++++++ include/linux/arch_topology.h | 4 ++++ 2 files changed, 41 insertions(+) diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c index 921312a8d957..358e22cd629e 100644 --- a/drivers/base/arch_topology.c +++ b/drivers/base/arch_topology.c @@ -306,6 +306,43 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) return !ret; } +#ifdef CONFIG_ACPI_CPPC_LIB +#include <acpi/cppc_acpi.h> + +void topology_init_cpu_capacity_cppc(void) +{ + struct cppc_perf_caps perf_caps; + int cpu; + + if (likely(acpi_disabled || !acpi_cpc_valid())) + return; + + raw_capacity = kcalloc(num_possible_cpus(), sizeof(*raw_capacity), + GFP_KERNEL); + if (!raw_capacity) + return; + + for_each_possible_cpu(cpu) { + if (!cppc_get_perf_caps(cpu, &perf_caps)) { + raw_capacity[cpu] = perf_caps.highest_perf; + pr_debug("cpu_capacity: CPU%d cpu_capacity=%u (raw).\n", + cpu, raw_capacity[cpu]); + } else { + pr_err("cpu_capacity: CPU%d missing highest performance.\n", cpu); + pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n"); + goto exit; + } + } + + topology_normalize_cpu_scale(); + schedule_work(&update_topology_flags_work); + pr_debug("cpu_capacity: cpu_capacity initialization done\n"); + +exit: + free_raw_capacity(); +} +#endif + #ifdef CONFIG_CPU_FREQ static cpumask_var_t cpus_to_visit; static void parsing_done_workfn(struct work_struct *work); diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h index f180240dc95f..9cf1a17938f0 100644 --- a/include/linux/arch_topology.h +++ b/include/linux/arch_topology.h @@ -11,6 +11,10 @@ void topology_normalize_cpu_scale(void); int topology_update_cpu_topology(void); +#ifdef CONFIG_ACPI_CPPC_LIB +void topology_init_cpu_capacity_cppc(void); +#endif + struct device_node; bool topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu); -- 2.29.2.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC 2021-08-24 10:56 ` [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC Ionela Voinescu @ 2021-08-25 17:54 ` Rafael J. Wysocki 2021-08-26 17:51 ` Ionela Voinescu 0 siblings, 1 reply; 9+ messages in thread From: Rafael J. Wysocki @ 2021-08-25 17:54 UTC (permalink / raw) To: Ionela Voinescu Cc: Sudeep Holla, Rafael J . Wysocki, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley, Linux Kernel Mailing List, ACPI Devel Maling List, Linux ARM On Tue, Aug 24, 2021 at 12:57 PM Ionela Voinescu <ionela.voinescu@arm.com> wrote: > > Define topology_init_cpu_capacity_cppc() to use highest performance > values from _CPC objects to obtain and set maximum capacity information > for each CPU. acpi_cppc_processor_probe() is a good point at which to > trigger the initialization of CPU (u-arch) capacity values, as at this > point the highest performance values can be obtained from each CPU's > _CPC objects. Architectures can therefore use this functionality > through arch_init_invariance_cppc(). > > The performance scale used by CPPC is a unified scale for all CPUs in > the system. Therefore, by obtaining the raw highest performance values > from the _CPC objects, and normalizing them on the [0, 1024] capacity > scale, used by the task scheduler, we obtain the CPU capacity of each > CPU. > > While an ACPI Notify(0x85) could alert about a change in the highest > performance value, which should in turn retrigger the CPU capacity > computations, this notification is not currently handled by the ACPI > processor driver. When supported, a call to arch_init_invariance_cppc() > would perform the update. > > Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> > Tested-by: Valentin Schneider <valentin.schneider@arm.com> > Cc: Sudeep Holla <sudeep.holla@arm.com> > --- > drivers/base/arch_topology.c | 37 +++++++++++++++++++++++++++++++++++ > include/linux/arch_topology.h | 4 ++++ > 2 files changed, 41 insertions(+) > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > index 921312a8d957..358e22cd629e 100644 > --- a/drivers/base/arch_topology.c > +++ b/drivers/base/arch_topology.c > @@ -306,6 +306,43 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) > return !ret; > } > > +#ifdef CONFIG_ACPI_CPPC_LIB > +#include <acpi/cppc_acpi.h> > + > +void topology_init_cpu_capacity_cppc(void) > +{ > + struct cppc_perf_caps perf_caps; > + int cpu; > + > + if (likely(acpi_disabled || !acpi_cpc_valid())) > + return; > + > + raw_capacity = kcalloc(num_possible_cpus(), sizeof(*raw_capacity), > + GFP_KERNEL); > + if (!raw_capacity) > + return; > + > + for_each_possible_cpu(cpu) { > + if (!cppc_get_perf_caps(cpu, &perf_caps)) { > + raw_capacity[cpu] = perf_caps.highest_perf; From experience, I would advise doing some sanity checking on the per_caps values before using them here. Also note that highest_perf may not be sustainable, so would using highest_perf as raw_capacity[] always work as expected? > + pr_debug("cpu_capacity: CPU%d cpu_capacity=%u (raw).\n", > + cpu, raw_capacity[cpu]); > + } else { > + pr_err("cpu_capacity: CPU%d missing highest performance.\n", cpu); > + pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n"); > + goto exit; > + } > + } > + > + topology_normalize_cpu_scale(); > + schedule_work(&update_topology_flags_work); > + pr_debug("cpu_capacity: cpu_capacity initialization done\n"); > + > +exit: > + free_raw_capacity(); > +} > +#endif > + > #ifdef CONFIG_CPU_FREQ > static cpumask_var_t cpus_to_visit; > static void parsing_done_workfn(struct work_struct *work); > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > index f180240dc95f..9cf1a17938f0 100644 > --- a/include/linux/arch_topology.h > +++ b/include/linux/arch_topology.h > @@ -11,6 +11,10 @@ > void topology_normalize_cpu_scale(void); > int topology_update_cpu_topology(void); > > +#ifdef CONFIG_ACPI_CPPC_LIB > +void topology_init_cpu_capacity_cppc(void); > +#endif > + > struct device_node; > bool topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu); > > -- > 2.29.2.dirty > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC 2021-08-25 17:54 ` Rafael J. Wysocki @ 2021-08-26 17:51 ` Ionela Voinescu 2021-08-26 18:32 ` Rafael J. Wysocki 0 siblings, 1 reply; 9+ messages in thread From: Ionela Voinescu @ 2021-08-26 17:51 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Sudeep Holla, Rafael J . Wysocki, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley, Linux Kernel Mailing List, ACPI Devel Maling List, Linux ARM Thanks for the review, Rafael! On Wednesday 25 Aug 2021 at 19:54:26 (+0200), Rafael J. Wysocki wrote: > On Tue, Aug 24, 2021 at 12:57 PM Ionela Voinescu > <ionela.voinescu@arm.com> wrote: > > > > Define topology_init_cpu_capacity_cppc() to use highest performance > > values from _CPC objects to obtain and set maximum capacity information > > for each CPU. acpi_cppc_processor_probe() is a good point at which to > > trigger the initialization of CPU (u-arch) capacity values, as at this > > point the highest performance values can be obtained from each CPU's > > _CPC objects. Architectures can therefore use this functionality > > through arch_init_invariance_cppc(). > > > > The performance scale used by CPPC is a unified scale for all CPUs in > > the system. Therefore, by obtaining the raw highest performance values > > from the _CPC objects, and normalizing them on the [0, 1024] capacity > > scale, used by the task scheduler, we obtain the CPU capacity of each > > CPU. > > > > While an ACPI Notify(0x85) could alert about a change in the highest > > performance value, which should in turn retrigger the CPU capacity > > computations, this notification is not currently handled by the ACPI > > processor driver. When supported, a call to arch_init_invariance_cppc() > > would perform the update. > > > > Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> > > Tested-by: Valentin Schneider <valentin.schneider@arm.com> > > Cc: Sudeep Holla <sudeep.holla@arm.com> > > --- > > drivers/base/arch_topology.c | 37 +++++++++++++++++++++++++++++++++++ > > include/linux/arch_topology.h | 4 ++++ > > 2 files changed, 41 insertions(+) > > > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > > index 921312a8d957..358e22cd629e 100644 > > --- a/drivers/base/arch_topology.c > > +++ b/drivers/base/arch_topology.c > > @@ -306,6 +306,43 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) > > return !ret; > > } > > > > +#ifdef CONFIG_ACPI_CPPC_LIB > > +#include <acpi/cppc_acpi.h> > > + > > +void topology_init_cpu_capacity_cppc(void) > > +{ > > + struct cppc_perf_caps perf_caps; > > + int cpu; > > + > > + if (likely(acpi_disabled || !acpi_cpc_valid())) > > + return; > > + > > + raw_capacity = kcalloc(num_possible_cpus(), sizeof(*raw_capacity), > > + GFP_KERNEL); > > + if (!raw_capacity) > > + return; > > + > > + for_each_possible_cpu(cpu) { > > + if (!cppc_get_perf_caps(cpu, &perf_caps)) { > > + raw_capacity[cpu] = perf_caps.highest_perf; > > From experience, I would advise doing some sanity checking on the > per_caps values before using them here. > cppc_get_perf_caps() already returns -EFAULT if highest_perf is 0, and I'm not sure if I can make any other assumptions about what a sane highest_perf value would need to be here. Did you have anything else in mind for sanity checking? > Also note that highest_perf may not be sustainable, so would using > highest_perf as raw_capacity[] always work as expected? > Yes, in my opinion using it is better than the alternative, using the nominal performance value. This highest performance value helps obtain the maximum capacity of a CPU on a scale [0, 1024] when referenced to the highest performance of the biggest CPU in the system. There is no assumption in the task scheduler that this capacity is sustainable. Using lower values (nominal performance) would shorten the scale and make smaller CPUs seem bigger than they are. Also, using highest performance gives a better indication of micro-architectural differences in performance between CPUs, which plays a role in scaling utilization, even if some of the performance levels are not sustainable (which is platform dependent). Thanks, Ionela. > > + pr_debug("cpu_capacity: CPU%d cpu_capacity=%u (raw).\n", > > + cpu, raw_capacity[cpu]); > > + } else { > > + pr_err("cpu_capacity: CPU%d missing highest performance.\n", cpu); > > + pr_err("cpu_capacity: partial information: fallback to 1024 for all CPUs\n"); > > + goto exit; > > + } > > + } > > + > > + topology_normalize_cpu_scale(); > > + schedule_work(&update_topology_flags_work); > > + pr_debug("cpu_capacity: cpu_capacity initialization done\n"); > > + > > +exit: > > + free_raw_capacity(); > > +} > > +#endif > > + > > #ifdef CONFIG_CPU_FREQ > > static cpumask_var_t cpus_to_visit; > > static void parsing_done_workfn(struct work_struct *work); > > diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h > > index f180240dc95f..9cf1a17938f0 100644 > > --- a/include/linux/arch_topology.h > > +++ b/include/linux/arch_topology.h > > @@ -11,6 +11,10 @@ > > void topology_normalize_cpu_scale(void); > > int topology_update_cpu_topology(void); > > > > +#ifdef CONFIG_ACPI_CPPC_LIB > > +void topology_init_cpu_capacity_cppc(void); > > +#endif > > + > > struct device_node; > > bool topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu); > > > > -- > > 2.29.2.dirty > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC 2021-08-26 17:51 ` Ionela Voinescu @ 2021-08-26 18:32 ` Rafael J. Wysocki 2022-03-01 16:46 ` Ionela Voinescu 0 siblings, 1 reply; 9+ messages in thread From: Rafael J. Wysocki @ 2021-08-26 18:32 UTC (permalink / raw) To: Ionela Voinescu Cc: Rafael J. Wysocki, Sudeep Holla, Rafael J . Wysocki, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley, Linux Kernel Mailing List, ACPI Devel Maling List, Linux ARM On Thu, Aug 26, 2021 at 7:51 PM Ionela Voinescu <ionela.voinescu@arm.com> wrote: > > Thanks for the review, Rafael! > > On Wednesday 25 Aug 2021 at 19:54:26 (+0200), Rafael J. Wysocki wrote: > > On Tue, Aug 24, 2021 at 12:57 PM Ionela Voinescu > > <ionela.voinescu@arm.com> wrote: > > > > > > Define topology_init_cpu_capacity_cppc() to use highest performance > > > values from _CPC objects to obtain and set maximum capacity information > > > for each CPU. acpi_cppc_processor_probe() is a good point at which to > > > trigger the initialization of CPU (u-arch) capacity values, as at this > > > point the highest performance values can be obtained from each CPU's > > > _CPC objects. Architectures can therefore use this functionality > > > through arch_init_invariance_cppc(). > > > > > > The performance scale used by CPPC is a unified scale for all CPUs in > > > the system. Therefore, by obtaining the raw highest performance values > > > from the _CPC objects, and normalizing them on the [0, 1024] capacity > > > scale, used by the task scheduler, we obtain the CPU capacity of each > > > CPU. > > > > > > While an ACPI Notify(0x85) could alert about a change in the highest > > > performance value, which should in turn retrigger the CPU capacity > > > computations, this notification is not currently handled by the ACPI > > > processor driver. When supported, a call to arch_init_invariance_cppc() > > > would perform the update. > > > > > > Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> > > > Tested-by: Valentin Schneider <valentin.schneider@arm.com> > > > Cc: Sudeep Holla <sudeep.holla@arm.com> > > > --- > > > drivers/base/arch_topology.c | 37 +++++++++++++++++++++++++++++++++++ > > > include/linux/arch_topology.h | 4 ++++ > > > 2 files changed, 41 insertions(+) > > > > > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > > > index 921312a8d957..358e22cd629e 100644 > > > --- a/drivers/base/arch_topology.c > > > +++ b/drivers/base/arch_topology.c > > > @@ -306,6 +306,43 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) > > > return !ret; > > > } > > > > > > +#ifdef CONFIG_ACPI_CPPC_LIB > > > +#include <acpi/cppc_acpi.h> > > > + > > > +void topology_init_cpu_capacity_cppc(void) > > > +{ > > > + struct cppc_perf_caps perf_caps; > > > + int cpu; > > > + > > > + if (likely(acpi_disabled || !acpi_cpc_valid())) > > > + return; > > > + > > > + raw_capacity = kcalloc(num_possible_cpus(), sizeof(*raw_capacity), > > > + GFP_KERNEL); > > > + if (!raw_capacity) > > > + return; > > > + > > > + for_each_possible_cpu(cpu) { > > > + if (!cppc_get_perf_caps(cpu, &perf_caps)) { > > > + raw_capacity[cpu] = perf_caps.highest_perf; > > > > From experience, I would advise doing some sanity checking on the > > per_caps values before using them here. > > > > cppc_get_perf_caps() already returns -EFAULT if highest_perf is 0, and > I'm not sure if I can make any other assumptions about what a sane > highest_perf value would need to be here. Well, it cannot be less than lowest_perf or nominal_perf or guaranteed_perf, for instance. > Did you have anything else in mind for sanity checking? > > > Also note that highest_perf may not be sustainable, so would using > > highest_perf as raw_capacity[] always work as expected? > > > > Yes, in my opinion using it is better than the alternative, using the > nominal performance value. This highest performance value helps obtain > the maximum capacity of a CPU on a scale [0, 1024] when referenced to > the highest performance of the biggest CPU in the system. There is no > assumption in the task scheduler that this capacity is sustainable. That's true, but there are consequences if it is the case. Namely, you may find that the big CPUs run at the highest performance for a small fraction of time, so most of the time they may appear to be underutilized no matter how many tasks are packed on them, which then will influence the utilization metrics of those tasks. It may be better to use guaranteed_perf or some value between it at the highest for this reason. > Using lower values (nominal performance) would shorten the scale and > make smaller CPUs seem bigger than they are. Also, using highest > performance gives a better indication of micro-architectural > differences in performance between CPUs, which plays a role in scaling > utilization, even if some of the performance levels are not sustainable > (which is platform dependent). ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC 2021-08-26 18:32 ` Rafael J. Wysocki @ 2022-03-01 16:46 ` Ionela Voinescu 0 siblings, 0 replies; 9+ messages in thread From: Ionela Voinescu @ 2022-03-01 16:46 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Sudeep Holla, Rafael J . Wysocki, Pierre Gondois, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley, Linux Kernel Mailing List, ACPI Devel Maling List, Linux ARM Hi Rafael, Apologies for the delay. On Thursday 26 Aug 2021 at 20:32:52 (+0200), Rafael J. Wysocki wrote: > On Thu, Aug 26, 2021 at 7:51 PM Ionela Voinescu <ionela.voinescu@arm.com> wrote: > > > > Thanks for the review, Rafael! > > > > On Wednesday 25 Aug 2021 at 19:54:26 (+0200), Rafael J. Wysocki wrote: > > > On Tue, Aug 24, 2021 at 12:57 PM Ionela Voinescu > > > <ionela.voinescu@arm.com> wrote: > > > > > > > > Define topology_init_cpu_capacity_cppc() to use highest performance > > > > values from _CPC objects to obtain and set maximum capacity information > > > > for each CPU. acpi_cppc_processor_probe() is a good point at which to > > > > trigger the initialization of CPU (u-arch) capacity values, as at this > > > > point the highest performance values can be obtained from each CPU's > > > > _CPC objects. Architectures can therefore use this functionality > > > > through arch_init_invariance_cppc(). > > > > > > > > The performance scale used by CPPC is a unified scale for all CPUs in > > > > the system. Therefore, by obtaining the raw highest performance values > > > > from the _CPC objects, and normalizing them on the [0, 1024] capacity > > > > scale, used by the task scheduler, we obtain the CPU capacity of each > > > > CPU. > > > > > > > > While an ACPI Notify(0x85) could alert about a change in the highest > > > > performance value, which should in turn retrigger the CPU capacity > > > > computations, this notification is not currently handled by the ACPI > > > > processor driver. When supported, a call to arch_init_invariance_cppc() > > > > would perform the update. > > > > > > > > Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> > > > > Tested-by: Valentin Schneider <valentin.schneider@arm.com> > > > > Cc: Sudeep Holla <sudeep.holla@arm.com> > > > > --- > > > > drivers/base/arch_topology.c | 37 +++++++++++++++++++++++++++++++++++ > > > > include/linux/arch_topology.h | 4 ++++ > > > > 2 files changed, 41 insertions(+) > > > > > > > > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c > > > > index 921312a8d957..358e22cd629e 100644 > > > > --- a/drivers/base/arch_topology.c > > > > +++ b/drivers/base/arch_topology.c > > > > @@ -306,6 +306,43 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu) > > > > return !ret; > > > > } > > > > > > > > +#ifdef CONFIG_ACPI_CPPC_LIB > > > > +#include <acpi/cppc_acpi.h> > > > > + > > > > +void topology_init_cpu_capacity_cppc(void) > > > > +{ > > > > + struct cppc_perf_caps perf_caps; > > > > + int cpu; > > > > + > > > > + if (likely(acpi_disabled || !acpi_cpc_valid())) > > > > + return; > > > > + > > > > + raw_capacity = kcalloc(num_possible_cpus(), sizeof(*raw_capacity), > > > > + GFP_KERNEL); > > > > + if (!raw_capacity) > > > > + return; > > > > + > > > > + for_each_possible_cpu(cpu) { > > > > + if (!cppc_get_perf_caps(cpu, &perf_caps)) { > > > > + raw_capacity[cpu] = perf_caps.highest_perf; > > > > > > From experience, I would advise doing some sanity checking on the > > > per_caps values before using them here. > > > > > > > cppc_get_perf_caps() already returns -EFAULT if highest_perf is 0, and > > I'm not sure if I can make any other assumptions about what a sane > > highest_perf value would need to be here. > > Well, it cannot be less than lowest_perf or nominal_perf or > guaranteed_perf, for instance. > True! I'll push a v3 with this change tomorrow, after I rebase and re-test on 5.17-rc6. I've not checked for guaranteed performance as according to the specification it's optional and it's designed to be between lowest_perf and nominal_perf. If guaranteed performance is/will be used anywhere, it should have its own validation against lower/nominal/highest. Therefore I think checking that we have a highest_perf value higher or equal to both nominal_perf and lowest_perf is the right balance in validation. > > Did you have anything else in mind for sanity checking? > > > > > Also note that highest_perf may not be sustainable, so would using > > > highest_perf as raw_capacity[] always work as expected? > > > > > > > Yes, in my opinion using it is better than the alternative, using the > > nominal performance value. This highest performance value helps obtain > > the maximum capacity of a CPU on a scale [0, 1024] when referenced to > > the highest performance of the biggest CPU in the system. There is no > > assumption in the task scheduler that this capacity is sustainable. > > That's true, but there are consequences if it is the case. Namely, > you may find that the big CPUs run at the highest performance for a > small fraction of time, so most of the time they may appear to be > underutilized no matter how many tasks are packed on them, which then > will influence the utilization metrics of those tasks. > Yes, there are no perfect solutions, and whether one exposes nominal or highest performance there will be consequences. But I think the task scheduler has progressed to cope better with unavailable capacity. For example, after the PELT changes at [1], CPUs won't seem underutilized if they are always running, even if they are not running at the highest frequency. Before these changes to PELT, the utilization of a CPU was capped to the current capacity of the CPU. So in that case, if the nominal_perf was 70% of highest_perf and the CPU was only running at nominal_freq, its utilization could not exceed 716 (big CPU). But after [1] if the CPU had no idle time, it does exceed 716 and could be marked as overutilized. That utilization doesn't really have a meaning (we don't know how utilized the CPU would have been if it would have run at boosted frequencies) but it does not have to, as long as it signals the need of an action: migrate tasks to another CPU, possibly disable EAS. [1] https://lore.kernel.org/lkml/tip-23127296889fe84b0762b191b5d041e8ba6f2599@git.kernel.org/ > It may be better to use guaranteed_perf or some value between it at > the highest for this reason. > Guaranteed performance is a tricky one: " Guaranteed Performance Register conveys to OSPM a Guaranteed Performance level, which is the current maximum sustained performance level of a processor, taking into account all known external constraints (power budgeting, thermal constraints, AC vs DC power source, etc.). All processors are expected to be able to sustain their guaranteed performance levels simultaneously. The guaranteed performance level is required to fall in the range [Lowest Performance, Nominal performance], inclusive. " In my experience everyone interprets this differently and therefore platform providers could populate this vastly differently. One could argue that the only true guaranteed performance is the lowest performance as it's the only one potentially sustainable under any power and especially thermal conditions. As well one could argue that it could be closer or equal to nominal performance, but that is the case only if one considers thermal effects unlikely. Therefore it may be that it's highly unreliable as a generic source of capacity information. Therefore, I'm still leaning towards keeping the use of highest_perf in this case due to the scheduler's ability to cope with unavailable capacity but also because it reflects better u-arch differences between CPUs. Thank you, Ionela. > > Using lower values (nominal performance) would shorten the scale and > > make smaller CPUs seem bigger than they are. Also, using highest > > performance gives a better indication of micro-architectural > > differences in performance between CPUs, which plays a role in scaling > > utilization, even if some of the performance levels are not sustainable > > (which is platform dependent). ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] arm64, topology: enable use of init_cpu_capacity_cppc() 2021-08-24 10:56 [PATCH v2 0/3] arch_topology, ACPI: populate cpu capacity from CPPC Ionela Voinescu 2021-08-24 10:56 ` [PATCH v2 1/3] x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc Ionela Voinescu 2021-08-24 10:56 ` [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC Ionela Voinescu @ 2021-08-24 10:56 ` Ionela Voinescu 2 siblings, 0 replies; 9+ messages in thread From: Ionela Voinescu @ 2021-08-24 10:56 UTC (permalink / raw) To: Sudeep Holla, Rafael J . Wysocki, Thomas Gleixner, Ingo Molnar, Giovanni Gherdovich, Catalin Marinas, Will Deacon, Valentin Schneider, Dietmar Eggemann, Sean Kelley Cc: linux-kernel, linux-acpi, linux-arm-kernel Now that the arch topology driver provides a method of setting CPU capacity values based on information on highest performance from CPPC, use this functionality on arm64 platforms. Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com> Acked-by: Catalin Marinas <catalin.marinas@arm.com> Tested-by: Valentin Schneider <valentin.schneider@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> --- arch/arm64/include/asm/topology.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm64/include/asm/topology.h b/arch/arm64/include/asm/topology.h index ec2db3419c41..5ebe51ef6d6c 100644 --- a/arch/arm64/include/asm/topology.h +++ b/arch/arm64/include/asm/topology.h @@ -24,6 +24,10 @@ void update_freq_counters_refs(void); #define arch_scale_freq_capacity topology_get_freq_scale #define arch_scale_freq_invariant topology_scale_freq_invariant +#ifdef CONFIG_ACPI_CPPC_LIB +#define arch_init_invariance_cppc topology_init_cpu_capacity_cppc +#endif + /* Replace task scheduler's default cpu-invariant accounting */ #define arch_scale_cpu_capacity topology_get_cpu_scale -- 2.29.2.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-01 16:49 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-24 10:56 [PATCH v2 0/3] arch_topology, ACPI: populate cpu capacity from CPPC Ionela Voinescu 2021-08-24 10:56 ` [PATCH v2 1/3] x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc Ionela Voinescu 2021-08-25 17:48 ` Rafael J. Wysocki 2021-08-24 10:56 ` [PATCH v2 2/3] arch_topology: obtain cpu capacity using information from CPPC Ionela Voinescu 2021-08-25 17:54 ` Rafael J. Wysocki 2021-08-26 17:51 ` Ionela Voinescu 2021-08-26 18:32 ` Rafael J. Wysocki 2022-03-01 16:46 ` Ionela Voinescu 2021-08-24 10:56 ` [PATCH v2 3/3] arm64, topology: enable use of init_cpu_capacity_cppc() Ionela Voinescu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).