Re: [PATCH v1 6/9] ACPI: processor: idle: Do not change power states if get power info failed
From: lihuisong (C)
Date: Sun Oct 26 2025 - 23:01:42 EST
在 2025/10/26 20:34, Rafael J. Wysocki 写道:
On Fri, Oct 24, 2025 at 11:10 AM lihuisong (C) <lihuisong@xxxxxxxxxx> wrote:
在 2025/10/22 3:49, Rafael J. Wysocki 写道:
On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@xxxxxxxxxx> wrote:
Driver will update power states when processor power states have been
changed. To prevent any other abnormal issues, here add the verification
for the result of getting power information, don't change power states
and one error log when get power information failed.
But the old states may not be usable any more in that case.
Yes
If you want to check the acpi_processor_get_power_info(), it should
disable ACPi idle entirely on failures.
From the modification of this patch, this cpuidle device will be
disabled if the acpi_processor_get_power_info(）fails to get on this device.
And the cpuidle of the device will be disabled according to the
definition of cpuidle_not_available().
We should not call disable_cpuidle() to disable cpuidle of all CPUs.
Since the same idle state data is used for all CPUs, I'd say cpuidle
Yes.
From the current implementation perspective, the idle state is
initialized by the first available CPU.
If there is one CPU get power management information failed later, the
ACPI idle driver doesn't disable cpuidle of all CPUs and
just doesn't register cpudile_device and enable cpuidle_device.
should be disabled for all of them in that case.
I can understand this. I think it is reasonable.
What do you think how to disable cpuidle of all CPUs here?
How about call disable_cpuidle() and disable all cpuidle devices?
Alternatively, check if it works for any of them and apply the data
from the CPU where it works to all of them. If it doesn't work for
any of them, there's nothing to apply.
How should we check if the idle states can work to all of CPUs?
So the modification in this patch is enough, right?
Fixes: f427e5f1cf75 ("ACPI / processor: Get power info before updating the C-states")
Signed-off-by: Huisong Li <lihuisong@xxxxxxxxxx>
---
drivers/acpi/processor_idle.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index b0d6b51ee363..92b231f5d514 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1315,6 +1315,7 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
int cpu;
struct acpi_processor *_pr;
struct cpuidle_device *dev;
+ int ret = 0;
if (disabled_by_idle_boot_param())
return 0;
@@ -1344,16 +1345,20 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
}
/* Populate Updated C-state information */
- acpi_processor_get_power_info(pr);
- acpi_processor_setup_cpuidle_states(pr);
+ ret = acpi_processor_get_power_info(pr);
+ if (ret)
+ pr_err("Get processor-%u power information failed.\n",
+ pr->id);
+ else
+ acpi_processor_setup_cpuidle_states(pr);
/* Enable all cpuidle devices */
for_each_online_cpu(cpu) {
_pr = per_cpu(processors, cpu);
if (!_pr || !_pr->flags.power_setup_done)
continue;
- acpi_processor_get_power_info(_pr);
- if (_pr->flags.power) {
+ ret = acpi_processor_get_power_info(_pr);
+ if (!ret && _pr->flags.power) {
dev = per_cpu(acpi_cpuidle_device, cpu);
acpi_processor_setup_cpuidle_dev(_pr, dev);
cpuidle_enable_device(dev);
@@ -1363,7 +1368,7 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
cpus_read_unlock();
}
- return 0;
+ return ret;
}
void acpi_processor_register_idle_driver(void)
--
2.33.0