On Wed, 2021-11-24 at 14:29 +0100, Rafael J. Wysocki wrote:

On Wed, Oct 27, 2021 at 10:07 AM Zhang Rui [off-list ref]
wrote:

quoted

Only limited number of CPUHP_AP_ONLINE_DYN callbacks can be
registered,
thus cpuhp_remove_state() should be invoked to release the resource
when
it is not used.

Fixes: fb1013a01673 ("intel_idle: Convert to hotplug state
machine")
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/idle/intel_idle.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index ae9d8c43e6a5..e7f2a5f85bf9 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c

@@ -1676,6 +1676,8 @@ static int intel_idle_cpu_online(unsigned int

cpu)
        return 0;
 }

+static enum cpuhp_state intel_idle_cpuhp_state;
+
 /**
  * intel_idle_cpuidle_unregister - unregister from cpuidle
framework
  */

@@ -1683,6 +1685,8 @@ static void __init

intel_idle_cpuidle_unregister(struct cpuidle_driver *drv)
 {
        int i;

+       if (intel_idle_cpuhp_state > 0)
+               cpuhp_remove_state(intel_idle_cpuhp_state);

It would be more straightforward to do that directly in
intel_idle_init(), because intel_idle_cpuhp_state could be a local
variable in that function then.

I rechecked the code.
in intel_idle_init(), cpuhp_setup_state() is the last step, so either
it succeeds and we never unload the driver, or it fails and the
intel_idle_init() fails directly.
So nothing needs to be fixed if we don't support intel_idle driver
module unload.

thanks,
rui

quoted

        for_each_online_cpu(i)
                cpuidle_unregister_device(per_cpu_ptr(intel_idle_cp
uidle_devices, i));
        cpuidle_unregister_driver(drv);

@@ -1710,11 +1714,11 @@ static int __init

intel_idle_cpuidle_register(struct cpuidle_driver *drv)
                return retval;
        }

-       retval = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
"idle/intel:online",
-                               intel_idle_cpu_online, NULL);
-       if (retval < 0) {
+       intel_idle_cpuhp_state =
cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+               "idle/intel:online", intel_idle_cpu_online, NULL);
+       if (intel_idle_cpuhp_state < 0) {
                intel_idle_cpuidle_unregister(drv);
-               return retval;
+               return intel_idle_cpuhp_state;
        }
        return 0;
 }
--
2.17.1