From patchwork Wed Feb 25 20:16:59 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Pitre X-Patchwork-Id: 5883381 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 D0F4D9F269 for ; Wed, 25 Feb 2015 20:20:17 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id F161620373 for ; Wed, 25 Feb 2015 20:20:16 +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 12F9B2034E for ; Wed, 25 Feb 2015 20:20:16 +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 1YQiOh-0005QJ-HR; Wed, 25 Feb 2015 20:17:31 +0000 Received: from relais.videotron.ca ([24.201.245.36]) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YQiOc-0005K3-MI for linux-arm-kernel@lists.infradead.org; Wed, 25 Feb 2015 20:17:27 +0000 MIME-version: 1.0 Received: from yoda.home ([66.131.180.142]) by VL-VM-MR007.ip.videotron.ca (Oracle Communications Messaging Exchange Server 7u4-22.01 64bit (built Apr 21 2011)) with ESMTP id <0NKC00LA6GCE9X70@VL-VM-MR007.ip.videotron.ca> for linux-arm-kernel@lists.infradead.org; Wed, 25 Feb 2015 15:17:02 -0500 (EST) Received: from xanadu.home (xanadu.home [192.168.2.2]) by yoda.home (Postfix) with ESMTPSA id 0E8492DA045B; Wed, 25 Feb 2015 15:17:02 -0500 (EST) Date: Wed, 25 Feb 2015 15:16:59 -0500 (EST) From: Nicolas Pitre To: Russell King - ARM Linux Subject: Re: [PATCH v2] ARM: Don't use complete() during __cpu_die In-reply-to: Message-id: References: <1423131270-24047-1-git-send-email-k.kozlowski@samsung.com> <20150205105035.GL8656@n2100.arm.linux.org.uk> <20150205142918.GA10634@linux.vnet.ibm.com> <20150205161100.GQ8656@n2100.arm.linux.org.uk> <20150225125610.GY8656@n2100.arm.linux.org.uk> <20150225170011.GC8656@n2100.arm.linux.org.uk> User-Agent: Alpine 2.11 (LFD 23 2013-08-11) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150225_121726_804965_84A7A38A X-CRM114-Status: GOOD ( 20.90 ) X-Spam-Score: -0.0 (/) Cc: Mark Rutland , Krzysztof Kozlowski , Arnd Bergmann , Bartlomiej Zolnierkiewicz , Catalin Marinas , Stephen Boyd , linux-kernel@vger.kernel.org, Will Deacon , "Paul E. McKenney" , 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.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 Wed, 25 Feb 2015, Nicolas Pitre wrote: > On Wed, 25 Feb 2015, Russell King - ARM Linux wrote: > > > We could just use the spin-and-poll solution instead of an IPI, but > > I really don't like that - when you see the complexity needed to > > re-initialise it each time, it quickly becomes very yucky because > > there is no well defined order between __cpu_die() and __cpu_kill() > > being called by the two respective CPUs. > > > > The last patch I saw doing that had multiple bits to indicate success > > and timeout, and rather a lot of complexity to recover from failures, > > and reinitialise state for a second CPU going down. > > What about a per CPU state? That would at least avoid the need to > serialize things across CPUs. If only one CPU may write its state, that > should eliminate the need for any kind of locking. Something like the following? If according to $subject it is the complete() usage that has problems, then this replacement certainly has it removed while keeping things simple. And I doubt CPU hotplug is performance critical so a simple polling is certainly good enough. diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 86ef244c5a..f253f79a34 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -213,7 +213,7 @@ int __cpu_disable(void) return 0; } -static DECLARE_COMPLETION(cpu_died); +static struct cpumask dead_cpus; /* * called on the thread which is asking for a CPU to be shutdown - @@ -221,7 +221,14 @@ static DECLARE_COMPLETION(cpu_died); */ void __cpu_die(unsigned int cpu) { - if (!wait_for_completion_timeout(&cpu_died, msecs_to_jiffies(5000))) { + int i; + + for (i = 5 * HZ; i > 0; i -= 10) { + if (cpumask_test_cpu(cpu, &dead_cpus)) + break; + schedule_timeout_uninterruptible(10); + } + if (i <= 0) { pr_err("CPU%u: cpu didn't die\n", cpu); return; } @@ -267,12 +274,12 @@ void __ref cpu_die(void) * this returns, power and/or clocks can be removed at any point * from this CPU and its cache by platform_cpu_kill(). */ - complete(&cpu_died); + cpumask_set_cpu(cpu, &dead_cpus); /* - * Ensure that the cache lines associated with that completion are + * Ensure that the cache line associated with that dead_cpus update is * written out. This covers the case where _this_ CPU is doing the - * powering down, to ensure that the completion is visible to the + * powering down, to ensure that the update is visible to the * CPU waiting for this one. */ flush_cache_louis(); @@ -349,6 +356,8 @@ asmlinkage void secondary_start_kernel(void) current->active_mm = mm; cpumask_set_cpu(cpu, mm_cpumask(mm)); + cpumask_clear_cpu(cpu, &dead_cpus); + cpu_init(); pr_debug("CPU%u: Booted secondary processor\n", cpu);