Re: [PATCH v1 6/9] ACPI: processor: idle: Do not change power states if get power info failed

["lihuisong (C)" <lihuisong@huawei.com>]

在 2025/10/22 3:49, Rafael J. Wysocki 写道:
> On Mon, Sep 29, 2025 at 11:38 AM Huisong Li <lihuisong@huawei.com> 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.
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@huawei.com>
>> ---
>>   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
>>