Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

x86, oprofile, nmi: Fix CPU hotplug callback registration

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_notifier_register_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_notifier_register_done();

Fix the oprofile code in x86 by using this latter form of callback
registration. But retain the calls to get/put_online_cpus(), since they are
used in other places as well, to protect the variables 'nmi_enabled' and
'ctr_running'. Strictly speaking, this is not necessary since
cpu_notifier_register_begin/done() provide a stronger synchronization
with CPU hotplug than get/put_online_cpus(). However, let's retain the
calls to get/put_online_cpus() to be consistent with the other call-sites.

By nesting get/put_online_cpus() *inside* cpu_notifier_register_begin/done(),
we avoid the ABBA deadlock possibility mentioned above.

Cc: Robert Richter <rric@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

authored by

Srivatsa S. Bhat and committed by
Rafael J. Wysocki
76902e3d 9f668f66

+13 -2
+13 -2
arch/x86/oprofile/nmi_int.c
··· 494 494 if (err) 495 495 goto fail; 496 496 497 + cpu_notifier_register_begin(); 498 + 499 + /* Use get/put_online_cpus() to protect 'nmi_enabled' */ 497 500 get_online_cpus(); 498 - register_cpu_notifier(&oprofile_cpu_nb); 499 501 nmi_enabled = 1; 500 502 /* make nmi_enabled visible to the nmi handler: */ 501 503 smp_mb(); 502 504 on_each_cpu(nmi_cpu_setup, NULL, 1); 505 + __register_cpu_notifier(&oprofile_cpu_nb); 503 506 put_online_cpus(); 507 + 508 + cpu_notifier_register_done(); 504 509 505 510 return 0; 506 511 fail: ··· 517 512 { 518 513 struct op_msrs *msrs; 519 514 515 + cpu_notifier_register_begin(); 516 + 517 + /* Use get/put_online_cpus() to protect 'nmi_enabled' & 'ctr_running' */ 520 518 get_online_cpus(); 521 - unregister_cpu_notifier(&oprofile_cpu_nb); 522 519 on_each_cpu(nmi_cpu_shutdown, NULL, 1); 523 520 nmi_enabled = 0; 524 521 ctr_running = 0; 522 + __unregister_cpu_notifier(&oprofile_cpu_nb); 525 523 put_online_cpus(); 524 + 525 + cpu_notifier_register_done(); 526 + 526 527 /* make variables visible to the nmi handler: */ 527 528 smp_mb(); 528 529 unregister_nmi_handler(NMI_LOCAL, "oprofile");