* [PATCH] PM: EM: Fix late boot with holes in CPU topology
@ 2025-08-31 21:43 Christian Loehle
2025-09-01 16:58 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Christian Loehle @ 2025-08-31 21:43 UTC (permalink / raw)
To: rafael, lukasz.luba
Cc: linux-pm, linux-kernel, dietmar.eggemann, kenneth.crudup,
Christian Loehle, stable
commit e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity
adjustment") added a mechanism to handle CPUs that come up late by
retrying when any of the `cpufreq_cpu_get()` call fails.
However, if there are holes in the CPU topology (offline CPUs, e.g.
nosmt), the first missing CPU causes the loop to break, preventing
subsequent online CPUs from being updated.
Instead of aborting on the first missing CPU policy, loop through all
and retry if any were missing.
Fixes: e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity adjustment")
Suggested-by: Kenneth Crudup <kenneth.crudup@gmail.com>
Reported-by: Kenneth Crudup <kenneth.crudup@gmail.com>
Closes: https://lore.kernel.org/linux-pm/40212796-734c-4140-8a85-854f72b8144d@panix.com/
Cc: stable@vger.kernel.org
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
kernel/power/energy_model.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index ea7995a25780..b63c2afc1379 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -778,7 +778,7 @@ void em_adjust_cpu_capacity(unsigned int cpu)
static void em_check_capacity_update(void)
{
cpumask_var_t cpu_done_mask;
- int cpu;
+ int cpu, failed_cpus = 0;
if (!zalloc_cpumask_var(&cpu_done_mask, GFP_KERNEL)) {
pr_warn("no free memory\n");
@@ -796,10 +796,8 @@ static void em_check_capacity_update(void)
policy = cpufreq_cpu_get(cpu);
if (!policy) {
- pr_debug("Accessing cpu%d policy failed\n", cpu);
- schedule_delayed_work(&em_update_work,
- msecs_to_jiffies(1000));
- break;
+ failed_cpus++;
+ continue;
}
cpufreq_cpu_put(policy);
@@ -814,6 +812,11 @@ static void em_check_capacity_update(void)
em_adjust_new_capacity(cpu, dev, pd);
}
+ if (failed_cpus) {
+ pr_debug("Accessing %d policies failed, retrying\n", failed_cpus);
+ schedule_delayed_work(&em_update_work, msecs_to_jiffies(1000));
+ }
+
free_cpumask_var(cpu_done_mask);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: EM: Fix late boot with holes in CPU topology
2025-08-31 21:43 [PATCH] PM: EM: Fix late boot with holes in CPU topology Christian Loehle
@ 2025-09-01 16:58 ` Rafael J. Wysocki
2025-09-01 17:33 ` Christian Loehle
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-09-01 16:58 UTC (permalink / raw)
To: Christian Loehle
Cc: rafael, lukasz.luba, linux-pm, linux-kernel, dietmar.eggemann,
kenneth.crudup, stable
On Sun, Aug 31, 2025 at 11:44 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> commit e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity
> adjustment") added a mechanism to handle CPUs that come up late by
> retrying when any of the `cpufreq_cpu_get()` call fails.
>
> However, if there are holes in the CPU topology (offline CPUs, e.g.
> nosmt), the first missing CPU causes the loop to break, preventing
> subsequent online CPUs from being updated.
> Instead of aborting on the first missing CPU policy, loop through all
> and retry if any were missing.
>
> Fixes: e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity adjustment")
> Suggested-by: Kenneth Crudup <kenneth.crudup@gmail.com>
> Reported-by: Kenneth Crudup <kenneth.crudup@gmail.com>
> Closes: https://lore.kernel.org/linux-pm/40212796-734c-4140-8a85-854f72b8144d@panix.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
> kernel/power/energy_model.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index ea7995a25780..b63c2afc1379 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -778,7 +778,7 @@ void em_adjust_cpu_capacity(unsigned int cpu)
> static void em_check_capacity_update(void)
> {
> cpumask_var_t cpu_done_mask;
> - int cpu;
> + int cpu, failed_cpus = 0;
>
> if (!zalloc_cpumask_var(&cpu_done_mask, GFP_KERNEL)) {
> pr_warn("no free memory\n");
> @@ -796,10 +796,8 @@ static void em_check_capacity_update(void)
>
> policy = cpufreq_cpu_get(cpu);
> if (!policy) {
> - pr_debug("Accessing cpu%d policy failed\n", cpu);
I'm still quite unsure why you want to stop printing this message. It
is kind of useful to know which policies have had to be retried, while
printing the number of them really isn't particularly useful. And
this is pr_debug(), so user selectable anyway.
So I'm inclined to retain the line above and drop the new pr_debug() below.
Please let me know if this is a problem.
> - schedule_delayed_work(&em_update_work,
> - msecs_to_jiffies(1000));
> - break;
> + failed_cpus++;
> + continue;
> }
> cpufreq_cpu_put(policy);
>
> @@ -814,6 +812,11 @@ static void em_check_capacity_update(void)
> em_adjust_new_capacity(cpu, dev, pd);
> }
>
> + if (failed_cpus) {
> + pr_debug("Accessing %d policies failed, retrying\n", failed_cpus);
> + schedule_delayed_work(&em_update_work, msecs_to_jiffies(1000));
> + }
> +
> free_cpumask_var(cpu_done_mask);
> }
>
> --
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: EM: Fix late boot with holes in CPU topology
2025-09-01 16:58 ` Rafael J. Wysocki
@ 2025-09-01 17:33 ` Christian Loehle
2025-09-01 17:41 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Christian Loehle @ 2025-09-01 17:33 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: lukasz.luba, linux-pm, linux-kernel, dietmar.eggemann,
kenneth.crudup, stable
On 9/1/25 17:58, Rafael J. Wysocki wrote:
> On Sun, Aug 31, 2025 at 11:44 PM Christian Loehle
> <christian.loehle@arm.com> wrote:
>>
>> commit e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity
>> adjustment") added a mechanism to handle CPUs that come up late by
>> retrying when any of the `cpufreq_cpu_get()` call fails.
>>
>> However, if there are holes in the CPU topology (offline CPUs, e.g.
>> nosmt), the first missing CPU causes the loop to break, preventing
>> subsequent online CPUs from being updated.
>> Instead of aborting on the first missing CPU policy, loop through all
>> and retry if any were missing.
>>
>> Fixes: e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity adjustment")
>> Suggested-by: Kenneth Crudup <kenneth.crudup@gmail.com>
>> Reported-by: Kenneth Crudup <kenneth.crudup@gmail.com>
>> Closes: https://lore.kernel.org/linux-pm/40212796-734c-4140-8a85-854f72b8144d@panix.com/
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>> ---
>> kernel/power/energy_model.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
>> index ea7995a25780..b63c2afc1379 100644
>> --- a/kernel/power/energy_model.c
>> +++ b/kernel/power/energy_model.c
>> @@ -778,7 +778,7 @@ void em_adjust_cpu_capacity(unsigned int cpu)
>> static void em_check_capacity_update(void)
>> {
>> cpumask_var_t cpu_done_mask;
>> - int cpu;
>> + int cpu, failed_cpus = 0;
>>
>> if (!zalloc_cpumask_var(&cpu_done_mask, GFP_KERNEL)) {
>> pr_warn("no free memory\n");
>> @@ -796,10 +796,8 @@ static void em_check_capacity_update(void)
>>
>> policy = cpufreq_cpu_get(cpu);
>> if (!policy) {
>> - pr_debug("Accessing cpu%d policy failed\n", cpu);
>
> I'm still quite unsure why you want to stop printing this message. It
> is kind of useful to know which policies have had to be retried, while
> printing the number of them really isn't particularly useful. And
> this is pr_debug(), so user selectable anyway.
>
> So I'm inclined to retain the line above and drop the new pr_debug() below.
>
> Please let me know if this is a problem.
For nosmt this leads to a lot of prints every seconds, that's all.
I can resend with the pr_debug for every fail, alternatively print a
cpumask.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: EM: Fix late boot with holes in CPU topology
2025-09-01 17:33 ` Christian Loehle
@ 2025-09-01 17:41 ` Rafael J. Wysocki
2025-09-01 19:47 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-09-01 17:41 UTC (permalink / raw)
To: Christian Loehle
Cc: Rafael J. Wysocki, lukasz.luba, linux-pm, linux-kernel,
dietmar.eggemann, kenneth.crudup, stable
On Mon, Sep 1, 2025 at 7:33 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 9/1/25 17:58, Rafael J. Wysocki wrote:
> > On Sun, Aug 31, 2025 at 11:44 PM Christian Loehle
> > <christian.loehle@arm.com> wrote:
> >>
> >> commit e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity
> >> adjustment") added a mechanism to handle CPUs that come up late by
> >> retrying when any of the `cpufreq_cpu_get()` call fails.
> >>
> >> However, if there are holes in the CPU topology (offline CPUs, e.g.
> >> nosmt), the first missing CPU causes the loop to break, preventing
> >> subsequent online CPUs from being updated.
> >> Instead of aborting on the first missing CPU policy, loop through all
> >> and retry if any were missing.
> >>
> >> Fixes: e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity adjustment")
> >> Suggested-by: Kenneth Crudup <kenneth.crudup@gmail.com>
> >> Reported-by: Kenneth Crudup <kenneth.crudup@gmail.com>
> >> Closes: https://lore.kernel.org/linux-pm/40212796-734c-4140-8a85-854f72b8144d@panix.com/
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> >> ---
> >> kernel/power/energy_model.c | 13 ++++++++-----
> >> 1 file changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> >> index ea7995a25780..b63c2afc1379 100644
> >> --- a/kernel/power/energy_model.c
> >> +++ b/kernel/power/energy_model.c
> >> @@ -778,7 +778,7 @@ void em_adjust_cpu_capacity(unsigned int cpu)
> >> static void em_check_capacity_update(void)
> >> {
> >> cpumask_var_t cpu_done_mask;
> >> - int cpu;
> >> + int cpu, failed_cpus = 0;
> >>
> >> if (!zalloc_cpumask_var(&cpu_done_mask, GFP_KERNEL)) {
> >> pr_warn("no free memory\n");
> >> @@ -796,10 +796,8 @@ static void em_check_capacity_update(void)
> >>
> >> policy = cpufreq_cpu_get(cpu);
> >> if (!policy) {
> >> - pr_debug("Accessing cpu%d policy failed\n", cpu);
> >
> > I'm still quite unsure why you want to stop printing this message. It
> > is kind of useful to know which policies have had to be retried, while
> > printing the number of them really isn't particularly useful. And
> > this is pr_debug(), so user selectable anyway.
> >
> > So I'm inclined to retain the line above and drop the new pr_debug() below.
> >
> > Please let me know if this is a problem.
>
> For nosmt this leads to a lot of prints every seconds, that's all.
> I can resend with the pr_debug for every fail, alternatively print a
> cpumask.
Printing a cpumask might be better, but it would add some complexity
only needed for the printing.
Maybe it's just better to not print anything at all.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: EM: Fix late boot with holes in CPU topology
2025-09-01 17:41 ` Rafael J. Wysocki
@ 2025-09-01 19:47 ` Rafael J. Wysocki
2025-09-01 21:17 ` Christian Loehle
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2025-09-01 19:47 UTC (permalink / raw)
To: Christian Loehle
Cc: lukasz.luba, linux-pm, linux-kernel, dietmar.eggemann,
kenneth.crudup, stable
On Mon, Sep 1, 2025 at 7:41 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Mon, Sep 1, 2025 at 7:33 PM Christian Loehle
> <christian.loehle@arm.com> wrote:
> >
> > On 9/1/25 17:58, Rafael J. Wysocki wrote:
> > > On Sun, Aug 31, 2025 at 11:44 PM Christian Loehle
> > > <christian.loehle@arm.com> wrote:
> > >>
> > >> commit e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity
> > >> adjustment") added a mechanism to handle CPUs that come up late by
> > >> retrying when any of the `cpufreq_cpu_get()` call fails.
> > >>
> > >> However, if there are holes in the CPU topology (offline CPUs, e.g.
> > >> nosmt), the first missing CPU causes the loop to break, preventing
> > >> subsequent online CPUs from being updated.
> > >> Instead of aborting on the first missing CPU policy, loop through all
> > >> and retry if any were missing.
> > >>
> > >> Fixes: e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity adjustment")
> > >> Suggested-by: Kenneth Crudup <kenneth.crudup@gmail.com>
> > >> Reported-by: Kenneth Crudup <kenneth.crudup@gmail.com>
> > >> Closes: https://lore.kernel.org/linux-pm/40212796-734c-4140-8a85-854f72b8144d@panix.com/
> > >> Cc: stable@vger.kernel.org
> > >> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> > >> ---
> > >> kernel/power/energy_model.c | 13 ++++++++-----
> > >> 1 file changed, 8 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> > >> index ea7995a25780..b63c2afc1379 100644
> > >> --- a/kernel/power/energy_model.c
> > >> +++ b/kernel/power/energy_model.c
> > >> @@ -778,7 +778,7 @@ void em_adjust_cpu_capacity(unsigned int cpu)
> > >> static void em_check_capacity_update(void)
> > >> {
> > >> cpumask_var_t cpu_done_mask;
> > >> - int cpu;
> > >> + int cpu, failed_cpus = 0;
> > >>
> > >> if (!zalloc_cpumask_var(&cpu_done_mask, GFP_KERNEL)) {
> > >> pr_warn("no free memory\n");
> > >> @@ -796,10 +796,8 @@ static void em_check_capacity_update(void)
> > >>
> > >> policy = cpufreq_cpu_get(cpu);
> > >> if (!policy) {
> > >> - pr_debug("Accessing cpu%d policy failed\n", cpu);
> > >
> > > I'm still quite unsure why you want to stop printing this message. It
> > > is kind of useful to know which policies have had to be retried, while
> > > printing the number of them really isn't particularly useful. And
> > > this is pr_debug(), so user selectable anyway.
> > >
> > > So I'm inclined to retain the line above and drop the new pr_debug() below.
> > >
> > > Please let me know if this is a problem.
> >
> > For nosmt this leads to a lot of prints every seconds, that's all.
> > I can resend with the pr_debug for every fail, alternatively print a
> > cpumask.
>
> Printing a cpumask might be better, but it would add some complexity
> only needed for the printing.
>
> Maybe it's just better to not print anything at all.
I've changed the patch to that effect and tentatively applied it, so
no need to resend if you agree with this modification.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: EM: Fix late boot with holes in CPU topology
2025-09-01 19:47 ` Rafael J. Wysocki
@ 2025-09-01 21:17 ` Christian Loehle
0 siblings, 0 replies; 6+ messages in thread
From: Christian Loehle @ 2025-09-01 21:17 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: lukasz.luba, linux-pm, linux-kernel, dietmar.eggemann,
kenneth.crudup, stable
On 9/1/25 20:47, Rafael J. Wysocki wrote:
> On Mon, Sep 1, 2025 at 7:41 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> On Mon, Sep 1, 2025 at 7:33 PM Christian Loehle
>> <christian.loehle@arm.com> wrote:
>>>
>>> On 9/1/25 17:58, Rafael J. Wysocki wrote:
>>>> On Sun, Aug 31, 2025 at 11:44 PM Christian Loehle
>>>> <christian.loehle@arm.com> wrote:
>>>>>
>>>>> commit e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity
>>>>> adjustment") added a mechanism to handle CPUs that come up late by
>>>>> retrying when any of the `cpufreq_cpu_get()` call fails.
>>>>>
>>>>> However, if there are holes in the CPU topology (offline CPUs, e.g.
>>>>> nosmt), the first missing CPU causes the loop to break, preventing
>>>>> subsequent online CPUs from being updated.
>>>>> Instead of aborting on the first missing CPU policy, loop through all
>>>>> and retry if any were missing.
>>>>>
>>>>> Fixes: e3f1164fc9ee ("PM: EM: Support late CPUs booting and capacity adjustment")
>>>>> Suggested-by: Kenneth Crudup <kenneth.crudup@gmail.com>
>>>>> Reported-by: Kenneth Crudup <kenneth.crudup@gmail.com>
>>>>> Closes: https://lore.kernel.org/linux-pm/40212796-734c-4140-8a85-854f72b8144d@panix.com/
>>>>> Cc: stable@vger.kernel.org
>>>>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>>>>> ---
>>>>> kernel/power/energy_model.c | 13 ++++++++-----
>>>>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
>>>>> index ea7995a25780..b63c2afc1379 100644
>>>>> --- a/kernel/power/energy_model.c
>>>>> +++ b/kernel/power/energy_model.c
>>>>> @@ -778,7 +778,7 @@ void em_adjust_cpu_capacity(unsigned int cpu)
>>>>> static void em_check_capacity_update(void)
>>>>> {
>>>>> cpumask_var_t cpu_done_mask;
>>>>> - int cpu;
>>>>> + int cpu, failed_cpus = 0;
>>>>>
>>>>> if (!zalloc_cpumask_var(&cpu_done_mask, GFP_KERNEL)) {
>>>>> pr_warn("no free memory\n");
>>>>> @@ -796,10 +796,8 @@ static void em_check_capacity_update(void)
>>>>>
>>>>> policy = cpufreq_cpu_get(cpu);
>>>>> if (!policy) {
>>>>> - pr_debug("Accessing cpu%d policy failed\n", cpu);
>>>>
>>>> I'm still quite unsure why you want to stop printing this message. It
>>>> is kind of useful to know which policies have had to be retried, while
>>>> printing the number of them really isn't particularly useful. And
>>>> this is pr_debug(), so user selectable anyway.
>>>>
>>>> So I'm inclined to retain the line above and drop the new pr_debug() below.
>>>>
>>>> Please let me know if this is a problem.
>>>
>>> For nosmt this leads to a lot of prints every seconds, that's all.
>>> I can resend with the pr_debug for every fail, alternatively print a
>>> cpumask.
>>
>> Printing a cpumask might be better, but it would add some complexity
>> only needed for the printing.
>>
>> Maybe it's just better to not print anything at all.
>
> I've changed the patch to that effect and tentatively applied it, so
> no need to resend if you agree with this modification.
>
> Thanks!
All good, thanks!
Yeah I was already leaning towards that now anyway.
I think Kenneth's report (which although he hasn't confirmed would be
that these prints were a red herring for him, they are expected) is
at least an indication that these prints might not be that useful after
all.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-01 21:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-31 21:43 [PATCH] PM: EM: Fix late boot with holes in CPU topology Christian Loehle
2025-09-01 16:58 ` Rafael J. Wysocki
2025-09-01 17:33 ` Christian Loehle
2025-09-01 17:41 ` Rafael J. Wysocki
2025-09-01 19:47 ` Rafael J. Wysocki
2025-09-01 21:17 ` Christian Loehle
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).