* [PATCH] cpufreq: acpi: Defer setting boost MSRs
@ 2022-11-02 19:59 Stuart Hayes
2022-11-03 18:19 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Stuart Hayes @ 2022-11-02 19:59 UTC (permalink / raw)
To: Rafael J . Wysocki, Viresh Kumar, Stuart Hayes, Kyle Meyer
Cc: linux-pm, linux-kernel
When acpi-cpufreq is loaded, boost is enabled on every CPU (by setting an
MSR) before the driver is registered with cpufreq. This can be very time
consuming, because it is done with a CPU hotplug startup callback, and
cpuhp_setup_state() schedules the callback (cpufreq_boost_online()) to run
on each CPU one at a time, waiting for each to run before calling the next.
If cpufreq_register_driver() fails--if, for example, there are no ACPI
P-states present--this is wasted time.
Since cpufreq already sets up a CPU hotplug startup callback if and when
acpi-cpufreq is registered, set the boost MSRs in acpi_cpufreq_cpu_init(),
which is called by the cpufreq cpuhp callback. This allows acpi-cpufreq to
exit quickly if it is loaded but not needed.
On one system with 192 CPUs, this patch speeds up boot by about 30 seconds.
Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
---
drivers/cpufreq/acpi-cpufreq.c | 31 +++----------------------------
1 file changed, 3 insertions(+), 28 deletions(-)
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index 1bb2b90ebb21..cb167263de72 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -535,15 +535,6 @@ static void free_acpi_perf_data(void)
free_percpu(acpi_perf_data);
}
-static int cpufreq_boost_online(unsigned int cpu)
-{
- /*
- * On the CPU_UP path we simply keep the boost-disable flag
- * in sync with the current global state.
- */
- return boost_set_msr(acpi_cpufreq_driver.boost_enabled);
-}
-
static int cpufreq_boost_down_prep(unsigned int cpu)
{
/*
@@ -897,6 +888,8 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
if (perf->states[0].core_frequency * 1000 != freq_table[0].frequency)
pr_warn(FW_WARN "P-state 0 is not max freq\n");
+ set_boost(policy, acpi_cpufreq_driver.boost_enabled);
+
return result;
err_unreg:
@@ -916,6 +909,7 @@ static int acpi_cpufreq_cpu_exit(struct cpufreq_policy *policy)
pr_debug("%s\n", __func__);
+ cpufreq_boost_down_prep(policy->cpu);
policy->fast_switch_possible = false;
policy->driver_data = NULL;
acpi_processor_unregister_performance(data->acpi_perf_cpu);
@@ -972,25 +966,9 @@ static void __init acpi_cpufreq_boost_init(void)
acpi_cpufreq_driver.set_boost = set_boost;
acpi_cpufreq_driver.boost_enabled = boost_state(0);
- /*
- * This calls the online callback on all online cpu and forces all
- * MSRs to the same value.
- */
- ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "cpufreq/acpi:online",
- cpufreq_boost_online, cpufreq_boost_down_prep);
- if (ret < 0) {
- pr_err("acpi_cpufreq: failed to register hotplug callbacks\n");
- return;
- }
acpi_cpufreq_online = ret;
}
-static void acpi_cpufreq_boost_exit(void)
-{
- if (acpi_cpufreq_online > 0)
- cpuhp_remove_state_nocalls(acpi_cpufreq_online);
-}
-
static int __init acpi_cpufreq_init(void)
{
int ret;
@@ -1032,7 +1010,6 @@ static int __init acpi_cpufreq_init(void)
ret = cpufreq_register_driver(&acpi_cpufreq_driver);
if (ret) {
free_acpi_perf_data();
- acpi_cpufreq_boost_exit();
}
return ret;
}
@@ -1041,8 +1018,6 @@ static void __exit acpi_cpufreq_exit(void)
{
pr_debug("%s\n", __func__);
- acpi_cpufreq_boost_exit();
-
cpufreq_unregister_driver(&acpi_cpufreq_driver);
free_acpi_perf_data();
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] cpufreq: acpi: Defer setting boost MSRs
2022-11-02 19:59 [PATCH] cpufreq: acpi: Defer setting boost MSRs Stuart Hayes
@ 2022-11-03 18:19 ` Rafael J. Wysocki
2022-12-04 18:29 ` Borislav Petkov
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2022-11-03 18:19 UTC (permalink / raw)
To: Stuart Hayes
Cc: Rafael J . Wysocki, Viresh Kumar, Kyle Meyer, linux-pm, linux-kernel
On Wed, Nov 2, 2022 at 9:01 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
>
> When acpi-cpufreq is loaded, boost is enabled on every CPU (by setting an
> MSR) before the driver is registered with cpufreq. This can be very time
> consuming, because it is done with a CPU hotplug startup callback, and
> cpuhp_setup_state() schedules the callback (cpufreq_boost_online()) to run
> on each CPU one at a time, waiting for each to run before calling the next.
>
> If cpufreq_register_driver() fails--if, for example, there are no ACPI
> P-states present--this is wasted time.
>
> Since cpufreq already sets up a CPU hotplug startup callback if and when
> acpi-cpufreq is registered, set the boost MSRs in acpi_cpufreq_cpu_init(),
> which is called by the cpufreq cpuhp callback. This allows acpi-cpufreq to
> exit quickly if it is loaded but not needed.
>
> On one system with 192 CPUs, this patch speeds up boot by about 30 seconds.
>
> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> ---
> drivers/cpufreq/acpi-cpufreq.c | 31 +++----------------------------
> 1 file changed, 3 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index 1bb2b90ebb21..cb167263de72 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -535,15 +535,6 @@ static void free_acpi_perf_data(void)
> free_percpu(acpi_perf_data);
> }
>
> -static int cpufreq_boost_online(unsigned int cpu)
> -{
> - /*
> - * On the CPU_UP path we simply keep the boost-disable flag
> - * in sync with the current global state.
> - */
> - return boost_set_msr(acpi_cpufreq_driver.boost_enabled);
> -}
> -
> static int cpufreq_boost_down_prep(unsigned int cpu)
> {
> /*
> @@ -897,6 +888,8 @@ static int acpi_cpufreq_cpu_init(struct cpufreq_policy *policy)
> if (perf->states[0].core_frequency * 1000 != freq_table[0].frequency)
> pr_warn(FW_WARN "P-state 0 is not max freq\n");
>
> + set_boost(policy, acpi_cpufreq_driver.boost_enabled);
> +
> return result;
>
> err_unreg:
> @@ -916,6 +909,7 @@ static int acpi_cpufreq_cpu_exit(struct cpufreq_policy *policy)
>
> pr_debug("%s\n", __func__);
>
> + cpufreq_boost_down_prep(policy->cpu);
> policy->fast_switch_possible = false;
> policy->driver_data = NULL;
> acpi_processor_unregister_performance(data->acpi_perf_cpu);
> @@ -972,25 +966,9 @@ static void __init acpi_cpufreq_boost_init(void)
> acpi_cpufreq_driver.set_boost = set_boost;
> acpi_cpufreq_driver.boost_enabled = boost_state(0);
>
> - /*
> - * This calls the online callback on all online cpu and forces all
> - * MSRs to the same value.
> - */
> - ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "cpufreq/acpi:online",
> - cpufreq_boost_online, cpufreq_boost_down_prep);
> - if (ret < 0) {
> - pr_err("acpi_cpufreq: failed to register hotplug callbacks\n");
> - return;
> - }
> acpi_cpufreq_online = ret;
> }
>
> -static void acpi_cpufreq_boost_exit(void)
> -{
> - if (acpi_cpufreq_online > 0)
> - cpuhp_remove_state_nocalls(acpi_cpufreq_online);
> -}
> -
> static int __init acpi_cpufreq_init(void)
> {
> int ret;
> @@ -1032,7 +1010,6 @@ static int __init acpi_cpufreq_init(void)
> ret = cpufreq_register_driver(&acpi_cpufreq_driver);
> if (ret) {
> free_acpi_perf_data();
> - acpi_cpufreq_boost_exit();
> }
> return ret;
> }
> @@ -1041,8 +1018,6 @@ static void __exit acpi_cpufreq_exit(void)
> {
> pr_debug("%s\n", __func__);
>
> - acpi_cpufreq_boost_exit();
> -
> cpufreq_unregister_driver(&acpi_cpufreq_driver);
>
> free_acpi_perf_data();
> --
Applied as 6.2 material, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] cpufreq: acpi: Defer setting boost MSRs
2022-11-03 18:19 ` Rafael J. Wysocki
@ 2022-12-04 18:29 ` Borislav Petkov
2022-12-04 19:20 ` stuart hayes
0 siblings, 1 reply; 6+ messages in thread
From: Borislav Petkov @ 2022-12-04 18:29 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Stuart Hayes, Viresh Kumar, Kyle Meyer, linux-pm, linux-kernel
On Thu, Nov 03, 2022 at 07:19:47PM +0100, Rafael J. Wysocki wrote:
> On Wed, Nov 2, 2022 at 9:01 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
> >
> > When acpi-cpufreq is loaded, boost is enabled on every CPU (by setting an
> > MSR) before the driver is registered with cpufreq. This can be very time
> > consuming, because it is done with a CPU hotplug startup callback, and
> > cpuhp_setup_state() schedules the callback (cpufreq_boost_online()) to run
> > on each CPU one at a time, waiting for each to run before calling the next.
> >
> > If cpufreq_register_driver() fails--if, for example, there are no ACPI
> > P-states present--this is wasted time.
> >
> > Since cpufreq already sets up a CPU hotplug startup callback if and when
> > acpi-cpufreq is registered, set the boost MSRs in acpi_cpufreq_cpu_init(),
> > which is called by the cpufreq cpuhp callback. This allows acpi-cpufreq to
> > exit quickly if it is loaded but not needed.
> >
> > On one system with 192 CPUs, this patch speeds up boot by about 30 seconds.
> >
> > Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> > ---
> > drivers/cpufreq/acpi-cpufreq.c | 31 +++----------------------------
> > 1 file changed, 3 insertions(+), 28 deletions(-)
...
> Applied as 6.2 material, thanks!
My 32-bit Atom doesn't like this one. Reverting fixes it ofc.
[ 22.780260] unchecked MSR access error: WRMSR to 0x1a0 (tried to write 0x0000004364950488) at rIP: 0xf80b37d4 (boost_set_msr.isra.0+0x9c/0x114 [acpi_cpufreq])
[ 22.781186] Call Trace:
[ 22.781186] boost_set_msr_each+0x15/0x1c [acpi_cpufreq]
[ 22.781186] __flush_smp_call_function_queue+0x132/0x1cc
[ 22.781186] ? sysvec_call_function+0x30/0x30
[ 22.781186] generic_smp_call_function_single_interrupt+0x12/0x18
[ 22.781186] __sysvec_call_function_single.constprop.0+0x43/0x1d8
[ 22.781186] sysvec_call_function_single+0x18/0x30
[ 22.781186] handle_exception+0x133/0x133
[ 22.781186] EIP: finish_task_switch.isra.0+0x124/0x3a8
[ 22.781186] Code: d8 e8 8c 16 92 00 85 f6 75 e8 a1 04 24 6c c2 85 c0 0f 8f 9b 00 00 00 89 d8 e8 e4 02 92 00 e8 53 9e 0b 00 fb 64 a1 40 f9 69 c2 <8b> 80 5c 0f 00 00 85 c0 0f 85 72 01 00 00 a1 28 24 6c c2 64 8b 15
[ 22.781186] EAX: c32e2700 EBX: f748af40 ECX: 00000000 EDX: c1c3876e
[ 22.781186] ESI: 00000000 EDI: 00000000 EBP: c3241f90 ESP: c3241f78
[ 22.781186] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00000206
[ 22.781186] ? uevent_seqnum_show+0x1b/0x28
[ 22.781186] ? pid_list_refill_irq+0x128/0x1c0
[ 22.781186] ? sysvec_call_function+0x30/0x30
[ 22.781186] ? pid_list_refill_irq+0x128/0x1c0
[ 22.781186] ? sysvec_call_function+0x30/0x30
[ 22.781186] ? finish_task_switch.isra.0+0x124/0x3a8
[ 22.781186] schedule_tail+0x12/0x78
[ 22.781186] schedule_tail_wrapper+0x9/0x10
[ 22.781186] ret_from_fork+0x5/0x28
[ 22.781186] EIP: 0xb7fba549
[ 22.781186] Code: Unable to access opcode bytes at 0xb7fba51f.
[ 22.781186] EAX: 00000000 EBX: 01200011 ECX: 00000000 EDX: 00000000
[ 22.781186] ESI: 00000000 EDI: b7bfe868 EBP: 00000000 ESP: bfcfedc0
[ 22.781186] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000246
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] cpufreq: acpi: Defer setting boost MSRs
2022-12-04 18:29 ` Borislav Petkov
@ 2022-12-04 19:20 ` stuart hayes
2022-12-05 12:43 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: stuart hayes @ 2022-12-04 19:20 UTC (permalink / raw)
To: Borislav Petkov, Rafael J. Wysocki
Cc: Viresh Kumar, Kyle Meyer, linux-pm, linux-kernel
On 12/4/2022 12:29 PM, Borislav Petkov wrote:
> On Thu, Nov 03, 2022 at 07:19:47PM +0100, Rafael J. Wysocki wrote:
>> On Wed, Nov 2, 2022 at 9:01 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
>>>
>>> When acpi-cpufreq is loaded, boost is enabled on every CPU (by setting an
>>> MSR) before the driver is registered with cpufreq. This can be very time
>>> consuming, because it is done with a CPU hotplug startup callback, and
>>> cpuhp_setup_state() schedules the callback (cpufreq_boost_online()) to run
>>> on each CPU one at a time, waiting for each to run before calling the next.
>>>
>>> If cpufreq_register_driver() fails--if, for example, there are no ACPI
>>> P-states present--this is wasted time.
>>>
>>> Since cpufreq already sets up a CPU hotplug startup callback if and when
>>> acpi-cpufreq is registered, set the boost MSRs in acpi_cpufreq_cpu_init(),
>>> which is called by the cpufreq cpuhp callback. This allows acpi-cpufreq to
>>> exit quickly if it is loaded but not needed.
>>>
>>> On one system with 192 CPUs, this patch speeds up boot by about 30 seconds.
>>>
>>> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
>>> ---
>>> drivers/cpufreq/acpi-cpufreq.c | 31 +++----------------------------
>>> 1 file changed, 3 insertions(+), 28 deletions(-)
>
> ...
>
>> Applied as 6.2 material, thanks!
>
> My 32-bit Atom doesn't like this one. Reverting fixes it ofc.
>
> [ 22.780260] unchecked MSR access error: WRMSR to 0x1a0 (tried to write 0x0000004364950488) at rIP: 0xf80b37d4 (boost_set_msr.isra.0+0x9c/0x114 [acpi_cpufreq])
> [ 22.781186] Call Trace:
> [ 22.781186] boost_set_msr_each+0x15/0x1c [acpi_cpufreq]
> [ 22.781186] __flush_smp_call_function_queue+0x132/0x1cc
> [ 22.781186] ? sysvec_call_function+0x30/0x30
> [ 22.781186] generic_smp_call_function_single_interrupt+0x12/0x18
> [ 22.781186] __sysvec_call_function_single.constprop.0+0x43/0x1d8
> [ 22.781186] sysvec_call_function_single+0x18/0x30
> [ 22.781186] handle_exception+0x133/0x133
> [ 22.781186] EIP: finish_task_switch.isra.0+0x124/0x3a8
> [ 22.781186] Code: d8 e8 8c 16 92 00 85 f6 75 e8 a1 04 24 6c c2 85 c0 0f 8f 9b 00 00 00 89 d8 e8 e4 02 92 00 e8 53 9e 0b 00 fb 64 a1 40 f9 69 c2 <8b> 80 5c 0f 00 00 85 c0 0f 85 72 01 00 00 a1 28 24 6c c2 64 8b 15
> [ 22.781186] EAX: c32e2700 EBX: f748af40 ECX: 00000000 EDX: c1c3876e
> [ 22.781186] ESI: 00000000 EDI: 00000000 EBP: c3241f90 ESP: c3241f78
> [ 22.781186] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00000206
> [ 22.781186] ? uevent_seqnum_show+0x1b/0x28
> [ 22.781186] ? pid_list_refill_irq+0x128/0x1c0
> [ 22.781186] ? sysvec_call_function+0x30/0x30
> [ 22.781186] ? pid_list_refill_irq+0x128/0x1c0
> [ 22.781186] ? sysvec_call_function+0x30/0x30
> [ 22.781186] ? finish_task_switch.isra.0+0x124/0x3a8
> [ 22.781186] schedule_tail+0x12/0x78
> [ 22.781186] schedule_tail_wrapper+0x9/0x10
> [ 22.781186] ret_from_fork+0x5/0x28
> [ 22.781186] EIP: 0xb7fba549
> [ 22.781186] Code: Unable to access opcode bytes at 0xb7fba51f.
> [ 22.781186] EAX: 00000000 EBX: 01200011 ECX: 00000000 EDX: 00000000
> [ 22.781186] ESI: 00000000 EDI: b7bfe868 EBP: 00000000 ESP: bfcfedc0
> [ 22.781186] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000246
>
I believe I see the problem... acpi_cpufreq_cpu_init is calling set_boost() directly without checking to see if acpi_cpufreq_driver.set_boost was set, so it is trying to set the MSR on CPUs that don't support it.
Thanks, I can submit a patch to fix this.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cpufreq: acpi: Defer setting boost MSRs
2022-12-04 19:20 ` stuart hayes
@ 2022-12-05 12:43 ` Rafael J. Wysocki
2022-12-05 22:57 ` stuart hayes
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2022-12-05 12:43 UTC (permalink / raw)
To: stuart hayes
Cc: Borislav Petkov, Rafael J. Wysocki, Viresh Kumar, Kyle Meyer,
linux-pm, linux-kernel
On Sun, Dec 4, 2022 at 8:20 PM stuart hayes <stuart.w.hayes@gmail.com> wrote:
>
>
>
> On 12/4/2022 12:29 PM, Borislav Petkov wrote:
> > On Thu, Nov 03, 2022 at 07:19:47PM +0100, Rafael J. Wysocki wrote:
> >> On Wed, Nov 2, 2022 at 9:01 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
> >>>
> >>> When acpi-cpufreq is loaded, boost is enabled on every CPU (by setting an
> >>> MSR) before the driver is registered with cpufreq. This can be very time
> >>> consuming, because it is done with a CPU hotplug startup callback, and
> >>> cpuhp_setup_state() schedules the callback (cpufreq_boost_online()) to run
> >>> on each CPU one at a time, waiting for each to run before calling the next.
> >>>
> >>> If cpufreq_register_driver() fails--if, for example, there are no ACPI
> >>> P-states present--this is wasted time.
> >>>
> >>> Since cpufreq already sets up a CPU hotplug startup callback if and when
> >>> acpi-cpufreq is registered, set the boost MSRs in acpi_cpufreq_cpu_init(),
> >>> which is called by the cpufreq cpuhp callback. This allows acpi-cpufreq to
> >>> exit quickly if it is loaded but not needed.
> >>>
> >>> On one system with 192 CPUs, this patch speeds up boot by about 30 seconds.
> >>>
> >>> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
> >>> ---
> >>> drivers/cpufreq/acpi-cpufreq.c | 31 +++----------------------------
> >>> 1 file changed, 3 insertions(+), 28 deletions(-)
> >
> > ...
> >
> >> Applied as 6.2 material, thanks!
> >
> > My 32-bit Atom doesn't like this one. Reverting fixes it ofc.
> >
> > [ 22.780260] unchecked MSR access error: WRMSR to 0x1a0 (tried to write 0x0000004364950488) at rIP: 0xf80b37d4 (boost_set_msr.isra.0+0x9c/0x114 [acpi_cpufreq])
> > [ 22.781186] Call Trace:
> > [ 22.781186] boost_set_msr_each+0x15/0x1c [acpi_cpufreq]
> > [ 22.781186] __flush_smp_call_function_queue+0x132/0x1cc
> > [ 22.781186] ? sysvec_call_function+0x30/0x30
> > [ 22.781186] generic_smp_call_function_single_interrupt+0x12/0x18
> > [ 22.781186] __sysvec_call_function_single.constprop.0+0x43/0x1d8
> > [ 22.781186] sysvec_call_function_single+0x18/0x30
> > [ 22.781186] handle_exception+0x133/0x133
> > [ 22.781186] EIP: finish_task_switch.isra.0+0x124/0x3a8
> > [ 22.781186] Code: d8 e8 8c 16 92 00 85 f6 75 e8 a1 04 24 6c c2 85 c0 0f 8f 9b 00 00 00 89 d8 e8 e4 02 92 00 e8 53 9e 0b 00 fb 64 a1 40 f9 69 c2 <8b> 80 5c 0f 00 00 85 c0 0f 85 72 01 00 00 a1 28 24 6c c2 64 8b 15
> > [ 22.781186] EAX: c32e2700 EBX: f748af40 ECX: 00000000 EDX: c1c3876e
> > [ 22.781186] ESI: 00000000 EDI: 00000000 EBP: c3241f90 ESP: c3241f78
> > [ 22.781186] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00000206
> > [ 22.781186] ? uevent_seqnum_show+0x1b/0x28
> > [ 22.781186] ? pid_list_refill_irq+0x128/0x1c0
> > [ 22.781186] ? sysvec_call_function+0x30/0x30
> > [ 22.781186] ? pid_list_refill_irq+0x128/0x1c0
> > [ 22.781186] ? sysvec_call_function+0x30/0x30
> > [ 22.781186] ? finish_task_switch.isra.0+0x124/0x3a8
> > [ 22.781186] schedule_tail+0x12/0x78
> > [ 22.781186] schedule_tail_wrapper+0x9/0x10
> > [ 22.781186] ret_from_fork+0x5/0x28
> > [ 22.781186] EIP: 0xb7fba549
> > [ 22.781186] Code: Unable to access opcode bytes at 0xb7fba51f.
> > [ 22.781186] EAX: 00000000 EBX: 01200011 ECX: 00000000 EDX: 00000000
> > [ 22.781186] ESI: 00000000 EDI: b7bfe868 EBP: 00000000 ESP: bfcfedc0
> > [ 22.781186] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000246
> >
>
> I believe I see the problem... acpi_cpufreq_cpu_init is calling set_boost() directly without checking to see if acpi_cpufreq_driver.set_boost was set, so it is trying to set the MSR on CPUs that don't support it.
>
> Thanks, I can submit a patch to fix this.
Yes, please.
Note that I need to get this fix shortly, though, or I will just
revert the problemating commit before the 6.2 merge window opens.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cpufreq: acpi: Defer setting boost MSRs
2022-12-05 12:43 ` Rafael J. Wysocki
@ 2022-12-05 22:57 ` stuart hayes
0 siblings, 0 replies; 6+ messages in thread
From: stuart hayes @ 2022-12-05 22:57 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Borislav Petkov, Viresh Kumar, Kyle Meyer, linux-pm, linux-kernel
On 12/5/2022 6:43 AM, Rafael J. Wysocki wrote:
> On Sun, Dec 4, 2022 at 8:20 PM stuart hayes <stuart.w.hayes@gmail.com> wrote:
>>
>>
>>
>> On 12/4/2022 12:29 PM, Borislav Petkov wrote:
>>> On Thu, Nov 03, 2022 at 07:19:47PM +0100, Rafael J. Wysocki wrote:
>>>> On Wed, Nov 2, 2022 at 9:01 PM Stuart Hayes <stuart.w.hayes@gmail.com> wrote:
>>>>>
>>>>> When acpi-cpufreq is loaded, boost is enabled on every CPU (by setting an
>>>>> MSR) before the driver is registered with cpufreq. This can be very time
>>>>> consuming, because it is done with a CPU hotplug startup callback, and
>>>>> cpuhp_setup_state() schedules the callback (cpufreq_boost_online()) to run
>>>>> on each CPU one at a time, waiting for each to run before calling the next.
>>>>>
>>>>> If cpufreq_register_driver() fails--if, for example, there are no ACPI
>>>>> P-states present--this is wasted time.
>>>>>
>>>>> Since cpufreq already sets up a CPU hotplug startup callback if and when
>>>>> acpi-cpufreq is registered, set the boost MSRs in acpi_cpufreq_cpu_init(),
>>>>> which is called by the cpufreq cpuhp callback. This allows acpi-cpufreq to
>>>>> exit quickly if it is loaded but not needed.
>>>>>
>>>>> On one system with 192 CPUs, this patch speeds up boot by about 30 seconds.
>>>>>
>>>>> Signed-off-by: Stuart Hayes <stuart.w.hayes@gmail.com>
>>>>> ---
>>>>> drivers/cpufreq/acpi-cpufreq.c | 31 +++----------------------------
>>>>> 1 file changed, 3 insertions(+), 28 deletions(-)
>>>
>>> ...
>>>
>>>> Applied as 6.2 material, thanks!
>>>
>>> My 32-bit Atom doesn't like this one. Reverting fixes it ofc.
>>>
>>> [ 22.780260] unchecked MSR access error: WRMSR to 0x1a0 (tried to write 0x0000004364950488) at rIP: 0xf80b37d4 (boost_set_msr.isra.0+0x9c/0x114 [acpi_cpufreq])
>>> [ 22.781186] Call Trace:
>>> [ 22.781186] boost_set_msr_each+0x15/0x1c [acpi_cpufreq]
>>> [ 22.781186] __flush_smp_call_function_queue+0x132/0x1cc
>>> [ 22.781186] ? sysvec_call_function+0x30/0x30
>>> [ 22.781186] generic_smp_call_function_single_interrupt+0x12/0x18
>>> [ 22.781186] __sysvec_call_function_single.constprop.0+0x43/0x1d8
>>> [ 22.781186] sysvec_call_function_single+0x18/0x30
>>> [ 22.781186] handle_exception+0x133/0x133
>>> [ 22.781186] EIP: finish_task_switch.isra.0+0x124/0x3a8
>>> [ 22.781186] Code: d8 e8 8c 16 92 00 85 f6 75 e8 a1 04 24 6c c2 85 c0 0f 8f 9b 00 00 00 89 d8 e8 e4 02 92 00 e8 53 9e 0b 00 fb 64 a1 40 f9 69 c2 <8b> 80 5c 0f 00 00 85 c0 0f 85 72 01 00 00 a1 28 24 6c c2 64 8b 15
>>> [ 22.781186] EAX: c32e2700 EBX: f748af40 ECX: 00000000 EDX: c1c3876e
>>> [ 22.781186] ESI: 00000000 EDI: 00000000 EBP: c3241f90 ESP: c3241f78
>>> [ 22.781186] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00000206
>>> [ 22.781186] ? uevent_seqnum_show+0x1b/0x28
>>> [ 22.781186] ? pid_list_refill_irq+0x128/0x1c0
>>> [ 22.781186] ? sysvec_call_function+0x30/0x30
>>> [ 22.781186] ? pid_list_refill_irq+0x128/0x1c0
>>> [ 22.781186] ? sysvec_call_function+0x30/0x30
>>> [ 22.781186] ? finish_task_switch.isra.0+0x124/0x3a8
>>> [ 22.781186] schedule_tail+0x12/0x78
>>> [ 22.781186] schedule_tail_wrapper+0x9/0x10
>>> [ 22.781186] ret_from_fork+0x5/0x28
>>> [ 22.781186] EIP: 0xb7fba549
>>> [ 22.781186] Code: Unable to access opcode bytes at 0xb7fba51f.
>>> [ 22.781186] EAX: 00000000 EBX: 01200011 ECX: 00000000 EDX: 00000000
>>> [ 22.781186] ESI: 00000000 EDI: b7bfe868 EBP: 00000000 ESP: bfcfedc0
>>> [ 22.781186] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000246
>>>
>>
>> I believe I see the problem... acpi_cpufreq_cpu_init is calling set_boost() directly without checking to see if acpi_cpufreq_driver.set_boost was set, so it is trying to set the MSR on CPUs that don't support it.
>>
>> Thanks, I can submit a patch to fix this.
>
> Yes, please.
>
> Note that I need to get this fix shortly, though, or I will just
> revert the problemating commit before the 6.2 merge window opens.
I sent it a few hours ago as "[PATCH] cpufreq: acpi: Only set boost MSRs on supported CPUs".
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-05 22:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-02 19:59 [PATCH] cpufreq: acpi: Defer setting boost MSRs Stuart Hayes
2022-11-03 18:19 ` Rafael J. Wysocki
2022-12-04 18:29 ` Borislav Petkov
2022-12-04 19:20 ` stuart hayes
2022-12-05 12:43 ` Rafael J. Wysocki
2022-12-05 22:57 ` stuart hayes
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).