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

[lihuisong (C)]

在 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.
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