Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756868AbbBEKus (ORCPT ); Thu, 5 Feb 2015 05:50:48 -0500 Received: from pandora.arm.linux.org.uk ([78.32.30.218]:49888 "EHLO pandora.arm.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751212AbbBEKur (ORCPT ); Thu, 5 Feb 2015 05:50:47 -0500 Date: Thu, 5 Feb 2015 10:50:35 +0000 From: Russell King - ARM Linux To: Krzysztof Kozlowski Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, paulmck@linux.vnet.ibm.com, Arnd Bergmann , Mark Rutland , Bartlomiej Zolnierkiewicz , Marek Szyprowski , Stephen Boyd , Catalin Marinas , Will Deacon 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-Type: text/plain; charset=us-ascii 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) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3920 Lines: 121 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); -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/