linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>
To: paulus@samba.org, oleg@redhat.com, mingo@kernel.org,
	rjw@rjwysocki.net, rusty@rustcorp.com.au, peterz@infradead.org,
	tglx@linutronix.de, akpm@linux-foundation.org
Cc: linux-arch@vger.kernel.org, ego@linux.vnet.ibm.com,
	walken@google.com, linux@arm.linux.org.uk,
	linux-pm@vger.kernel.org,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	David Vrabel <david.vrabel@citrix.com>,
	"Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com>,
	tj@kernel.org, xen-devel@lists.xenproject.org,
	paulmck@linux.vnet.ibm.com, Ingo Molnar <mingo@kernel.org>
Subject: [PATCH v3 46/52] xen, balloon: Fix CPU hotplug callback registration
Date: Tue, 11 Mar 2014 02:11:45 +0530	[thread overview]
Message-ID: <20140310204144.10746.62444.stgit@srivatsabhat.in.ibm.com> (raw)
In-Reply-To: <20140310203312.10746.310.stgit@srivatsabhat.in.ibm.com>

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).

The xen balloon driver doesn't take get/put_online_cpus() around this code,
but that is also buggy, since it can miss CPU hotplug events in between the
initialization and callback registration:

	for_each_online_cpu(cpu)
		init_cpu(cpu);
		   ^
		   |  Race window; Can miss CPU hotplug events here.
		   v
	register_cpu_notifier(&foobar_cpu_notifier);

Interestingly, the balloon code in xen can simply be reorganized as shown
below, to have a race-free method to register hotplug callbacks, without even
taking get/put_online_cpus(). This is because the initialization performed for
already online CPUs is exactly the same as that performed for CPUs that come
online later. Moreover, the code has checks in place to avoid double
initialization.

	register_cpu_notifier(&foobar_cpu_notifier);

	get_online_cpus();

	for_each_online_cpu(cpu)
		init_cpu(cpu);

	put_online_cpus();

A hotplug operation that occurs between registering the notifier and calling
get_online_cpus(), won't disrupt anything, because the code takes care to
perform the memory allocations only once.

So reorganize the balloon code in xen this way to fix the issues with CPU
hotplug callback registration.

Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
---

 drivers/xen/balloon.c |   36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 37d06ea..dd79549 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -592,19 +592,29 @@ static void __init balloon_add_region(unsigned long start_pfn,
 	}
 }
 
+static int alloc_balloon_scratch_page(int cpu)
+{
+	if (per_cpu(balloon_scratch_page, cpu) != NULL)
+		return 0;
+
+	per_cpu(balloon_scratch_page, cpu) = alloc_page(GFP_KERNEL);
+	if (per_cpu(balloon_scratch_page, cpu) == NULL) {
+		pr_warn("Failed to allocate balloon_scratch_page for cpu %d\n", cpu);
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+
 static int balloon_cpu_notify(struct notifier_block *self,
 				    unsigned long action, void *hcpu)
 {
 	int cpu = (long)hcpu;
 	switch (action) {
 	case CPU_UP_PREPARE:
-		if (per_cpu(balloon_scratch_page, cpu) != NULL)
-			break;
-		per_cpu(balloon_scratch_page, cpu) = alloc_page(GFP_KERNEL);
-		if (per_cpu(balloon_scratch_page, cpu) == NULL) {
-			pr_warn("Failed to allocate balloon_scratch_page for cpu %d\n", cpu);
+		if (alloc_balloon_scratch_page(cpu))
 			return NOTIFY_BAD;
-		}
 		break;
 	default:
 		break;
@@ -624,15 +634,17 @@ static int __init balloon_init(void)
 		return -ENODEV;
 
 	if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-		for_each_online_cpu(cpu)
-		{
-			per_cpu(balloon_scratch_page, cpu) = alloc_page(GFP_KERNEL);
-			if (per_cpu(balloon_scratch_page, cpu) == NULL) {
-				pr_warn("Failed to allocate balloon_scratch_page for cpu %d\n", cpu);
+		register_cpu_notifier(&balloon_cpu_notifier);
+
+		get_online_cpus();
+		for_each_online_cpu(cpu) {
+			if (alloc_balloon_scratch_page(cpu)) {
+				put_online_cpus();
+				unregister_cpu_notifier(&balloon_cpu_notifier);
 				return -ENOMEM;
 			}
 		}
-		register_cpu_notifier(&balloon_cpu_notifier);
+		put_online_cpus();
 	}
 
 	pr_info("Initialising balloon driver\n");

  parent reply	other threads:[~2014-03-10 20:42 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-10 20:33 [PATCH v3 00/52] CPU hotplug: Fix issues with callback registration Srivatsa S. Bhat
2014-03-10 20:34 ` [PATCH v3 01/52] CPU hotplug: Add lockdep annotations to get/put_online_cpus() Srivatsa S. Bhat
2014-03-10 20:34 ` [PATCH v3 02/52] CPU hotplug: Provide lockless versions of callback registration functions Srivatsa S. Bhat
2014-03-10 20:34 ` [PATCH v3 03/52] Doc/cpu-hotplug: Specify race-free way to register CPU hotplug callbacks Srivatsa S. Bhat
2014-03-10 20:34 ` [PATCH v3 04/52] CPU hotplug, perf: Fix CPU hotplug callback registration Srivatsa S. Bhat
2014-03-10 20:34 ` [PATCH v3 05/52] ia64, salinfo: Fix " Srivatsa S. Bhat
2014-03-10 20:34 ` [PATCH v3 06/52] ia64, palinfo: Fix CPU " Srivatsa S. Bhat
2014-03-10 20:34 ` [PATCH v3 07/52] ia64, topology: " Srivatsa S. Bhat
2014-03-10 20:35 ` [PATCH v3 08/52] ia64, err-inject: " Srivatsa S. Bhat
2014-03-10 20:35 ` [PATCH v3 09/52] arm, hw-breakpoint: " Srivatsa S. Bhat
2014-03-10 20:35 ` [PATCH v3 10/52] arm, kvm: " Srivatsa S. Bhat
2014-03-12 23:21   ` Christoffer Dall
2014-03-14  5:43     ` Srivatsa S. Bhat
2014-03-14 19:10       ` Christoffer Dall
2014-03-18 10:23         ` [UPDATED PATCH " Srivatsa S. Bhat
2014-03-18 22:08           ` Christoffer Dall
2014-03-10 20:35 ` [PATCH v3 11/52] s390, cacheinfo: " Srivatsa S. Bhat
2014-03-10 20:35 ` [PATCH v3 12/52] s390, smp: " Srivatsa S. Bhat
2014-03-10 20:36 ` [PATCH v3 13/52] sparc, sysfs: " Srivatsa S. Bhat
2014-03-10 20:36 ` [PATCH v3 14/52] powerpc, " Srivatsa S. Bhat
2014-03-10 20:36 ` [PATCH v3 15/52] x86, msr: " Srivatsa S. Bhat
2014-03-10 20:36 ` [PATCH v3 16/52] x86, cpuid: " Srivatsa S. Bhat
2014-03-10 20:36 ` [PATCH v3 17/52] x86, vsyscall: " Srivatsa S. Bhat
2014-03-10 20:36 ` [PATCH v3 18/52] x86, intel, uncore: " Srivatsa S. Bhat
2014-03-10 20:37 ` [PATCH v3 19/52] x86, mce: " Srivatsa S. Bhat
2014-03-10 20:37 ` [PATCH v3 20/52] x86, therm_throt.c: " Srivatsa S. Bhat
2014-03-10 20:37 ` [PATCH v3 21/52] x86, therm_throt.c: Remove unused therm_cpu_lock Srivatsa S. Bhat
2014-03-10 20:37 ` [PATCH v3 22/52] x86, amd, ibs: Fix CPU hotplug callback registration Srivatsa S. Bhat
2014-03-10 20:37 ` [PATCH v3 23/52] x86, intel, cacheinfo: " Srivatsa S. Bhat
2014-03-10 20:38 ` [PATCH v3 24/52] x86, intel, rapl: " Srivatsa S. Bhat
2014-03-10 20:38 ` [PATCH v3 25/52] x86, amd, uncore: " Srivatsa S. Bhat
2014-03-10 20:38 ` [PATCH v3 26/52] x86, hpet: " Srivatsa S. Bhat
2014-03-10 20:38 ` [PATCH v3 27/52] x86, pci, amd-bus: " Srivatsa S. Bhat
2014-03-10 20:38 ` [PATCH v3 28/52] x86, oprofile, nmi: " Srivatsa S. Bhat
2014-03-10 20:39 ` [PATCH v3 29/52] x86, kvm: " Srivatsa S. Bhat
2014-03-10 20:39 ` [PATCH v3 30/52] arm64, hw_breakpoint.c: " Srivatsa S. Bhat
2014-03-10 20:39 ` [PATCH v3 31/52] arm64, debug-monitors: " Srivatsa S. Bhat
2014-03-10 20:39 ` [PATCH v3 32/52] powercap, intel-rapl: " Srivatsa S. Bhat
2014-03-12 22:27   ` Jacob Pan
2014-03-10 20:39 ` [PATCH v3 33/52] scsi, bnx2i: " Srivatsa S. Bhat
2014-03-10 20:39 ` [PATCH v3 34/52] scsi, bnx2fc: " Srivatsa S. Bhat
2014-03-10 20:39 ` [PATCH v3 35/52] scsi, fcoe: " Srivatsa S. Bhat
2014-03-10 20:39 ` [PATCH v3 36/52] zsmalloc: " Srivatsa S. Bhat
2014-03-14  6:41   ` Minchan Kim
2014-03-10 20:40 ` [PATCH v3 37/52] acpi-cpufreq: " Srivatsa S. Bhat
2014-03-10 20:40 ` [PATCH v3 38/52] drivers/base/topology.c: " Srivatsa S. Bhat
2014-03-10 20:40 ` [PATCH v3 39/52] clocksource, dummy-timer: " Srivatsa S. Bhat
2014-03-10 20:40 ` [PATCH v3 40/52] intel-idle: " Srivatsa S. Bhat
2014-03-10 20:40 ` [PATCH v3 41/52] oprofile, nmi-timer: " Srivatsa S. Bhat
2014-03-10 20:40 ` [PATCH v3 42/52] octeon, watchdog: " Srivatsa S. Bhat
2014-03-10 20:40 ` [PATCH v3 43/52] thermal, x86-pkg-temp: " Srivatsa S. Bhat
2014-03-10 20:41 ` [PATCH v3 44/52] hwmon, coretemp: " Srivatsa S. Bhat
2014-03-10 20:41 ` [PATCH v3 45/52] hwmon, via-cputemp: " Srivatsa S. Bhat
2014-03-10 20:41 ` Srivatsa S. Bhat [this message]
2014-03-10 20:41 ` [PATCH v3 47/52] trace, ring-buffer: " Srivatsa S. Bhat
2014-03-10 20:42 ` [PATCH v3 48/52] profile: " Srivatsa S. Bhat
2014-03-10 20:42 ` [PATCH v3 49/52] mm, vmstat: " Srivatsa S. Bhat
2014-03-10 20:42 ` [PATCH v3 50/52] mm, zswap: " Srivatsa S. Bhat
2014-03-10 20:42 ` [PATCH v3 51/52] net/core/flow.c: " Srivatsa S. Bhat
2014-03-10 20:42 ` [PATCH v3 52/52] net/iucv/iucv.c: " Srivatsa S. Bhat
2014-03-11  0:31 ` [PATCH v3 00/52] CPU hotplug: Fix issues with " Rafael J. Wysocki
2014-03-11 22:07 ` Andrew Morton
2014-03-12 20:48   ` Srivatsa S. Bhat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140310204144.10746.62444.stgit@srivatsabhat.in.ibm.com \
    --to=srivatsa.bhat@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=david.vrabel@citrix.com \
    --cc=ego@linux.vnet.ibm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=rusty@rustcorp.com.au \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=walken@google.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).