2021-05-14 10:41:58

by Ionela Voinescu

[permalink] [raw]
Subject: [PATCH 0/3] arch_topology, ACPI: populate cpu capacity from CPPC

Hi all,

These are a few trivial patches to populate cpu capacity information
using performance information from ACPI's CPPC.

I've tied this functionality to the existing function
init_freq_invariance_cppc() called in acpi_cppc_processor_probe().
This function is renamed to a more generic arch_init_invariance_cppc().

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@ubuntu:~# dmesg | grep cpu_capacity
[ 2.157494] init_cpu_capacity_cppc: CPU0 cpu_capacity=38300 (raw).
[ 2.163699] init_cpu_capacity_cppc: CPU1 cpu_capacity=38300 (raw).
[ 2.169899] init_cpu_capacity_cppc: CPU2 cpu_capacity=38300 (raw).
[ 2.176098] init_cpu_capacity_cppc: CPU3 cpu_capacity=38300 (raw).
[ 2.182296] init_cpu_capacity_cppc: CPU4 cpu_capacity=102400 (raw).
[ 2.188581] init_cpu_capacity_cppc: CPU5 cpu_capacity=102400 (raw).
[ 2.194867] cpu_capacity: capacity_scale=102400
[ 2.199409] cpu_capacity: CPU0 cpu_capacity=383
[ 2.203952] cpu_capacity: CPU1 cpu_capacity=383
[ 2.208495] cpu_capacity: CPU2 cpu_capacity=383
[ 2.213037] cpu_capacity: CPU3 cpu_capacity=383
[ 2.217580] cpu_capacity: CPU4 cpu_capacity=1024
[ 2.222209] cpu_capacity: CPU5 cpu_capacity=1024
[ 2.226886] init_cpu_capacity_cppc: cpu_capacity initialization done

root@ubuntu:~# 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

All works as expected even if ACPI processor support is built as a
module.

Patches are based on v5.13-rc1.

Let me know what you think!

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 | 39 +++++++++++++++++++++++++++++++
include/linux/arch_topology.h | 4 ++++
5 files changed, 51 insertions(+), 4 deletions(-)

--
2.29.2.dirty



2021-05-14 10:42:05

by Ionela Voinescu

[permalink] [raw]
Subject: [PATCH 2/3] arch_topology: obtain cpu capacity using information from CPPC

Define 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 <[email protected]>
Cc: Sudeep Holla <[email protected]>
---
drivers/base/arch_topology.c | 39 +++++++++++++++++++++++++++++++++++
include/linux/arch_topology.h | 4 ++++
2 files changed, 43 insertions(+)

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index c1179edc0f3b..f710d64f125b 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -291,6 +291,45 @@ 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 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("%s: CPU%d cpu_capacity=%u (raw).\n",
+ __func__, cpu, raw_capacity[cpu]);
+ } else {
+ pr_err("%s: CPU%d missing highest performance.\n",
+ __func__, cpu);
+ pr_err("%s: fallback to 1024 for all CPUs\n",
+ __func__);
+ goto exit;
+ }
+ }
+
+ topology_normalize_cpu_scale();
+ schedule_work(&update_topology_flags_work);
+ pr_debug("%s: cpu_capacity initialization done\n", __func__);
+
+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..fbd829c3b7f7 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 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


2021-05-14 15:40:23

by Ionela Voinescu

[permalink] [raw]
Subject: [PATCH 1/3] x86, ACPI: rename init_freq_invariance_cppc to arch_init_invariance_cppc

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 <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Giovanni Gherdovich <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
---
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


2021-05-15 01:45:12

by Dietmar Eggemann

[permalink] [raw]
Subject: Re: [PATCH 2/3] arch_topology: obtain cpu capacity using information from CPPC

On 14/05/2021 11:53, Ionela Voinescu wrote:

[...]

> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index c1179edc0f3b..f710d64f125b 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -291,6 +291,45 @@ 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>

init_cpu_capacity_cppc() shares a lot of functionality with the existing
DT/CPUfreq-based approach (topology_parse_cpu_capacity(),
register_cpufreq_notifier(), init_cpu_capacity_callback()). It looks
like that the different ways of invocation (two steps per cpu vs. one
step for all cpus) makes it hard to restructure the code to create more
common bits.

> +void init_cpu_capacity_cppc(void)
> +{
> + struct cppc_perf_caps perf_caps;
> + int cpu;
> +
> + if (likely(acpi_disabled || !acpi_cpc_valid()))

likely(acpi_disabled) ?

> + 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("%s: CPU%d cpu_capacity=%u (raw).\n",
> + __func__, cpu, raw_capacity[cpu]);

There is quite a variety in the layout of the pr_xxx() log messages in
this file. Originally the 'cpu_capacity:' was used to indicate that this
log is from drivers/base/arch_topology.c. Now the GCC __func__
identifier is used. Maybe this can be aligned better? Especially since
the functionality used in the existing DT-driven and now in the new
CPPC-driven functionality is quite similar. Debugging is so much easier
with consistent log strings.


> + } else {
> + pr_err("%s: CPU%d missing highest performance.\n",
> + __func__, cpu);
> + pr_err("%s: fallback to 1024 for all CPUs\n",
> + __func__);
> + goto exit;
> + }
> + }
> +
> + topology_normalize_cpu_scale();
> + schedule_work(&update_topology_flags_work);
> + pr_debug("%s: cpu_capacity initialization done\n", __func__);
> +
> +exit:
> + free_raw_capacity();
> +}
> +#endif

In case a system has CONFIG_ACPI_CPPC_LIB what does this mean for the
DT-based approach via `register_cpufreq_notifier()`?

Looks like we rely on:

376 static int __init register_cpufreq_notifier(void)
...
385 if (!acpi_disabled || ...)
386 return -EINVAL;

to disable the CPUfreq part of the DT/CPUfreq-based approach on an ACPI
system.

[...]

2021-05-19 18:09:44

by Valentin Schneider

[permalink] [raw]
Subject: Re: [PATCH 0/3] arch_topology, ACPI: populate cpu capacity from CPPC

Hi,

On 14/05/21 10:53, Ionela Voinescu wrote:
> Hi all,
>
> These are a few trivial patches to populate cpu capacity information
> using performance information from ACPI's CPPC.
>
> I've tied this functionality to the existing function
> init_freq_invariance_cppc() called in acpi_cppc_processor_probe().
> This function is renamed to a more generic arch_init_invariance_cppc().
>
> 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@ubuntu:~# dmesg | grep cpu_capacity
> [ 2.157494] init_cpu_capacity_cppc: CPU0 cpu_capacity=38300 (raw).
> [ 2.163699] init_cpu_capacity_cppc: CPU1 cpu_capacity=38300 (raw).
> [ 2.169899] init_cpu_capacity_cppc: CPU2 cpu_capacity=38300 (raw).
> [ 2.176098] init_cpu_capacity_cppc: CPU3 cpu_capacity=38300 (raw).
> [ 2.182296] init_cpu_capacity_cppc: CPU4 cpu_capacity=102400 (raw).
> [ 2.188581] init_cpu_capacity_cppc: CPU5 cpu_capacity=102400 (raw).
> [ 2.194867] cpu_capacity: capacity_scale=102400
> [ 2.199409] cpu_capacity: CPU0 cpu_capacity=383
> [ 2.203952] cpu_capacity: CPU1 cpu_capacity=383
> [ 2.208495] cpu_capacity: CPU2 cpu_capacity=383
> [ 2.213037] cpu_capacity: CPU3 cpu_capacity=383
> [ 2.217580] cpu_capacity: CPU4 cpu_capacity=1024
> [ 2.222209] cpu_capacity: CPU5 cpu_capacity=1024
> [ 2.226886] init_cpu_capacity_cppc: cpu_capacity initialization done
>
> root@ubuntu:~# 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
>
> All works as expected even if ACPI processor support is built as a
> module.
>

Tested on my Ampere eMAG; this all seems to work fine except for some
scheduler debug stuff that gets confused; see

http://lore.kernel.org/r/[email protected]

With that in mind:
Tested-by: Valentin Schneider <[email protected]>

2021-05-19 18:09:48

by Valentin Schneider

[permalink] [raw]
Subject: Re: [PATCH 2/3] arch_topology: obtain cpu capacity using information from CPPC

On 14/05/21 10:53, Ionela Voinescu wrote:
> +#ifdef CONFIG_ACPI_CPPC_LIB
> +#include <acpi/cppc_acpi.h>
> +
> +void 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);

Per the below loop, the memory shouldn't need to be cleared.

> + 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("%s: CPU%d cpu_capacity=%u (raw).\n",
> + __func__, cpu, raw_capacity[cpu]);
> + } else {
> + pr_err("%s: CPU%d missing highest performance.\n",
> + __func__, cpu);
> + pr_err("%s: fallback to 1024 for all CPUs\n",
> + __func__);
> + goto exit;
> + }
> + }
> +
> + topology_normalize_cpu_scale();
> + schedule_work(&update_topology_flags_work);
> + pr_debug("%s: cpu_capacity initialization done\n", __func__);
> +
> +exit:
> + free_raw_capacity();
> +}
> +#endif

2021-05-19 19:26:34

by Ionela Voinescu

[permalink] [raw]
Subject: Re: [PATCH 0/3] arch_topology, ACPI: populate cpu capacity from CPPC

Hi Valentin,

On Tuesday 18 May 2021 at 14:12:03 (+0100), Valentin Schneider wrote:
> Hi,
>
> On 14/05/21 10:53, Ionela Voinescu wrote:
> > Hi all,
> >
> > These are a few trivial patches to populate cpu capacity information
> > using performance information from ACPI's CPPC.
> >
> > I've tied this functionality to the existing function
> > init_freq_invariance_cppc() called in acpi_cppc_processor_probe().
> > This function is renamed to a more generic arch_init_invariance_cppc().
> >
> > 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@ubuntu:~# dmesg | grep cpu_capacity
> > [ 2.157494] init_cpu_capacity_cppc: CPU0 cpu_capacity=38300 (raw).
> > [ 2.163699] init_cpu_capacity_cppc: CPU1 cpu_capacity=38300 (raw).
> > [ 2.169899] init_cpu_capacity_cppc: CPU2 cpu_capacity=38300 (raw).
> > [ 2.176098] init_cpu_capacity_cppc: CPU3 cpu_capacity=38300 (raw).
> > [ 2.182296] init_cpu_capacity_cppc: CPU4 cpu_capacity=102400 (raw).
> > [ 2.188581] init_cpu_capacity_cppc: CPU5 cpu_capacity=102400 (raw).
> > [ 2.194867] cpu_capacity: capacity_scale=102400
> > [ 2.199409] cpu_capacity: CPU0 cpu_capacity=383
> > [ 2.203952] cpu_capacity: CPU1 cpu_capacity=383
> > [ 2.208495] cpu_capacity: CPU2 cpu_capacity=383
> > [ 2.213037] cpu_capacity: CPU3 cpu_capacity=383
> > [ 2.217580] cpu_capacity: CPU4 cpu_capacity=1024
> > [ 2.222209] cpu_capacity: CPU5 cpu_capacity=1024
> > [ 2.226886] init_cpu_capacity_cppc: cpu_capacity initialization done
> >
> > root@ubuntu:~# 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
> >
> > All works as expected even if ACPI processor support is built as a
> > module.
> >
>
> Tested on my Ampere eMAG; this all seems to work fine except for some
> scheduler debug stuff that gets confused; see
>
> http://lore.kernel.org/r/[email protected]
>
> With that in mind:
> Tested-by: Valentin Schneider <[email protected]>

Many thanks for testing and fixing the debugfs problem. I'll take a look
over your patch.

Ionela.

2021-05-19 19:29:57

by Ionela Voinescu

[permalink] [raw]
Subject: Re: [PATCH 2/3] arch_topology: obtain cpu capacity using information from CPPC

Hi Dietmar,

Many thanks for the review!

On Friday 14 May 2021 at 18:16:50 (+0200), Dietmar Eggemann wrote:
> On 14/05/2021 11:53, Ionela Voinescu wrote:
>
> [...]
>
> > diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> > index c1179edc0f3b..f710d64f125b 100644
> > --- a/drivers/base/arch_topology.c
> > +++ b/drivers/base/arch_topology.c
> > @@ -291,6 +291,45 @@ 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>
>
> init_cpu_capacity_cppc() shares a lot of functionality with the existing
> DT/CPUfreq-based approach (topology_parse_cpu_capacity(),
> register_cpufreq_notifier(), init_cpu_capacity_callback()). It looks
> like that the different ways of invocation (two steps per cpu vs. one
> step for all cpus) makes it hard to restructure the code to create more
> common bits.
>

Yes, I looked at ways to reuse more of the DT-based functionality, but I
did not find a better way. We reuse the normalization functionality and
the rebuild of the scheduling domains, but I'm not sure there's room for
much more, as the rest is specific to each source of capacity
information, DT+cpufreq or CPPC.

I also did not want to tie the new CPPC based functionality to cpufreq.
While the DT-based path needs cpufreq policies initialized as it needs
information on maximum frequency to obtain capacity, this is not needed
in the new code. This results in simpler code and ensures support even
for systems that do not have a cpufreq driver.

> > +void init_cpu_capacity_cppc(void)
> > +{
> > + struct cppc_perf_caps perf_caps;
> > + int cpu;
> > +
> > + if (likely(acpi_disabled || !acpi_cpc_valid()))
>
> likely(acpi_disabled) ?
>

likely (acpi_disabled || !acpi_cpc_valid()) :)

It's "likely" useless, but this function gets called for each CPU from
acpi_cppc_processor_probe(), but it only continues with setting the
cpu_scale after all possible CPUs have their _CPC objects populated.

Therefore it's a lot more likely we return here.

> > + 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("%s: CPU%d cpu_capacity=%u (raw).\n",
> > + __func__, cpu, raw_capacity[cpu]);
>
> There is quite a variety in the layout of the pr_xxx() log messages in
> this file. Originally the 'cpu_capacity:' was used to indicate that this
> log is from drivers/base/arch_topology.c. Now the GCC __func__
> identifier is used. Maybe this can be aligned better? Especially since
> the functionality used in the existing DT-driven and now in the new
> CPPC-driven functionality is quite similar. Debugging is so much easier
> with consistent log strings.
>

Right! My intention was to keep the prints relatively similar, but my
wanting to reduce the line length got the better of me. I'll keep the
prints consistent.

>
> > + } else {
> > + pr_err("%s: CPU%d missing highest performance.\n",
> > + __func__, cpu);
> > + pr_err("%s: fallback to 1024 for all CPUs\n",
> > + __func__);
> > + goto exit;
> > + }
> > + }
> > +
> > + topology_normalize_cpu_scale();
> > + schedule_work(&update_topology_flags_work);
> > + pr_debug("%s: cpu_capacity initialization done\n", __func__);
> > +
> > +exit:
> > + free_raw_capacity();
> > +}
> > +#endif
>
> In case a system has CONFIG_ACPI_CPPC_LIB what does this mean for the
> DT-based approach via `register_cpufreq_notifier()`?
>

CONFIG_ACPI_CPPC_LIB is enabled by default on arm64. This only ensures
that we have the functionality to parse and work with the ACPI _CPC
objects and it does not guarantee that ACPI will be used.

> Looks like we rely on:
>
> 376 static int __init register_cpufreq_notifier(void)
> ...
> 385 if (!acpi_disabled || ...)
> 386 return -EINVAL;
>
> to disable the CPUfreq part of the DT/CPUfreq-based approach on an ACPI
> system.
>

It's both acpi_disabled and raw_capacity that guard the DT path. You
need both to not use ACPI (therefore using DT) and to have valid
capacity-dmips-mhz in DT for the cpufreq notifier that will eventually
populate the cpu_scale variables to be registered.

Thank you,
Ionela.
> [...]

2021-06-28 13:59:25

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: sched/core] sched/debug: Don't update sched_domain debug directories before sched_debug_init()

The following commit has been merged into the sched/core branch of tip:

Commit-ID: 459b09b5a3254008b63382bf41a9b36d0b590f57
Gitweb: https://git.kernel.org/tip/459b09b5a3254008b63382bf41a9b36d0b590f57
Author: Valentin Schneider <[email protected]>
AuthorDate: Tue, 18 May 2021 14:07:25 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Mon, 28 Jun 2021 15:42:24 +02:00

sched/debug: Don't update sched_domain debug directories before sched_debug_init()

Since CPU capacity asymmetry can stem purely from maximum frequency
differences (e.g. Pixel 1), a rebuild of the scheduler topology can be
issued upon loading cpufreq, see:

arch_topology.c::init_cpu_capacity_callback()

Turns out that if this rebuild happens *before* sched_debug_init() is
run (which is a late initcall), we end up messing up the sched_domain debug
directory: passing a NULL parent to debugfs_create_dir() ends up creating
the directory at the debugfs root, which in this case creates
/sys/kernel/debug/domains (instead of /sys/kernel/debug/sched/domains).

This currently doesn't happen on asymmetric systems which use cpufreq-scpi
or cpufreq-dt drivers, as those are loaded via
deferred_probe_initcall() (it is also a late initcall, but appears to be
ordered *after* sched_debug_init()).

Ionela has been working on detecting maximum frequency asymmetry via ACPI,
and that actually happens via a *device* initcall, thus before
sched_debug_init(), and causes the aforementionned debugfs mayhem.

One option would be to punt sched_debug_init() down to
fs_initcall_sync(). Preventing update_sched_domain_debugfs() from running
before sched_debug_init() appears to be the safer option.

Fixes: 3b87f136f8fc ("sched,debug: Convert sysctl sched_domains to debugfs")
Signed-off-by: Valentin Schneider <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: http://lore.kernel.org/r/[email protected]
---
kernel/sched/debug.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c
index 0c5ec27..7e08e3d 100644
--- a/kernel/sched/debug.c
+++ b/kernel/sched/debug.c
@@ -388,6 +388,13 @@ void update_sched_domain_debugfs(void)
{
int cpu, i;

+ /*
+ * This can unfortunately be invoked before sched_debug_init() creates
+ * the debug directory. Don't touch sd_sysctl_cpus until then.
+ */
+ if (!debugfs_sched)
+ return;
+
if (!cpumask_available(sd_sysctl_cpus)) {
if (!alloc_cpumask_var(&sd_sysctl_cpus, GFP_KERNEL))
return;