[PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[Huisong Li]

There are two resource rollback issues in acpi_processor_power_init:
1> Do not unregister acpi_idle_driver when do kzalloc failed.
2> Do not free cpuidle device memory when register cpuidle device failed.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 2c2dc559e0f8..3548ab9dac9e 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
 		}
 
 		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-		if (!dev)
-			return -ENOMEM;
+		if (!dev) {
+			retval = -ENOMEM;
+			goto unregister_driver;
+		}
 		per_cpu(acpi_cpuidle_device, pr->id) = dev;
 
 		acpi_processor_setup_cpuidle_dev(pr, dev);
@@ -1402,14 +1404,22 @@ int acpi_processor_power_init(struct acpi_processor *pr)
 		 * must already be registered before registering device
 		 */
 		retval = cpuidle_register_device(dev);
-		if (retval) {
-			if (acpi_processor_registered == 0)
-				cpuidle_unregister_driver(&acpi_idle_driver);
-			return retval;
-		}
+		if (retval)
+			goto free_cpuidle_device;
+
 		acpi_processor_registered++;
 	}
 	return 0;
+
+free_cpuidle_device:
+	per_cpu(acpi_cpuidle_device, pr->id) = NULL;
+	kfree(dev);
+
+unregister_driver:
+	if (acpi_processor_registered == 0)
+		cpuidle_unregister_driver(&acpi_idle_driver);
+
+	return retval;
 }
 
 int acpi_processor_power_exit(struct acpi_processor *pr)
-- 
2.33.0

Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[lihuisong (C)]

Kindly ping for review.

在 2025/6/19 14:13, Huisong Li 写道:
> There are two resource rollback issues in acpi_processor_power_init:
> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
> 2> Do not free cpuidle device memory when register cpuidle device failed.
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>   drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
>   1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 2c2dc559e0f8..3548ab9dac9e 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>   		}
>   
>   		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> -		if (!dev)
> -			return -ENOMEM;
> +		if (!dev) {
> +			retval = -ENOMEM;
> +			goto unregister_driver;
> +		}
>   		per_cpu(acpi_cpuidle_device, pr->id) = dev;
>   
>   		acpi_processor_setup_cpuidle_dev(pr, dev);
> @@ -1402,14 +1404,22 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>   		 * must already be registered before registering device
>   		 */
>   		retval = cpuidle_register_device(dev);
> -		if (retval) {
> -			if (acpi_processor_registered == 0)
> -				cpuidle_unregister_driver(&acpi_idle_driver);
> -			return retval;
> -		}
> +		if (retval)
> +			goto free_cpuidle_device;
> +
>   		acpi_processor_registered++;
>   	}
>   	return 0;
> +
> +free_cpuidle_device:
> +	per_cpu(acpi_cpuidle_device, pr->id) = NULL;
> +	kfree(dev);
> +
> +unregister_driver:
> +	if (acpi_processor_registered == 0)
> +		cpuidle_unregister_driver(&acpi_idle_driver);
> +
> +	return retval;
>   }
>   
>   int acpi_processor_power_exit(struct acpi_processor *pr)

Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[Rafael J. Wysocki]

On Thu, Jun 19, 2025 at 8:13 AM Huisong Li <lihuisong@huawei.com> wrote:
>
> There are two resource rollback issues in acpi_processor_power_init:
> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
> 2> Do not free cpuidle device memory when register cpuidle device failed.
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 2c2dc559e0f8..3548ab9dac9e 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>                 }
>
>                 dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> -               if (!dev)
> -                       return -ENOMEM;
> +               if (!dev) {
> +                       retval = -ENOMEM;
> +                       goto unregister_driver;

No, unregistering the driver here is pointless.

> +               }
>                 per_cpu(acpi_cpuidle_device, pr->id) = dev;
>
>                 acpi_processor_setup_cpuidle_dev(pr, dev);
> @@ -1402,14 +1404,22 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>                  * must already be registered before registering device
>                  */
>                 retval = cpuidle_register_device(dev);
> -               if (retval) {
> -                       if (acpi_processor_registered == 0)
> -                               cpuidle_unregister_driver(&acpi_idle_driver);

Pretty much the same as here.

It would be good to clean up dev here though.

> -                       return retval;
> -               }
> +               if (retval)
> +                       goto free_cpuidle_device;
> +
>                 acpi_processor_registered++;
>         }
>         return 0;
> +
> +free_cpuidle_device:
> +       per_cpu(acpi_cpuidle_device, pr->id) = NULL;
> +       kfree(dev);
> +
> +unregister_driver:
> +       if (acpi_processor_registered == 0)
> +               cpuidle_unregister_driver(&acpi_idle_driver);
> +
> +       return retval;
>  }
>
>  int acpi_processor_power_exit(struct acpi_processor *pr)
> --
> 2.33.0
>

Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[lihuisong (C)]

Hi,

Thanks for your review.


在 2025/7/3 1:42, Rafael J. Wysocki 写道:
> On Thu, Jun 19, 2025 at 8:13 AM Huisong Li <lihuisong@huawei.com> wrote:
>> There are two resource rollback issues in acpi_processor_power_init:
>> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
>> 2> Do not free cpuidle device memory when register cpuidle device failed.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
>>   1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 2c2dc559e0f8..3548ab9dac9e 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>>                  }
>>
>>                  dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>> -               if (!dev)
>> -                       return -ENOMEM;
>> +               if (!dev) {
>> +                       retval = -ENOMEM;
>> +                       goto unregister_driver;
> No, unregistering the driver here is pointless.
I don't quite understand why here is pointless. Can you explain it?
>
>> +               }
>>                  per_cpu(acpi_cpuidle_device, pr->id) = dev;
>>
>>                  acpi_processor_setup_cpuidle_dev(pr, dev);
>> @@ -1402,14 +1404,22 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>>                   * must already be registered before registering device
>>                   */
>>                  retval = cpuidle_register_device(dev);
>> -               if (retval) {
>> -                       if (acpi_processor_registered == 0)
>> -                               cpuidle_unregister_driver(&acpi_idle_driver);
> Pretty much the same as here.
>
> It would be good to clean up dev here though.
It is ok if above is pointless.
>
>> -                       return retval;
>> -               }
>> +               if (retval)
>> +                       goto free_cpuidle_device;
>> +
>>                  acpi_processor_registered++;
>>          }
>>          return 0;
>> +
>> +free_cpuidle_device:
>> +       per_cpu(acpi_cpuidle_device, pr->id) = NULL;
>> +       kfree(dev);
>> +
>> +unregister_driver:
>> +       if (acpi_processor_registered == 0)
>> +               cpuidle_unregister_driver(&acpi_idle_driver);
>> +
>> +       return retval;
>>   }
>>
>>   int acpi_processor_power_exit(struct acpi_processor *pr)
>> --
>> 2.33.0
>>

Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[Rafael J. Wysocki]

On Thu, Jul 3, 2025 at 8:23 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>
> Hi,
>
> Thanks for your review.
>
>
> 在 2025/7/3 1:42, Rafael J. Wysocki 写道:
> > On Thu, Jun 19, 2025 at 8:13 AM Huisong Li <lihuisong@huawei.com> wrote:
> >> There are two resource rollback issues in acpi_processor_power_init:
> >> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
> >> 2> Do not free cpuidle device memory when register cpuidle device failed.
> >>
> >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >> ---
> >>   drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
> >>   1 file changed, 17 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> >> index 2c2dc559e0f8..3548ab9dac9e 100644
> >> --- a/drivers/acpi/processor_idle.c
> >> +++ b/drivers/acpi/processor_idle.c
> >> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> >>                  }
> >>
> >>                  dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> >> -               if (!dev)
> >> -                       return -ENOMEM;
> >> +               if (!dev) {
> >> +                       retval = -ENOMEM;
> >> +                       goto unregister_driver;
> > No, unregistering the driver here is pointless.
> I don't quite understand why here is pointless. Can you explain it?

When this function is run for another CPU, it will attempt to register
the driver again if it is unregistered here.

Quite frankly, the driver should be registered before running this
function because it is a CPU hotplug callback and registering a
cpuidle driver from within it is quite questionable.

Alternatively, it can be registered when all of the CPUs have been brought up.

> >
> >> +               }
> >>                  per_cpu(acpi_cpuidle_device, pr->id) = dev;
> >>
> >>                  acpi_processor_setup_cpuidle_dev(pr, dev);
> >> @@ -1402,14 +1404,22 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> >>                   * must already be registered before registering device
> >>                   */
> >>                  retval = cpuidle_register_device(dev);
> >> -               if (retval) {
> >> -                       if (acpi_processor_registered == 0)
> >> -                               cpuidle_unregister_driver(&acpi_idle_driver);
> > Pretty much the same as here.
> >
> > It would be good to clean up dev here though.
> It is ok if above is pointless.
> >
> >> -                       return retval;
> >> -               }
> >> +               if (retval)
> >> +                       goto free_cpuidle_device;
> >> +
> >>                  acpi_processor_registered++;
> >>          }
> >>          return 0;
> >> +
> >> +free_cpuidle_device:
> >> +       per_cpu(acpi_cpuidle_device, pr->id) = NULL;
> >> +       kfree(dev);
> >> +
> >> +unregister_driver:
> >> +       if (acpi_processor_registered == 0)
> >> +               cpuidle_unregister_driver(&acpi_idle_driver);
> >> +
> >> +       return retval;
> >>   }
> >>
> >>   int acpi_processor_power_exit(struct acpi_processor *pr)
> >> --
> >> 2.33.0
> >>

Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[lihuisong (C)]

在 2025/7/3 19:09, Rafael J. Wysocki 写道:
> On Thu, Jul 3, 2025 at 8:23 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>> Hi,
>>
>> Thanks for your review.
>>
>>
>> 在 2025/7/3 1:42, Rafael J. Wysocki 写道:
>>> On Thu, Jun 19, 2025 at 8:13 AM Huisong Li <lihuisong@huawei.com> wrote:
>>>> There are two resource rollback issues in acpi_processor_power_init:
>>>> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
>>>> 2> Do not free cpuidle device memory when register cpuidle device failed.
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> ---
>>>>    drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
>>>>    1 file changed, 17 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>>>> index 2c2dc559e0f8..3548ab9dac9e 100644
>>>> --- a/drivers/acpi/processor_idle.c
>>>> +++ b/drivers/acpi/processor_idle.c
>>>> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>>>>                   }
>>>>
>>>>                   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>>>> -               if (!dev)
>>>> -                       return -ENOMEM;
>>>> +               if (!dev) {
>>>> +                       retval = -ENOMEM;
>>>> +                       goto unregister_driver;
>>> No, unregistering the driver here is pointless.
>> I don't quite understand why here is pointless. Can you explain it?
> When this function is run for another CPU, it will attempt to register
> the driver again if it is unregistered here.
Yeah, got it.
So registering cpuidle also has a potential race issue here.
>
> Quite frankly, the driver should be registered before running this
> function because it is a CPU hotplug callback and registering a
> cpuidle driver from within it is quite questionable.
>
> Alternatively, it can be registered when all of the CPUs have been brought up.
Agree with you.
The reason why is that the initialization of acpi_idle_driver depands on 
the power management information of CPU.
But the power management information of CPU is obtained in this callback.
I have an idea.
Because acpi_idle_driver is applied to all possiable CPUs. And use the 
power information of the first onlined CPU to initialize and register 
acpi_idle_driver, currently.
So I think we can use this logic and dependency to extract a function to 
initialize and register acpi_idle_driver. And put this function to 
acpi_processor_driver_init().
I tested it is ok.
What do you think about this?

/Huisong
>
>>>> +               }
>>>>                   per_cpu(acpi_cpuidle_device, pr->id) = dev;
>>>>
>>>>                   acpi_processor_setup_cpuidle_dev(pr, dev);
>>>> @@ -1402,14 +1404,22 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>>>>                    * must already be registered before registering device
>>>>                    */
>>>>                   retval = cpuidle_register_device(dev);
>>>> -               if (retval) {
>>>> -                       if (acpi_processor_registered == 0)
>>>> -                               cpuidle_unregister_driver(&acpi_idle_driver);
>>> Pretty much the same as here.
>>>
>>> It would be good to clean up dev here though.
>> It is ok if above is pointless.
>>>> -                       return retval;
>>>> -               }
>>>> +               if (retval)
>>>> +                       goto free_cpuidle_device;
>>>> +
>>>>                   acpi_processor_registered++;
>>>>           }
>>>>           return 0;
>>>> +
>>>> +free_cpuidle_device:
>>>> +       per_cpu(acpi_cpuidle_device, pr->id) = NULL;
>>>> +       kfree(dev);
>>>> +
>>>> +unregister_driver:
>>>> +       if (acpi_processor_registered == 0)
>>>> +               cpuidle_unregister_driver(&acpi_idle_driver);
>>>> +
>>>> +       return retval;
>>>>    }
>>>>
>>>>    int acpi_processor_power_exit(struct acpi_processor *pr)
>>>> --
>>>> 2.33.0
>>>>

Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[lihuisong (C)]

Hi Rafael,

Can you take a look at my reply?
If it is ok to you, I will send it later.

/Huisong
在 2025/7/14 9:33, lihuisong (C) 写道:
>
> 在 2025/7/3 19:09, Rafael J. Wysocki 写道:
>> On Thu, Jul 3, 2025 at 8:23 AM lihuisong (C) <lihuisong@huawei.com> 
>> wrote:
>>> Hi,
>>>
>>> Thanks for your review.
>>>
>>>
>>> 在 2025/7/3 1:42, Rafael J. Wysocki 写道:
>>>> On Thu, Jun 19, 2025 at 8:13 AM Huisong Li <lihuisong@huawei.com> 
>>>> wrote:
>>>>> There are two resource rollback issues in acpi_processor_power_init:
>>>>> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
>>>>> 2> Do not free cpuidle device memory when register cpuidle device 
>>>>> failed.
>>>>>
>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>> ---
>>>>>    drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
>>>>>    1 file changed, 17 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/acpi/processor_idle.c 
>>>>> b/drivers/acpi/processor_idle.c
>>>>> index 2c2dc559e0f8..3548ab9dac9e 100644
>>>>> --- a/drivers/acpi/processor_idle.c
>>>>> +++ b/drivers/acpi/processor_idle.c
>>>>> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct 
>>>>> acpi_processor *pr)
>>>>>                   }
>>>>>
>>>>>                   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>>>>> -               if (!dev)
>>>>> -                       return -ENOMEM;
>>>>> +               if (!dev) {
>>>>> +                       retval = -ENOMEM;
>>>>> +                       goto unregister_driver;
>>>> No, unregistering the driver here is pointless.
>>> I don't quite understand why here is pointless. Can you explain it?
>> When this function is run for another CPU, it will attempt to register
>> the driver again if it is unregistered here.
> Yeah, got it.
> So registering cpuidle also has a potential race issue here.
>>
>> Quite frankly, the driver should be registered before running this
>> function because it is a CPU hotplug callback and registering a
>> cpuidle driver from within it is quite questionable.
>>
>> Alternatively, it can be registered when all of the CPUs have been 
>> brought up.
> Agree with you.
> The reason why is that the initialization of acpi_idle_driver depands 
> on the power management information of CPU.
> But the power management information of CPU is obtained in this callback.
> I have an idea.
> Because acpi_idle_driver is applied to all possiable CPUs. And use the 
> power information of the first onlined CPU to initialize and register 
> acpi_idle_driver, currently.
> So I think we can use this logic and dependency to extract a function 
> to initialize and register acpi_idle_driver. And put this function to 
> acpi_processor_driver_init().
> I tested it is ok.
> What do you think about this?
>
> /Huisong
>>
>>>>> +               }
>>>>>                   per_cpu(acpi_cpuidle_device, pr->id) = dev;
>>>>>
>>>>>                   acpi_processor_setup_cpuidle_dev(pr, dev);
>>>>> @@ -1402,14 +1404,22 @@ int acpi_processor_power_init(struct 
>>>>> acpi_processor *pr)
>>>>>                    * must already be registered before registering 
>>>>> device
>>>>>                    */
>>>>>                   retval = cpuidle_register_device(dev);
>>>>> -               if (retval) {
>>>>> -                       if (acpi_processor_registered == 0)
>>>>> - cpuidle_unregister_driver(&acpi_idle_driver);
>>>> Pretty much the same as here.
>>>>
>>>> It would be good to clean up dev here though.
>>> It is ok if above is pointless.
>>>>> -                       return retval;
>>>>> -               }
>>>>> +               if (retval)
>>>>> +                       goto free_cpuidle_device;
>>>>> +
>>>>>                   acpi_processor_registered++;
>>>>>           }
>>>>>           return 0;
>>>>> +
>>>>> +free_cpuidle_device:
>>>>> +       per_cpu(acpi_cpuidle_device, pr->id) = NULL;
>>>>> +       kfree(dev);
>>>>> +
>>>>> +unregister_driver:
>>>>> +       if (acpi_processor_registered == 0)
>>>>> + cpuidle_unregister_driver(&acpi_idle_driver);
>>>>> +
>>>>> +       return retval;
>>>>>    }
>>>>>
>>>>>    int acpi_processor_power_exit(struct acpi_processor *pr)
>>>>> -- 
>>>>> 2.33.0
>>>>>

Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[Rafael J. Wysocki]

On Mon, Jul 14, 2025 at 3:34 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>
>
> 在 2025/7/3 19:09, Rafael J. Wysocki 写道:
> > On Thu, Jul 3, 2025 at 8:23 AM lihuisong (C) <lihuisong@huawei.com> wrote:
> >> Hi,
> >>
> >> Thanks for your review.
> >>
> >>
> >> 在 2025/7/3 1:42, Rafael J. Wysocki 写道:
> >>> On Thu, Jun 19, 2025 at 8:13 AM Huisong Li <lihuisong@huawei.com> wrote:
> >>>> There are two resource rollback issues in acpi_processor_power_init:
> >>>> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
> >>>> 2> Do not free cpuidle device memory when register cpuidle device failed.
> >>>>
> >>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >>>> ---
> >>>>    drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
> >>>>    1 file changed, 17 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> >>>> index 2c2dc559e0f8..3548ab9dac9e 100644
> >>>> --- a/drivers/acpi/processor_idle.c
> >>>> +++ b/drivers/acpi/processor_idle.c
> >>>> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
> >>>>                   }
> >>>>
> >>>>                   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
> >>>> -               if (!dev)
> >>>> -                       return -ENOMEM;
> >>>> +               if (!dev) {
> >>>> +                       retval = -ENOMEM;
> >>>> +                       goto unregister_driver;
> >>> No, unregistering the driver here is pointless.
> >> I don't quite understand why here is pointless. Can you explain it?
> > When this function is run for another CPU, it will attempt to register
> > the driver again if it is unregistered here.
> Yeah, got it.
> So registering cpuidle also has a potential race issue here.
> >
> > Quite frankly, the driver should be registered before running this
> > function because it is a CPU hotplug callback and registering a
> > cpuidle driver from within it is quite questionable.
> >
> > Alternatively, it can be registered when all of the CPUs have been brought up.
> Agree with you.
> The reason why is that the initialization of acpi_idle_driver depands on
> the power management information of CPU.
> But the power management information of CPU is obtained in this callback.
> I have an idea.
> Because acpi_idle_driver is applied to all possiable CPUs. And use the
> power information of the first onlined CPU to initialize and register
> acpi_idle_driver, currently.
> So I think we can use this logic and dependency to extract a function to
> initialize and register acpi_idle_driver. And put this function to
> acpi_processor_driver_init().
> I tested it is ok.
> What do you think about this?

This is one of the options I mentioned above, isn't it?

Re: [PATCH] ACPI: processor: idle: Fix resource rollback in acpi_processor_power_init

[lihuisong (C)]

在 2025/7/22 19:10, Rafael J. Wysocki 写道:
> On Mon, Jul 14, 2025 at 3:34 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>>
>> 在 2025/7/3 19:09, Rafael J. Wysocki 写道:
>>> On Thu, Jul 3, 2025 at 8:23 AM lihuisong (C) <lihuisong@huawei.com> wrote:
>>>> Hi,
>>>>
>>>> Thanks for your review.
>>>>
>>>>
>>>> 在 2025/7/3 1:42, Rafael J. Wysocki 写道:
>>>>> On Thu, Jun 19, 2025 at 8:13 AM Huisong Li <lihuisong@huawei.com> wrote:
>>>>>> There are two resource rollback issues in acpi_processor_power_init:
>>>>>> 1> Do not unregister acpi_idle_driver when do kzalloc failed.
>>>>>> 2> Do not free cpuidle device memory when register cpuidle device failed.
>>>>>>
>>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>>> ---
>>>>>>     drivers/acpi/processor_idle.c | 24 +++++++++++++++++-------
>>>>>>     1 file changed, 17 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>>>>>> index 2c2dc559e0f8..3548ab9dac9e 100644
>>>>>> --- a/drivers/acpi/processor_idle.c
>>>>>> +++ b/drivers/acpi/processor_idle.c
>>>>>> @@ -1392,8 +1392,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>>>>>>                    }
>>>>>>
>>>>>>                    dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>>>>>> -               if (!dev)
>>>>>> -                       return -ENOMEM;
>>>>>> +               if (!dev) {
>>>>>> +                       retval = -ENOMEM;
>>>>>> +                       goto unregister_driver;
>>>>> No, unregistering the driver here is pointless.
>>>> I don't quite understand why here is pointless. Can you explain it?
>>> When this function is run for another CPU, it will attempt to register
>>> the driver again if it is unregistered here.
>> Yeah, got it.
>> So registering cpuidle also has a potential race issue here.
>>> Quite frankly, the driver should be registered before running this
>>> function because it is a CPU hotplug callback and registering a
>>> cpuidle driver from within it is quite questionable.
>>>
>>> Alternatively, it can be registered when all of the CPUs have been brought up.
>> Agree with you.
>> The reason why is that the initialization of acpi_idle_driver depands on
>> the power management information of CPU.
>> But the power management information of CPU is obtained in this callback.
>> I have an idea.
>> Because acpi_idle_driver is applied to all possiable CPUs. And use the
>> power information of the first onlined CPU to initialize and register
>> acpi_idle_driver, currently.
>> So I think we can use this logic and dependency to extract a function to
>> initialize and register acpi_idle_driver. And put this function to
>> acpi_processor_driver_init().
>> I tested it is ok.
>> What do you think about this?
> This is one of the options I mentioned above, isn't it?
Yes, it is what you mentioned. I think this option is relatively easier 
to handle.
I am not sure if I understand correctly.
So I wanted to confirmed with you.