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@xxxxxxxxxx> 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@xxxxxxxxxx> --- 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
