From patchwork Mon Mar 10 20:41:45 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Srivatsa S. Bhat" X-Patchwork-Id: 3806391 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 04998BF540 for ; Mon, 10 Mar 2014 20:46:54 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0AF192022F for ; Mon, 10 Mar 2014 20:46:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 106B4201D3 for ; Mon, 10 Mar 2014 20:46:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754666AbaCJUmF (ORCPT ); Mon, 10 Mar 2014 16:42:05 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:56287 "EHLO e23smtp03.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753027AbaCJUmB (ORCPT ); Mon, 10 Mar 2014 16:42:01 -0400 Received: from /spool/local by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 11 Mar 2014 06:42:00 +1000 Received: from d23dlp02.au.ibm.com (202.81.31.213) by e23smtp03.au.ibm.com (202.81.31.209) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 11 Mar 2014 06:41:57 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 1E4DA2BB0052; Tue, 11 Mar 2014 07:41:57 +1100 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id s2AKfhmp9830768; Tue, 11 Mar 2014 07:41:43 +1100 Received: from d23av03.au.ibm.com (localhost [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id s2AKfsvC023953; Tue, 11 Mar 2014 07:41:56 +1100 Received: from srivatsabhat.in.ibm.com ([9.79.217.186]) by d23av03.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id s2AKfmwd023902; Tue, 11 Mar 2014 07:41:49 +1100 From: "Srivatsa S. Bhat" Subject: [PATCH v3 46/52] xen, balloon: Fix CPU hotplug callback registration 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 , David Vrabel , Ingo Molnar , xen-devel@lists.xenproject.org, "Srivatsa S. Bhat" Date: Tue, 11 Mar 2014 02:11:45 +0530 Message-ID: <20140310204144.10746.62444.stgit@srivatsabhat.in.ibm.com> In-Reply-To: <20140310203312.10746.310.stgit@srivatsabhat.in.ibm.com> References: <20140310203312.10746.310.stgit@srivatsabhat.in.ibm.com> User-Agent: StGIT/0.14.3 MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 14031020-6102-0000-0000-0000051B2F8D Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 Cc: David Vrabel Cc: Ingo Molnar Cc: xen-devel@lists.xenproject.org Reviewed-by: Boris Ostrovsky Signed-off-by: Srivatsa S. Bhat --- drivers/xen/balloon.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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");