Subsystems that want to register CPU hotplug callbacks, as well as perform
initialization for the CPUs that are already online, often do it as shown
below:

        get_online_cpus();

        for_each_online_cpu(cpu)
                init_cpu(cpu);

        register_cpu_notifier(&foobar_cpu_notifier);

        put_online_cpus();

This is wrong, since it is prone to ABBA deadlocks involving the
cpu_add_remove_lock and the cpu_hotplug.lock (when running concurrently
with CPU hotplug operations).

Instead, the correct and race-free way of performing the callback
registration is:

        cpu_maps_update_begin();

        for_each_online_cpu(cpu)
                init_cpu(cpu);

        /* Note the use of the double underscored version of the API */
        __register_cpu_notifier(&foobar_cpu_notifier);

        cpu_maps_update_done();


Fix the intel-rapl code in the powercap driver by using this latter form
of callback registration. But retain the calls to get/put_online_cpus(),
since they also protect the function rapl_cleanup_data(). By nesting
get/put_online_cpus() *inside* cpu_maps_update_begin/done(), we avoid
the ABBA deadlock possibility mentioned above.

Cc: "Rafael J. Wysocki" <rafael.j.wyso...@intel.com>
Cc: Jacob Pan <jacob.jun....@linux.intel.com>
Cc: Srinivas Pandruvada <srinivas.pandruv...@linux.intel.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.b...@linux.vnet.ibm.com>
---

 drivers/powercap/intel_rapl.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index 3c67683..b460d46 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -1369,6 +1369,9 @@ static int __init rapl_init(void)
 
                return -ENODEV;
        }
+
+       cpu_maps_update_begin();
+
        /* prevent CPU hotplug during detection */
        get_online_cpus();
        ret = rapl_detect_topology();
@@ -1380,20 +1383,23 @@ static int __init rapl_init(void)
                ret = -ENODEV;
                goto done;
        }
-       register_hotcpu_notifier(&rapl_cpu_notifier);
+       __register_hotcpu_notifier(&rapl_cpu_notifier);
 done:
        put_online_cpus();
+       cpu_maps_update_done();
 
        return ret;
 }
 
 static void __exit rapl_exit(void)
 {
+       cpu_maps_update_begin();
        get_online_cpus();
-       unregister_hotcpu_notifier(&rapl_cpu_notifier);
+       __unregister_hotcpu_notifier(&rapl_cpu_notifier);
        rapl_unregister_powercap();
        rapl_cleanup_data();
        put_online_cpus();
+       cpu_maps_update_done();
 }
 
 module_init(rapl_init);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to