[PATCH v2] ACPI: processor: idle: Fix resource leak and potential concurrent in acpi_processor_power_init()

[PATCH v2] ACPI: processor: idle: Fix resource leak and potential concurrent in acpi_processor_power_init()

[Huisong Li]

There are three kind of issues:
1> There are two resource leak issues in acpi_processor_power_init:
   a> Don't unregister acpi_idle_driver when do kzalloc failed.
   b> Don't free cpuidle device memory when register cpuidle device failed.
2> There isn't lock to prevent the global acpi_processor_registered, which
   may lead to concurrent register cpuidle driver.
3> The cpuidle driver should be registered in advance when all of the CPUs
   have been brought up instead of being in a CPU hotplug callback.

To solve these issues, so add a new function to initialize acpi_idle_driver
based on the power management information of an available CPU and register
cpuidle driver in acpi_processor_driver_init().

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 v2: register cpuidle driver in advance when all of the CPUs have been
     brought up.
     v1 link: https://patchwork.kernel.org/project/linux-acpi/patch/20250619061327.1674384-1-lihuisong@huawei.com/
---
 drivers/acpi/processor_driver.c |  5 +++
 drivers/acpi/processor_idle.c   | 71 ++++++++++++++++++++++-----------
 include/acpi/processor.h        |  9 +++++
 3 files changed, 62 insertions(+), 23 deletions(-)

diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index 65e779be64ff..ff944c93b6ff 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -263,6 +263,10 @@ static int __init acpi_processor_driver_init(void)
 	if (result < 0)
 		return result;
 
+	result = acpi_processor_register_idle_driver();
+	if (result)
+		pr_info("register idle driver failed, ret = %d.\n", result);
+
 	result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
 				   "acpi/cpu-drv:online",
 				   acpi_soft_cpu_online, NULL);
@@ -301,6 +305,7 @@ static void __exit acpi_processor_driver_exit(void)
 
 	cpuhp_remove_state_nocalls(hp_online);
 	cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
+	acpi_processor_unregister_idle_driver();
 	driver_unregister(&acpi_processor_driver);
 }
 
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
index 2c2dc559e0f8..2408f1076631 100644
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -1360,7 +1360,52 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
 	return 0;
 }
 
-static int acpi_processor_registered;
+int acpi_processor_register_idle_driver(void)
+{
+	struct acpi_processor *pr;
+	int cpu;
+	int ret;
+
+	/*
+	 * Acpi idle driver is used by all possible CPUs.
+	 * Install the idle handler by the processor power info of one in them.
+	 * Note that we use previously set idle handler will be used on
+	 * platforms that only support C1.
+	 */
+	for_each_cpu(cpu, (struct cpumask *)cpu_possible_mask) {
+		pr = per_cpu(processors, cpu);
+		if (pr == NULL)
+			continue;
+
+		ret = acpi_processor_get_power_info(pr);
+		if (!ret) {
+			pr->flags.power_setup_done = 1;
+			break;
+		}
+	}
+
+	if (unlikely(!pr))
+		return -ENODEV;
+
+	if (ret) {
+		pr_err("%s get power information failed.\n",
+		       acpi_idle_driver.name);
+		return ret;
+	}
+
+	acpi_processor_setup_cpuidle_states(pr);
+	ret = cpuidle_register_driver(&acpi_idle_driver);
+	if (ret)
+		return ret;
+
+	pr_debug("%s registered with cpuidle\n", acpi_idle_driver.name);
+	return 0;
+}
+
+void acpi_processor_unregister_idle_driver(void)
+{
+	cpuidle_unregister_driver(&acpi_idle_driver);
+}
 
 int acpi_processor_power_init(struct acpi_processor *pr)
 {
@@ -1375,22 +1420,7 @@ int acpi_processor_power_init(struct acpi_processor *pr)
 	if (!acpi_processor_get_power_info(pr))
 		pr->flags.power_setup_done = 1;
 
-	/*
-	 * Install the idle handler if processor power management is supported.
-	 * Note that we use previously set idle handler will be used on
-	 * platforms that only support C1.
-	 */
 	if (pr->flags.power) {
-		/* Register acpi_idle_driver if not already registered */
-		if (!acpi_processor_registered) {
-			acpi_processor_setup_cpuidle_states(pr);
-			retval = cpuidle_register_driver(&acpi_idle_driver);
-			if (retval)
-				return retval;
-			pr_debug("%s registered with cpuidle\n",
-				 acpi_idle_driver.name);
-		}
-
 		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
 		if (!dev)
 			return -ENOMEM;
@@ -1403,11 +1433,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
 		 */
 		retval = cpuidle_register_device(dev);
 		if (retval) {
-			if (acpi_processor_registered == 0)
-				cpuidle_unregister_driver(&acpi_idle_driver);
+			per_cpu(acpi_cpuidle_device, pr->id) = NULL;
+			kfree(dev);
 			return retval;
 		}
-		acpi_processor_registered++;
 	}
 	return 0;
 }
@@ -1421,10 +1450,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr)
 
 	if (pr->flags.power) {
 		cpuidle_unregister_device(dev);
-		acpi_processor_registered--;
-		if (acpi_processor_registered == 0)
-			cpuidle_unregister_driver(&acpi_idle_driver);
-
 		kfree(dev);
 	}
 
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index d0eccbd920e5..3cb41a3f2d9a 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -423,6 +423,8 @@ int acpi_processor_power_init(struct acpi_processor *pr);
 int acpi_processor_power_exit(struct acpi_processor *pr);
 int acpi_processor_power_state_has_changed(struct acpi_processor *pr);
 int acpi_processor_hotplug(struct acpi_processor *pr);
+int acpi_processor_register_idle_driver(void);
+void acpi_processor_unregister_idle_driver(void);
 #else
 static inline int acpi_processor_power_init(struct acpi_processor *pr)
 {
@@ -443,6 +445,13 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
 {
 	return -ENODEV;
 }
+static int acpi_processor_register_idle_driver(void)
+{
+	return -ENODEV;
+}
+static void acpi_processor_unregister_idle_driver(void)
+{
+}
 #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
 
 /* in processor_thermal.c */
-- 
2.33.0

Re: [PATCH v2] ACPI: processor: idle: Fix resource leak and potential concurrent in acpi_processor_power_init()

[Rafael J. Wysocki]

On Wed, Jul 23, 2025 at 2:10 PM Huisong Li <lihuisong@huawei.com> wrote:
>
> There are three kind of issues:
> 1> There are two resource leak issues in acpi_processor_power_init:
>    a> Don't unregister acpi_idle_driver when do kzalloc failed.

This is not a resource leak.  As I said before, the driver need not be
unregistered on a memory allocation failure.

>    b> Don't free cpuidle device memory when register cpuidle device failed.

This is a separate minor issue that needs to be addressed by a separate patch.

> 2> There isn't lock to prevent the global acpi_processor_registered, which
>    may lead to concurrent register cpuidle driver.

That's not obvious because in principle the code in question is only
run during initialization which is serialized.

In theory, it could run in parallel CPU online, but that at least is
not default behavior AFAICS.

In any case, if you claim something like this, it is advisable to
mention a specific scenario in which the race in question can happen.

> 3> The cpuidle driver should be registered in advance when all of the CPUs
>    have been brought up instead of being in a CPU hotplug callback.

The "in advance" piece above is rather confusing and it can be dropped
without changing the meaning of the rest of the sentence.

> To solve these issues, so add a new function to initialize acpi_idle_driver
> based on the power management information of an available CPU and register
> cpuidle driver in acpi_processor_driver_init().

I think that the main problem here is that the cpuidle driver is
registered from within a CPU hotplug callback, which is questionable
and confusing.  Usually, however, this doesn't lead to any functional
issues AFAICS.

> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  v2: register cpuidle driver in advance when all of the CPUs have been
>      brought up.
>      v1 link: https://patchwork.kernel.org/project/linux-acpi/patch/20250619061327.1674384-1-lihuisong@huawei.com/
> ---
>  drivers/acpi/processor_driver.c |  5 +++
>  drivers/acpi/processor_idle.c   | 71 ++++++++++++++++++++++-----------
>  include/acpi/processor.h        |  9 +++++
>  3 files changed, 62 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index 65e779be64ff..ff944c93b6ff 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -263,6 +263,10 @@ static int __init acpi_processor_driver_init(void)
>         if (result < 0)
>                 return result;
>
> +       result = acpi_processor_register_idle_driver();
> +       if (result)
> +               pr_info("register idle driver failed, ret = %d.\n", result);

This registers the cpuidle driver before registering cpuidle devices
for all CPUs.

It would be better to make acpi_processor_register_idle_driver() print
the diagnostic message on failures and then it won't need to return a
value.

Note that it may fail if intel_idle is already registered, for
example, so the message should rather be a debug-level one.

> +
>         result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
>                                    "acpi/cpu-drv:online",
>                                    acpi_soft_cpu_online, NULL);
> @@ -301,6 +305,7 @@ static void __exit acpi_processor_driver_exit(void)
>
>         cpuhp_remove_state_nocalls(hp_online);
>         cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
> +       acpi_processor_unregister_idle_driver();
>         driver_unregister(&acpi_processor_driver);
>  }
>
> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
> index 2c2dc559e0f8..2408f1076631 100644
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -1360,7 +1360,52 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
>         return 0;
>  }
>
> -static int acpi_processor_registered;
> +int acpi_processor_register_idle_driver(void)
> +{
> +       struct acpi_processor *pr;
> +       int cpu;
> +       int ret;

The ret variable needs to be initialized here or tools will complain,
and so it may be initialized to -ENODEV:

int ret = -ENODEV;

> +
> +       /*
> +        * Acpi idle driver is used by all possible CPUs.
> +        * Install the idle handler by the processor power info of one in them.
> +        * Note that we use previously set idle handler will be used on
> +        * platforms that only support C1.
> +        */
> +       for_each_cpu(cpu, (struct cpumask *)cpu_possible_mask) {
> +               pr = per_cpu(processors, cpu);
> +               if (pr == NULL)

"if (!pr)" please.

> +                       continue;
> +
> +               ret = acpi_processor_get_power_info(pr);

if (ret)
        continue;

> +               if (!ret) {
> +                       pr->flags.power_setup_done = 1;

I think this is set here to prevent the subsequent
acpi_processor_setup_cpuidle_states() call from bailing out, but is
this not too early to set it?

> +                       break;
> +               }
> +       }
> +
> +       if (unlikely(!pr))
> +               return -ENODEV;

This is unnecessary if ret is initialized to -ENODEV;

> +
> +       if (ret) {
> +               pr_err("%s get power information failed.\n",
> +                      acpi_idle_driver.name);

This message is confusing at best.  It should be something like "No
ACPI power information for any CPUs" and the driver name in it has no
purpose.

> +               return ret;
> +       }
> +
> +       acpi_processor_setup_cpuidle_states(pr);

I'd call this in the loop right before breaking out of it, so the
scope of pr is clear.

> +       ret = cpuidle_register_driver(&acpi_idle_driver);
> +       if (ret)

Print a diagnostic message here and do not return a value (ie. make
the function void).

> +               return ret;
> +
> +       pr_debug("%s registered with cpuidle\n", acpi_idle_driver.name);
> +       return 0;
> +}
> +
> +void acpi_processor_unregister_idle_driver(void)
> +{
> +       cpuidle_unregister_driver(&acpi_idle_driver);
> +}
>
>  int acpi_processor_power_init(struct acpi_processor *pr)
>  {
> @@ -1375,22 +1420,7 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>         if (!acpi_processor_get_power_info(pr))
>                 pr->flags.power_setup_done = 1;
>
> -       /*
> -        * Install the idle handler if processor power management is supported.
> -        * Note that we use previously set idle handler will be used on
> -        * platforms that only support C1.
> -        */
>         if (pr->flags.power) {
> -               /* Register acpi_idle_driver if not already registered */
> -               if (!acpi_processor_registered) {
> -                       acpi_processor_setup_cpuidle_states(pr);
> -                       retval = cpuidle_register_driver(&acpi_idle_driver);
> -                       if (retval)
> -                               return retval;
> -                       pr_debug("%s registered with cpuidle\n",
> -                                acpi_idle_driver.name);
> -               }
> -
>                 dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>                 if (!dev)
>                         return -ENOMEM;
> @@ -1403,11 +1433,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>                  */
>                 retval = cpuidle_register_device(dev);
>                 if (retval) {
> -                       if (acpi_processor_registered == 0)
> -                               cpuidle_unregister_driver(&acpi_idle_driver);
> +                       per_cpu(acpi_cpuidle_device, pr->id) = NULL;
> +                       kfree(dev);

These two lines should be added in a separate patch.


>                         return retval;
>                 }
> -               acpi_processor_registered++;
>         }
>         return 0;
>  }
> @@ -1421,10 +1450,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr)
>
>         if (pr->flags.power) {
>                 cpuidle_unregister_device(dev);
> -               acpi_processor_registered--;
> -               if (acpi_processor_registered == 0)
> -                       cpuidle_unregister_driver(&acpi_idle_driver);
> -
>                 kfree(dev);
>         }
>
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index d0eccbd920e5..3cb41a3f2d9a 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -423,6 +423,8 @@ int acpi_processor_power_init(struct acpi_processor *pr);
>  int acpi_processor_power_exit(struct acpi_processor *pr);
>  int acpi_processor_power_state_has_changed(struct acpi_processor *pr);
>  int acpi_processor_hotplug(struct acpi_processor *pr);
> +int acpi_processor_register_idle_driver(void);
> +void acpi_processor_unregister_idle_driver(void);
>  #else
>  static inline int acpi_processor_power_init(struct acpi_processor *pr)
>  {
> @@ -443,6 +445,13 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
>  {
>         return -ENODEV;
>  }
> +static int acpi_processor_register_idle_driver(void)
> +{
> +       return -ENODEV;
> +}
> +static void acpi_processor_unregister_idle_driver(void)
> +{
> +}
>  #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
>
>  /* in processor_thermal.c */
> --
> 2.33.0
>

Re: [PATCH v2] ACPI: processor: idle: Fix resource leak and potential concurrent in acpi_processor_power_init()

[lihuisong (C)]

在 2025/7/23 21:35, Rafael J. Wysocki 写道:
> On Wed, Jul 23, 2025 at 2:10 PM Huisong Li <lihuisong@huawei.com> wrote:
>> There are three kind of issues:
>> 1> There are two resource leak issues in acpi_processor_power_init:
>>     a> Don't unregister acpi_idle_driver when do kzalloc failed.
> This is not a resource leak.  As I said before, the driver need not be
> unregistered on a memory allocation failure.
Ok, will remove it.
>
>>     b> Don't free cpuidle device memory when register cpuidle device failed.
> This is a separate minor issue that needs to be addressed by a separate patch.
Got it.  will add a separate patch in next version.
>
>> 2> There isn't lock to prevent the global acpi_processor_registered, which
>>     may lead to concurrent register cpuidle driver.
> That's not obvious because in principle the code in question is only
> run during initialization which is serialized.
>
> In theory, it could run in parallel CPU online, but that at least is
> not default behavior AFAICS.
Ack.
>
> In any case, if you claim something like this, it is advisable to
> mention a specific scenario in which the race in question can happen.

Yeah.

Anyway, it is ok if the initialization is serialized as you mentioned.


>
>> 3> The cpuidle driver should be registered in advance when all of the CPUs
>>     have been brought up instead of being in a CPU hotplug callback.
> The "in advance" piece above is rather confusing and it can be dropped
> without changing the meaning of the rest of the sentence.
Ack
>
>> To solve these issues, so add a new function to initialize acpi_idle_driver
>> based on the power management information of an available CPU and register
>> cpuidle driver in acpi_processor_driver_init().
> I think that the main problem here is that the cpuidle driver is
> registered from within a CPU hotplug callback, which is questionable
> and confusing.  Usually, however, this doesn't lead to any functional
> issues AFAICS.
Ok, this patch will use this tone in the commit log.
>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   v2: register cpuidle driver in advance when all of the CPUs have been
>>       brought up.
>>       v1 link: https://patchwork.kernel.org/project/linux-acpi/patch/20250619061327.1674384-1-lihuisong@huawei.com/
>> ---
>>   drivers/acpi/processor_driver.c |  5 +++
>>   drivers/acpi/processor_idle.c   | 71 ++++++++++++++++++++++-----------
>>   include/acpi/processor.h        |  9 +++++
>>   3 files changed, 62 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
>> index 65e779be64ff..ff944c93b6ff 100644
>> --- a/drivers/acpi/processor_driver.c
>> +++ b/drivers/acpi/processor_driver.c
>> @@ -263,6 +263,10 @@ static int __init acpi_processor_driver_init(void)
>>          if (result < 0)
>>                  return result;
>>
>> +       result = acpi_processor_register_idle_driver();
>> +       if (result)
>> +               pr_info("register idle driver failed, ret = %d.\n", result);
> This registers the cpuidle driver before registering cpuidle devices
> for all CPUs.
>
> It would be better to make acpi_processor_register_idle_driver() print
> the diagnostic message on failures and then it won't need to return a
> value.
Ack
>
> Note that it may fail if intel_idle is already registered, for
> example, so the message should rather be a debug-level one.
All right.
>> +
>>          result = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
>>                                     "acpi/cpu-drv:online",
>>                                     acpi_soft_cpu_online, NULL);
>> @@ -301,6 +305,7 @@ static void __exit acpi_processor_driver_exit(void)
>>
>>          cpuhp_remove_state_nocalls(hp_online);
>>          cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
>> +       acpi_processor_unregister_idle_driver();
>>          driver_unregister(&acpi_processor_driver);
>>   }
>>
>> diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c
>> index 2c2dc559e0f8..2408f1076631 100644
>> --- a/drivers/acpi/processor_idle.c
>> +++ b/drivers/acpi/processor_idle.c
>> @@ -1360,7 +1360,52 @@ int acpi_processor_power_state_has_changed(struct acpi_processor *pr)
>>          return 0;
>>   }
>>
>> -static int acpi_processor_registered;
>> +int acpi_processor_register_idle_driver(void)
>> +{
>> +       struct acpi_processor *pr;
>> +       int cpu;
>> +       int ret;
> The ret variable needs to be initialized here or tools will complain,
> and so it may be initialized to -ENODEV:
>
> int ret = -ENODEV;
Ack
>
>> +
>> +       /*
>> +        * Acpi idle driver is used by all possible CPUs.
>> +        * Install the idle handler by the processor power info of one in them.
>> +        * Note that we use previously set idle handler will be used on
>> +        * platforms that only support C1.
>> +        */
>> +       for_each_cpu(cpu, (struct cpumask *)cpu_possible_mask) {
>> +               pr = per_cpu(processors, cpu);
>> +               if (pr == NULL)
> "if (!pr)" please.
Ack
>
>> +                       continue;
>> +
>> +               ret = acpi_processor_get_power_info(pr);
> if (ret)
>          continue;
Please see the following reply.
>
>> +               if (!ret) {
>> +                       pr->flags.power_setup_done = 1;
> I think this is set here to prevent the subsequent
> acpi_processor_setup_cpuidle_states() call from bailing out, but is
> this not too early to set it?
Setting to 1 is necessary here,
otherwise acpi_processor_setup_cpuidle_states would not initialize 
acpi_idle_driver because this flag is false.
>
>> +                       break;
>> +               }
>> +       }
>> +
>> +       if (unlikely(!pr))
>> +               return -ENODEV;
> This is unnecessary if ret is initialized to -ENODEV;
ok
>
>> +
>> +       if (ret) {
>> +               pr_err("%s get power information failed.\n",
>> +                      acpi_idle_driver.name);
> This message is confusing at best.  It should be something like "No
> ACPI power information for any CPUs" and the driver name in it has no
> purpose.
Ack
>
>> +               return ret;
>> +       }
>> +
>> +       acpi_processor_setup_cpuidle_states(pr);
> I'd call this in the loop right before breaking out of it, so the
> scope of pr is clear.
will fix it as you said.
>
>> +       ret = cpuidle_register_driver(&acpi_idle_driver);
>> +       if (ret)
> Print a diagnostic message here and do not return a value (ie. make
> the function void).
All right.
>
>> +               return ret;
>> +
>> +       pr_debug("%s registered with cpuidle\n", acpi_idle_driver.name);
>> +       return 0;
>> +}
>> +
>> +void acpi_processor_unregister_idle_driver(void)
>> +{
>> +       cpuidle_unregister_driver(&acpi_idle_driver);
>> +}
>>
>>   int acpi_processor_power_init(struct acpi_processor *pr)
>>   {
>> @@ -1375,22 +1420,7 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>>          if (!acpi_processor_get_power_info(pr))
>>                  pr->flags.power_setup_done = 1;
>>
>> -       /*
>> -        * Install the idle handler if processor power management is supported.
>> -        * Note that we use previously set idle handler will be used on
>> -        * platforms that only support C1.
>> -        */
>>          if (pr->flags.power) {
>> -               /* Register acpi_idle_driver if not already registered */
>> -               if (!acpi_processor_registered) {
>> -                       acpi_processor_setup_cpuidle_states(pr);
>> -                       retval = cpuidle_register_driver(&acpi_idle_driver);
>> -                       if (retval)
>> -                               return retval;
>> -                       pr_debug("%s registered with cpuidle\n",
>> -                                acpi_idle_driver.name);
>> -               }
>> -
>>                  dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>>                  if (!dev)
>>                          return -ENOMEM;
>> @@ -1403,11 +1433,10 @@ int acpi_processor_power_init(struct acpi_processor *pr)
>>                   */
>>                  retval = cpuidle_register_device(dev);
>>                  if (retval) {
>> -                       if (acpi_processor_registered == 0)
>> -                               cpuidle_unregister_driver(&acpi_idle_driver);
>> +                       per_cpu(acpi_cpuidle_device, pr->id) = NULL;
>> +                       kfree(dev);
> These two lines should be added in a separate patch.
Ack.
>
>
>>                          return retval;
>>                  }
>> -               acpi_processor_registered++;
>>          }
>>          return 0;
>>   }
>> @@ -1421,10 +1450,6 @@ int acpi_processor_power_exit(struct acpi_processor *pr)
>>
>>          if (pr->flags.power) {
>>                  cpuidle_unregister_device(dev);
>> -               acpi_processor_registered--;
>> -               if (acpi_processor_registered == 0)
>> -                       cpuidle_unregister_driver(&acpi_idle_driver);
>> -
>>                  kfree(dev);
>>          }
>>
>> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
>> index d0eccbd920e5..3cb41a3f2d9a 100644
>> --- a/include/acpi/processor.h
>> +++ b/include/acpi/processor.h
>> @@ -423,6 +423,8 @@ int acpi_processor_power_init(struct acpi_processor *pr);
>>   int acpi_processor_power_exit(struct acpi_processor *pr);
>>   int acpi_processor_power_state_has_changed(struct acpi_processor *pr);
>>   int acpi_processor_hotplug(struct acpi_processor *pr);
>> +int acpi_processor_register_idle_driver(void);
>> +void acpi_processor_unregister_idle_driver(void);
>>   #else
>>   static inline int acpi_processor_power_init(struct acpi_processor *pr)
>>   {
>> @@ -443,6 +445,13 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
>>   {
>>          return -ENODEV;
>>   }
>> +static int acpi_processor_register_idle_driver(void)
>> +{
>> +       return -ENODEV;
>> +}
>> +static void acpi_processor_unregister_idle_driver(void)
>> +{
>> +}
>>   #endif /* CONFIG_ACPI_PROCESSOR_IDLE */
>>
>>   /* in processor_thermal.c */
>> --
>> 2.33.0
>>

Re: [PATCH v2] ACPI: processor: idle: Fix resource leak and potential concurrent in acpi_processor_power_init()

[kernel test robot]

Hi Huisong,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.16-rc7 next-20250723]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Huisong-Li/ACPI-processor-idle-Fix-resource-leak-and-potential-concurrent-in-acpi_processor_power_init/20250723-201246
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20250723121034.3685996-1-lihuisong%40huawei.com
patch subject: [PATCH v2] ACPI: processor: idle: Fix resource leak and potential concurrent in acpi_processor_power_init()
config: i386-buildonly-randconfig-001-20250724 (https://download.01.org/0day-ci/archive/20250724/202507240807.1ild86sv-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250724/202507240807.1ild86sv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507240807.1ild86sv-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from drivers/acpi/processor_core.c:15:
>> include/acpi/processor.h:452:13: warning: 'acpi_processor_unregister_idle_driver' defined but not used [-Wunused-function]
     452 | static void acpi_processor_unregister_idle_driver(void)
         |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> include/acpi/processor.h:448:12: warning: 'acpi_processor_register_idle_driver' defined but not used [-Wunused-function]
     448 | static int acpi_processor_register_idle_driver(void)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/acpi_processor_unregister_idle_driver +452 include/acpi/processor.h

   443	
   444	static inline int acpi_processor_hotplug(struct acpi_processor *pr)
   445	{
   446		return -ENODEV;
   447	}
 > 448	static int acpi_processor_register_idle_driver(void)
   449	{
   450		return -ENODEV;
   451	}
 > 452	static void acpi_processor_unregister_idle_driver(void)
   453	{
   454	}
   455	#endif /* CONFIG_ACPI_PROCESSOR_IDLE */
   456	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki