Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 19 Apr 2022 18:04:04 +0200 | Subject | Re: [patch 03/10] x86/aperfmperf: Separate AP/BP frequency invariance init |
| |
On Fri, Apr 15, 2022 at 9:19 PM Thomas Gleixner <tglx@linutronix.de> 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 <tglx@linutronix.de> > --- > 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 <rafael.j.wysocki@intel.com>
> +} > + > 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) { >
| |