2022-04-16 02:25:33

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 03/10] x86/aperfmperf: Separate AP/BP frequency invariance init

This code is convoluted and because it can be invoked post init via the
ACPI/CPPC code, all of the initialization functionality is built in instead
of being part of init text and init data.

As a first step create separate calls for the boot and the application
processors.

Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/include/asm/topology.h | 12 +++++-------
arch/x86/kernel/acpi/cppc.c | 3 ++-
arch/x86/kernel/cpu/aperfmperf.c | 23 +++++++++++------------
arch/x86/kernel/smpboot.c | 4 ++--
4 files changed, 20 insertions(+), 22 deletions(-)

--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -216,14 +216,12 @@ extern void arch_scale_freq_tick(void);
#define arch_scale_freq_tick arch_scale_freq_tick

extern void arch_set_max_freq_ratio(bool turbo_disabled);
-void init_freq_invariance(bool secondary, bool cppc_ready);
+extern void bp_init_freq_invariance(bool cppc_ready);
+extern void ap_init_freq_invariance(void);
#else
-static inline void arch_set_max_freq_ratio(bool turbo_disabled)
-{
-}
-static inline void init_freq_invariance(bool secondary, bool cppc_ready)
-{
-}
+static inline void arch_set_max_freq_ratio(bool turbo_disabled) { }
+static inline void bp_init_freq_invariance(bool cppc_ready) { }
+static inline void ap_init_freq_invariance(void) { }
#endif

#ifdef CONFIG_ACPI_CPPC_LIB
--- a/arch/x86/kernel/acpi/cppc.c
+++ b/arch/x86/kernel/acpi/cppc.c
@@ -96,7 +96,8 @@ void init_freq_invariance_cppc(void)

mutex_lock(&freq_invariance_lock);

- init_freq_invariance(secondary, true);
+ if (!secondary)
+ bp_init_freq_invariance(true);
secondary = true;

mutex_unlock(&freq_invariance_lock);
--- a/arch/x86/kernel/cpu/aperfmperf.c
+++ b/arch/x86/kernel/cpu/aperfmperf.c
@@ -428,31 +428,24 @@ static void register_freq_invariance_sys
static inline void register_freq_invariance_syscore_ops(void) {}
#endif

-void init_freq_invariance(bool secondary, bool cppc_ready)
+void bp_init_freq_invariance(bool cppc_ready)
{
- bool ret = false;
+ bool ret;

- if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
+ if (!cpu_feature_enabled(X86_FEATURE_APERFMPERF))
return;

- if (secondary) {
- if (static_branch_likely(&arch_scale_freq_key)) {
- init_counter_refs();
- }
- return;
- }
+ init_counter_refs();

if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
ret = intel_set_max_freq_ratio();
else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
- if (!cppc_ready) {
+ if (!cppc_ready)
return;
- }
ret = amd_set_max_freq_ratio(&arch_turbo_freq_ratio);
}

if (ret) {
- init_counter_refs();
static_branch_enable(&arch_scale_freq_key);
register_freq_invariance_syscore_ops();
pr_info("Estimated ratio of average max frequency by base frequency (times 1024): %llu\n", arch_max_freq_ratio);
@@ -461,6 +454,12 @@ void init_freq_invariance(bool secondary
}
}

+void ap_init_freq_invariance(void)
+{
+ if (cpu_feature_enabled(X86_FEATURE_APERFMPERF))
+ init_counter_refs();
+}
+
static void disable_freq_invariance_workfn(struct work_struct *work)
{
static_branch_disable(&arch_scale_freq_key);
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -186,7 +186,7 @@ static void smp_callin(void)
*/
set_cpu_sibling_map(raw_smp_processor_id());

- init_freq_invariance(true, false);
+ ap_init_freq_invariance();

/*
* Get our bogomips.
@@ -1396,7 +1396,7 @@ void __init native_smp_prepare_cpus(unsi
{
smp_prepare_cpus_common();

- init_freq_invariance(false, false);
+ bp_init_freq_invariance(false);
smp_sanity_check();

switch (apic_intr_mode) {


2022-04-21 16:41:29

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [patch 03/10] x86/aperfmperf: Separate AP/BP frequency invariance init

On Fri, Apr 15, 2022 at 9:19 PM Thomas Gleixner <[email protected]> wrote:
>
> This code is convoluted and because it can be invoked post init via the
> ACPI/CPPC code, all of the initialization functionality is built in instead
> of being part of init text and init data.
>
> As a first step create separate calls for the boot and the application
> processors.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> arch/x86/include/asm/topology.h | 12 +++++-------
> arch/x86/kernel/acpi/cppc.c | 3 ++-
> arch/x86/kernel/cpu/aperfmperf.c | 23 +++++++++++------------
> arch/x86/kernel/smpboot.c | 4 ++--
> 4 files changed, 20 insertions(+), 22 deletions(-)
>
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -216,14 +216,12 @@ extern void arch_scale_freq_tick(void);
> #define arch_scale_freq_tick arch_scale_freq_tick
>
> extern void arch_set_max_freq_ratio(bool turbo_disabled);
> -void init_freq_invariance(bool secondary, bool cppc_ready);
> +extern void bp_init_freq_invariance(bool cppc_ready);
> +extern void ap_init_freq_invariance(void);
> #else
> -static inline void arch_set_max_freq_ratio(bool turbo_disabled)
> -{
> -}
> -static inline void init_freq_invariance(bool secondary, bool cppc_ready)
> -{
> -}
> +static inline void arch_set_max_freq_ratio(bool turbo_disabled) { }
> +static inline void bp_init_freq_invariance(bool cppc_ready) { }
> +static inline void ap_init_freq_invariance(void) { }
> #endif
>
> #ifdef CONFIG_ACPI_CPPC_LIB
> --- a/arch/x86/kernel/acpi/cppc.c
> +++ b/arch/x86/kernel/acpi/cppc.c
> @@ -96,7 +96,8 @@ void init_freq_invariance_cppc(void)
>
> mutex_lock(&freq_invariance_lock);
>
> - init_freq_invariance(secondary, true);
> + if (!secondary)
> + bp_init_freq_invariance(true);
> secondary = true;
>
> mutex_unlock(&freq_invariance_lock);
> --- a/arch/x86/kernel/cpu/aperfmperf.c
> +++ b/arch/x86/kernel/cpu/aperfmperf.c
> @@ -428,31 +428,24 @@ static void register_freq_invariance_sys
> static inline void register_freq_invariance_syscore_ops(void) {}
> #endif
>
> -void init_freq_invariance(bool secondary, bool cppc_ready)
> +void bp_init_freq_invariance(bool cppc_ready)
> {
> - bool ret = false;
> + bool ret;
>
> - if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
> + if (!cpu_feature_enabled(X86_FEATURE_APERFMPERF))
> return;
>
> - if (secondary) {
> - if (static_branch_likely(&arch_scale_freq_key)) {
> - init_counter_refs();
> - }
> - return;
> - }
> + init_counter_refs();
>
> if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
> ret = intel_set_max_freq_ratio();
> else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
> - if (!cppc_ready) {
> + if (!cppc_ready)
> return;
> - }
> ret = amd_set_max_freq_ratio(&arch_turbo_freq_ratio);
> }
>
> if (ret) {
> - init_counter_refs();
> static_branch_enable(&arch_scale_freq_key);
> register_freq_invariance_syscore_ops();
> pr_info("Estimated ratio of average max frequency by base frequency (times 1024): %llu\n", arch_max_freq_ratio);
> @@ -461,6 +454,12 @@ void init_freq_invariance(bool secondary
> }
> }
>
> +void ap_init_freq_invariance(void)
> +{
> + if (cpu_feature_enabled(X86_FEATURE_APERFMPERF))
> + init_counter_refs();

This doesn't check arch_scale_freq_key now which may be a good thing
to mention in the changelog.

I don't see anything questionable in the patch, though, so

Reviewed-by: Rafael J. Wysocki <[email protected]>

> +}
> +
> static void disable_freq_invariance_workfn(struct work_struct *work)
> {
> static_branch_disable(&arch_scale_freq_key);
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -186,7 +186,7 @@ static void smp_callin(void)
> */
> set_cpu_sibling_map(raw_smp_processor_id());
>
> - init_freq_invariance(true, false);
> + ap_init_freq_invariance();
>
> /*
> * Get our bogomips.
> @@ -1396,7 +1396,7 @@ void __init native_smp_prepare_cpus(unsi
> {
> smp_prepare_cpus_common();
>
> - init_freq_invariance(false, false);
> + bp_init_freq_invariance(false);
> smp_sanity_check();
>
> switch (apic_intr_mode) {
>

Subject: [tip: x86/cleanups] x86/aperfmperf: Separate AP/BP frequency invariance init

The following commit has been merged into the x86/cleanups branch of tip:

Commit-ID: 138a7f9c6beae8d652113b8e7a44994b4200bbcd
Gitweb: https://git.kernel.org/tip/138a7f9c6beae8d652113b8e7a44994b4200bbcd
Author: Thomas Gleixner <[email protected]>
AuthorDate: Fri, 15 Apr 2022 21:19:53 +02:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Wed, 27 Apr 2022 15:51:08 +02:00

x86/aperfmperf: Separate AP/BP frequency invariance init

This code is convoluted and because it can be invoked post init via the
ACPI/CPPC code, all of the initialization functionality is built in instead
of being part of init text and init data.

As a first step create separate calls for the boot and the application
processors.

Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Rafael J. Wysocki <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Acked-by: Paul E. McKenney <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

---
arch/x86/include/asm/topology.h | 12 +++++-------
arch/x86/kernel/acpi/cppc.c | 3 ++-
arch/x86/kernel/cpu/aperfmperf.c | 23 +++++++++++------------
arch/x86/kernel/smpboot.c | 4 ++--
4 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index 9619385..e2faedc 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -216,14 +216,12 @@ extern void arch_scale_freq_tick(void);
#define arch_scale_freq_tick arch_scale_freq_tick

extern void arch_set_max_freq_ratio(bool turbo_disabled);
-void init_freq_invariance(bool secondary, bool cppc_ready);
+extern void bp_init_freq_invariance(bool cppc_ready);
+extern void ap_init_freq_invariance(void);
#else
-static inline void arch_set_max_freq_ratio(bool turbo_disabled)
-{
-}
-static inline void init_freq_invariance(bool secondary, bool cppc_ready)
-{
-}
+static inline void arch_set_max_freq_ratio(bool turbo_disabled) { }
+static inline void bp_init_freq_invariance(bool cppc_ready) { }
+static inline void ap_init_freq_invariance(void) { }
#endif

#ifdef CONFIG_ACPI_CPPC_LIB
diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
index df1644d..06109d9 100644
--- a/arch/x86/kernel/acpi/cppc.c
+++ b/arch/x86/kernel/acpi/cppc.c
@@ -96,7 +96,8 @@ void init_freq_invariance_cppc(void)

mutex_lock(&freq_invariance_lock);

- init_freq_invariance(secondary, true);
+ if (!secondary)
+ bp_init_freq_invariance(true);
secondary = true;

mutex_unlock(&freq_invariance_lock);
diff --git a/arch/x86/kernel/cpu/aperfmperf.c b/arch/x86/kernel/cpu/aperfmperf.c
index 35fff01..87f34f2 100644
--- a/arch/x86/kernel/cpu/aperfmperf.c
+++ b/arch/x86/kernel/cpu/aperfmperf.c
@@ -428,31 +428,24 @@ static void register_freq_invariance_syscore_ops(void)
static inline void register_freq_invariance_syscore_ops(void) {}
#endif

-void init_freq_invariance(bool secondary, bool cppc_ready)
+void bp_init_freq_invariance(bool cppc_ready)
{
- bool ret = false;
+ bool ret;

- if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
+ if (!cpu_feature_enabled(X86_FEATURE_APERFMPERF))
return;

- if (secondary) {
- if (static_branch_likely(&arch_scale_freq_key)) {
- init_counter_refs();
- }
- return;
- }
+ init_counter_refs();

if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL)
ret = intel_set_max_freq_ratio();
else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
- if (!cppc_ready) {
+ if (!cppc_ready)
return;
- }
ret = amd_set_max_freq_ratio(&arch_turbo_freq_ratio);
}

if (ret) {
- init_counter_refs();
static_branch_enable(&arch_scale_freq_key);
register_freq_invariance_syscore_ops();
pr_info("Estimated ratio of average max frequency by base frequency (times 1024): %llu\n", arch_max_freq_ratio);
@@ -461,6 +454,12 @@ void init_freq_invariance(bool secondary, bool cppc_ready)
}
}

+void ap_init_freq_invariance(void)
+{
+ if (cpu_feature_enabled(X86_FEATURE_APERFMPERF))
+ init_counter_refs();
+}
+
static void disable_freq_invariance_workfn(struct work_struct *work)
{
static_branch_disable(&arch_scale_freq_key);
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index a9fc16a..023feb4 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -186,7 +186,7 @@ static void smp_callin(void)
*/
set_cpu_sibling_map(raw_smp_processor_id());

- init_freq_invariance(true, false);
+ ap_init_freq_invariance();

/*
* Get our bogomips.
@@ -1396,7 +1396,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
{
smp_prepare_cpus_common();

- init_freq_invariance(false, false);
+ bp_init_freq_invariance(false);
smp_sanity_check();

switch (apic_intr_mode) {