From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> To: paulus@samba.org, oleg@redhat.com, rjw@rjwysocki.net, rusty@rustcorp.com.au, peterz@infradead.org, tglx@linutronix.de, akpm@linux-foundation.org Cc: paulmck@linux.vnet.ibm.com, tj@kernel.org, walken@google.com, ego@linux.vnet.ibm.com, linux@arm.linux.org.uk, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-pm@vger.kernel.org, linuxppc-dev@ozlabs.org, srivatsa.bhat@linux.vnet.ibm.com, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, David Vrabel <david.vrabel@citrix.com>, Ingo Molnar <mingo@kernel.org>, xen-devel@lists.xenproject.org"Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> 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");
WARNING: multiple messages have this Message-ID (diff)
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: paulmck@linux.vnet.ibm.com, tj@kernel.org, walken@google.com, ego@linux.vnet.ibm.com, linux@arm.linux.org.uk, linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-pm@vger.kernel.org, linuxppc-dev@ozlabs.org, srivatsa.bhat@linux.vnet.ibm.com, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, David Vrabel <david.vrabel@citrix.com>, xen-devel@lists.xenproject.org"Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> 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) Message-ID: <20140310204145.6l45O2tpl8GXXNbgZr5f_VM5QA4CJiAP3_o1c10SSyM@z> (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");
next prev parent reply other threads:[~2014-03-10 20:42 UTC|newest] Thread overview: 123+ 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:33 ` 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 ` 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 ` 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 ` 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 ` Srivatsa S. Bhat 2014-03-10 20:34 ` [PATCH v3 05/52] ia64, salinfo: Fix " Srivatsa S. Bhat 2014-03-10 20:34 ` Srivatsa S. Bhat 2014-03-10 20:34 ` [PATCH v3 06/52] ia64, palinfo: Fix CPU " Srivatsa S. Bhat 2014-03-10 20:34 ` Srivatsa S. Bhat 2014-03-10 20:34 ` [PATCH v3 07/52] ia64, topology: " Srivatsa S. Bhat 2014-03-10 20:34 ` Srivatsa S. Bhat 2014-03-10 20:35 ` [PATCH v3 08/52] ia64, err-inject: " Srivatsa S. Bhat 2014-03-10 20:35 ` Srivatsa S. Bhat 2014-03-10 20:35 ` [PATCH v3 09/52] arm, hw-breakpoint: " Srivatsa S. Bhat 2014-03-10 20:35 ` Srivatsa S. Bhat 2014-03-10 20:35 ` [PATCH v3 10/52] arm, kvm: " Srivatsa S. Bhat 2014-03-10 20:35 ` Srivatsa S. Bhat 2014-03-12 23:21 ` Christoffer Dall 2014-03-12 23:21 ` Christoffer Dall 2014-03-14 5:43 ` Srivatsa S. Bhat 2014-03-14 5:43 ` Srivatsa S. Bhat 2014-03-14 19:10 ` Christoffer Dall 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-18 22:08 ` Christoffer Dall 2014-03-10 20:35 ` [PATCH v3 11/52] s390, cacheinfo: " Srivatsa S. Bhat 2014-03-10 20:35 ` Srivatsa S. Bhat 2014-03-10 20:35 ` [PATCH v3 12/52] s390, smp: " Srivatsa S. Bhat 2014-03-10 20:35 ` Srivatsa S. Bhat 2014-03-10 20:36 ` [PATCH v3 13/52] sparc, sysfs: " Srivatsa S. Bhat 2014-03-10 20:36 ` Srivatsa S. Bhat 2014-03-10 20:36 ` [PATCH v3 14/52] powerpc, " Srivatsa S. Bhat 2014-03-10 20:36 ` Srivatsa S. Bhat 2014-03-10 20:36 ` [PATCH v3 15/52] x86, msr: " Srivatsa S. Bhat 2014-03-10 20:36 ` Srivatsa S. Bhat 2014-03-10 20:36 ` [PATCH v3 16/52] x86, cpuid: " Srivatsa S. Bhat 2014-03-10 20:36 ` Srivatsa S. Bhat 2014-03-10 20:36 ` [PATCH v3 17/52] x86, vsyscall: " Srivatsa S. Bhat 2014-03-10 20:36 ` Srivatsa S. Bhat 2014-03-10 20:36 ` [PATCH v3 18/52] x86, intel, uncore: " Srivatsa S. Bhat 2014-03-10 20:36 ` Srivatsa S. Bhat 2014-03-10 20:37 ` [PATCH v3 19/52] x86, mce: " Srivatsa S. Bhat 2014-03-10 20:37 ` Srivatsa S. Bhat 2014-03-10 20:37 ` [PATCH v3 20/52] x86, therm_throt.c: " Srivatsa S. Bhat 2014-03-10 20:37 ` 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 ` 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 ` Srivatsa S. Bhat 2014-03-10 20:37 ` [PATCH v3 23/52] x86, intel, cacheinfo: " Srivatsa S. Bhat 2014-03-10 20:37 ` Srivatsa S. Bhat 2014-03-10 20:38 ` [PATCH v3 24/52] x86, intel, rapl: " Srivatsa S. Bhat 2014-03-10 20:38 ` Srivatsa S. Bhat 2014-03-10 20:38 ` [PATCH v3 25/52] x86, amd, uncore: " Srivatsa S. Bhat 2014-03-10 20:38 ` Srivatsa S. Bhat 2014-03-10 20:38 ` [PATCH v3 26/52] x86, hpet: " Srivatsa S. Bhat 2014-03-10 20:38 ` Srivatsa S. Bhat 2014-03-10 20:38 ` [PATCH v3 27/52] x86, pci, amd-bus: " Srivatsa S. Bhat 2014-03-10 20:38 ` Srivatsa S. Bhat 2014-03-10 20:38 ` [PATCH v3 28/52] x86, oprofile, nmi: " Srivatsa S. Bhat 2014-03-10 20:38 ` Srivatsa S. Bhat 2014-03-10 20:39 ` [PATCH v3 29/52] x86, kvm: " Srivatsa S. Bhat 2014-03-10 20:39 ` Srivatsa S. Bhat 2014-03-10 20:39 ` [PATCH v3 30/52] arm64, hw_breakpoint.c: " Srivatsa S. Bhat 2014-03-10 20:39 ` Srivatsa S. Bhat 2014-03-10 20:39 ` [PATCH v3 31/52] arm64, debug-monitors: " Srivatsa S. Bhat 2014-03-10 20:39 ` Srivatsa S. Bhat 2014-03-10 20:39 ` [PATCH v3 32/52] powercap, intel-rapl: " Srivatsa S. Bhat 2014-03-10 20:39 ` 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 ` Srivatsa S. Bhat 2014-03-10 20:39 ` [PATCH v3 34/52] scsi, bnx2fc: " Srivatsa S. Bhat 2014-03-10 20:39 ` Srivatsa S. Bhat 2014-03-10 20:39 ` [PATCH v3 35/52] scsi, fcoe: " Srivatsa S. Bhat 2014-03-10 20:39 ` Srivatsa S. Bhat 2014-03-10 20:39 ` [PATCH v3 36/52] zsmalloc: " Srivatsa S. Bhat 2014-03-10 20:39 ` 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 ` Srivatsa S. Bhat 2014-03-10 20:40 ` [PATCH v3 38/52] drivers/base/topology.c: " Srivatsa S. Bhat 2014-03-10 20:40 ` Srivatsa S. Bhat 2014-03-10 20:40 ` [PATCH v3 39/52] clocksource, dummy-timer: " Srivatsa S. Bhat 2014-03-10 20:40 ` Srivatsa S. Bhat 2014-03-10 20:40 ` [PATCH v3 40/52] intel-idle: " Srivatsa S. Bhat 2014-03-10 20:40 ` Srivatsa S. Bhat 2014-03-10 20:40 ` [PATCH v3 41/52] oprofile, nmi-timer: " Srivatsa S. Bhat 2014-03-10 20:40 ` Srivatsa S. Bhat 2014-03-10 20:40 ` [PATCH v3 42/52] octeon, watchdog: " Srivatsa S. Bhat 2014-03-10 20:40 ` Srivatsa S. Bhat 2014-03-10 20:40 ` [PATCH v3 43/52] thermal, x86-pkg-temp: " Srivatsa S. Bhat 2014-03-10 20:40 ` Srivatsa S. Bhat 2014-03-10 20:41 ` [PATCH v3 44/52] hwmon, coretemp: " Srivatsa S. Bhat 2014-03-10 20:41 ` 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 2014-03-10 20:41 ` Srivatsa S. Bhat [this message] 2014-03-10 20:41 ` [PATCH v3 46/52] xen, balloon: " Srivatsa S. Bhat 2014-03-10 20:41 ` [PATCH v3 47/52] trace, ring-buffer: " Srivatsa S. Bhat 2014-03-10 20:41 ` Srivatsa S. Bhat 2014-03-10 20:42 ` [PATCH v3 48/52] profile: " Srivatsa S. Bhat 2014-03-10 20:42 ` Srivatsa S. Bhat 2014-03-10 20:42 ` [PATCH v3 49/52] mm, vmstat: " Srivatsa S. Bhat 2014-03-10 20:42 ` Srivatsa S. Bhat 2014-03-10 20:42 ` [PATCH v3 50/52] mm, zswap: " Srivatsa S. Bhat 2014-03-10 20:42 ` Srivatsa S. Bhat 2014-03-10 20:42 ` [PATCH v3 51/52] net/core/flow.c: " Srivatsa S. Bhat 2014-03-10 20:42 ` Srivatsa S. Bhat 2014-03-10 20:42 ` [PATCH v3 52/52] net/iucv/iucv.c: " Srivatsa S. Bhat 2014-03-10 20:42 ` Srivatsa S. Bhat 2014-03-11 0:31 ` [PATCH v3 00/52] CPU hotplug: Fix issues with " Rafael J. Wysocki 2014-03-11 0:31 ` Rafael J. Wysocki 2014-03-11 22:07 ` Andrew Morton 2014-03-11 22:07 ` Andrew Morton 2014-03-12 20:48 ` Srivatsa S. Bhat 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: linkBe 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).