From patchwork Thu Feb 5 10:50:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell King - ARM Linux X-Patchwork-Id: 5783711 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 21E939F30C for ; Thu, 5 Feb 2015 10:53:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2FF1C20259 for ; Thu, 5 Feb 2015 10:53:06 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2FDAC20253 for ; Thu, 5 Feb 2015 10:53:05 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YJK1h-00089k-KQ; Thu, 05 Feb 2015 10:51:13 +0000 Received: from pandora.arm.linux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YJK1e-00083a-Ke for linux-arm-kernel@lists.infradead.org; Thu, 05 Feb 2015 10:51:12 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=arm.linux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date; bh=v9fthqynIvAnnCnLnGuRa6MRBmHDdoRz0Ty/ehrOrgU=; b=JUjQStor9Or9YhhG85j/Hs08gyfU56xCyZBb9Ut2mShaqq3Ib039qhwVetwmjXWv86ypoW3flVPjpGIyJER0I2uvwCfjnmyq7MoRBREKENO1pwXvJwVXj4SW0rRoCMBSe19hOc4b4agTl9ZqpYMADFWDX8+bd/c8Uo9Qf+Q02gA=; Received: from n2100.arm.linux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:42734) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1YJK19-0003zx-AZ; Thu, 05 Feb 2015 10:50:39 +0000 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1YJK16-0007DH-4G; Thu, 05 Feb 2015 10:50:36 +0000 Date: Thu, 5 Feb 2015 10:50:35 +0000 From: Russell King - ARM Linux To: Krzysztof Kozlowski Subject: Re: [PATCH v2] ARM: Don't use complete() during __cpu_die Message-ID: <20150205105035.GL8656@n2100.arm.linux.org.uk> References: <1423131270-24047-1-git-send-email-k.kozlowski@samsung.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1423131270-24047-1-git-send-email-k.kozlowski@samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150205_025111_014702_5EE70FB5 X-CRM114-Status: GOOD ( 21.57 ) X-Spam-Score: -0.1 (/) Cc: Mark Rutland , Arnd Bergmann , Bartlomiej Zolnierkiewicz , Catalin Marinas , Stephen Boyd , linux-kernel@vger.kernel.org, Will Deacon , paulmck@linux.vnet.ibm.com, linux-arm-kernel@lists.infradead.org, Marek Szyprowski X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, 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 On Thu, Feb 05, 2015 at 11:14:30AM +0100, Krzysztof Kozlowski wrote: > The complete() should not be used on offlined CPU. Rewrite the > wait-complete mechanism with wait_on_bit_timeout(). Yuck. I think that the IPI idea would be far better, and a much smaller patch. We can continue using the completions, but instead of running the completion on the dying CPU, the dying CPU triggers an IPI which does the completion on the requesting CPU. You're modifying arch/arm/kernel/smp.c, so you just hook it directly into the IPI mechanism without any registration required. We can also kill the second cache flush by the dying CPU - as we're not writing to memory anymore by calling complete() after the first cache flush, so this will probably make CPU hotplug fractionally faster too. (You'll get some fuzz with this patch as I have the NMI backtrace stuff in my kernel.) Something like this - only build tested so far (waiting for the compile to finish...): arch/arm/kernel/smp.c | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 194df2f1aa87..c623e27a9c85 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -73,6 +73,9 @@ enum ipi_msg_type { IPI_IRQ_WORK, IPI_COMPLETION, IPI_CPU_BACKTRACE, +#ifdef CONFIG_HOTPLUG_CPU + IPI_CPU_DEAD, +#endif }; /* For reliability, we're prepared to waste bits here. */ @@ -88,6 +91,14 @@ void __init smp_set_ops(struct smp_operations *ops) smp_ops = *ops; }; +static void (*__smp_cross_call)(const struct cpumask *, unsigned int); + +void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int)) +{ + if (!__smp_cross_call) + __smp_cross_call = fn; +} + static unsigned long get_arch_pgd(pgd_t *pgd) { phys_addr_t pgdir = virt_to_idmap(pgd); @@ -267,19 +278,13 @@ void __ref cpu_die(void) flush_cache_louis(); /* - * Tell __cpu_die() that this CPU is now safe to dispose of. Once - * this returns, power and/or clocks can be removed at any point + * Tell __cpu_die() that this CPU is now safe to dispose of. We + * do this via an IPI to any online CPU - it doesn't matter, we + * just need another CPU to run the completion. Once this IPI + * has been sent, power and/or clocks can be removed at any point * from this CPU and its cache by platform_cpu_kill(). */ - complete(&cpu_died); - - /* - * Ensure that the cache lines associated with that completion are - * written out. This covers the case where _this_ CPU is doing the - * powering down, to ensure that the completion is visible to the - * CPU waiting for this one. - */ - flush_cache_louis(); + __smp_cross_call(cpumask_of(cpumask_any(cpu_online_mask)), IPI_CPU_DEAD); /* * The actual CPU shutdown procedure is at least platform (if not @@ -442,14 +447,6 @@ void __init smp_prepare_cpus(unsigned int max_cpus) } } -static void (*__smp_cross_call)(const struct cpumask *, unsigned int); - -void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int)) -{ - if (!__smp_cross_call) - __smp_cross_call = fn; -} - static const char *ipi_types[NR_IPI] __tracepoint_string = { #define S(x,s) [x] = s S(IPI_WAKEUP, "CPU wakeup interrupts"), @@ -648,6 +645,14 @@ void handle_IPI(int ipinr, struct pt_regs *regs) irq_exit(); break; +#ifdef CONFIG_HOTPLUG_CPU + case IPI_CPU_DEAD: + irq_enter(); + complete(&cpu_died); + irq_exit(); + break; +#endif + default: pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);