From patchwork Tue Nov 24 15:33:06 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: 7691951 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 F32829F1D3 for ; Tue, 24 Nov 2015 15:36:33 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 173A420600 for ; Tue, 24 Nov 2015 15:36:33 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 16727205C1 for ; Tue, 24 Nov 2015 15:36:32 +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 1a1Fbv-0005aK-KU; Tue, 24 Nov 2015 15:34:27 +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 1a1Fbr-0005Rw-Vx for linux-arm-kernel@lists.infradead.org; Tue, 24 Nov 2015 15:34:25 +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=nlBuzVEkr5SNiKMQOndkdNhKAaB9hCdMinT41k/yfj0=; b=GObdKljztDKRdTy9Ua79noaEB3NBeyg+NU5DM8VF3FskWcG5OlTW0RyKk9XFWMXq8eh5eQGIAGxfUJQpKP+iBxZRUGDOzBJbxd9tAZ0ku+z6sEnJAGuIonQKLKGWznAzsRavgzqDnQQJWVhWHHjpiY2DRHD8PHy0kxkohAIMIHA=; Received: from n2100.arm.linux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:4f86]:52080) by pandora.arm.linux.org.uk with esmtpsa (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1a1Fah-00051O-FS; Tue, 24 Nov 2015 15:33:11 +0000 Received: from linux by n2100.arm.linux.org.uk with local (Exim 4.76) (envelope-from ) id 1a1Fad-00058R-LP; Tue, 24 Nov 2015 15:33:08 +0000 Date: Tue, 24 Nov 2015 15:33:06 +0000 From: Russell King - ARM Linux To: Nikita Yushchenko Subject: Re: [RFC/PATCH] arm: do not skip SMP init calls on SMP_ON_UP case Message-ID: <20151124153305.GD8644@n2100.arm.linux.org.uk> References: <1448279946-19975-1-git-send-email-nyushchenko@dev.rtsoft.ru> <20151123120317.GN8644@n2100.arm.linux.org.uk> <5653015C.4020405@dev.rtsoft.ru> <56530769.4030403@arm.com> <5653099A.7020604@dev.rtsoft.ru> <56530AE6.2060407@dev.rtsoft.ru> <20151123130424.GQ8644@n2100.arm.linux.org.uk> <5654799E.5080903@dev.rtsoft.ru> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5654799E.5080903@dev.rtsoft.ru> 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-20151124_073424_475593_68394C84 X-CRM114-Status: GOOD ( 24.49 ) X-Spam-Score: -4.9 (----) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kuznetsovg@dev.rtsoft.ru, Vladimir Murzin , Ian Campbell , Mason , Ard Biesheuvel , Will Deacon , Paul Kocialkowski , linux-kernel@vger.kernel.org, Masahiro Yamada , Pavel Machek , linux-arm-kernel@lists.infradead.org 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.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,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 Tue, Nov 24, 2015 at 05:52:14PM +0300, Nikita Yushchenko wrote: > I'm still trying to understand what is going on, and my printk()s show > that this is not entirely true. > > When smp_init() is entered on mainline om imx6s, cpu_possible_mask and > cpu_present_mask both contain two cpus. These get initialized in > arm_dt_init_cpu_maps() and stay unmodified since then. > > But cpu_online() returns 1 for cpu0 and 0 from cpu1 - thus it is > cpu_online() check, not possible_mask or present_mask, that prevents > cpu1 initialization attempt. No. cpu_online() reports whether the CPU is currently online or not. It's the current state of the system wrt which CPUs are running and not running. Initially, only the boot CPU will be marked online, and the code which brings all CPUs online (see smp_init() in kernel/smp.c) will check whether the CPU is already online prior to trying to bring it up. It will attempt it for any present CPU, up to the maximum number of online CPUs (set by nosmp or maxcpus kernel options.) > Not sure I understand logic behind this. With the current code, > resulting cpu_possible_mask depends on CONFIG_SMP_ON_UP: > - if it is set, cpu_possible_mask contains (0 1), as initialized in > arm_dt_init_cpu_maps() > - if it is not set, cpu_possible_mask contains (0), since > imx_smp_init_cpus() removes 1 from there. Right, adding debug to arch/arm/kernel/setup.c, just before the "if (is_smp())" shows: is_smp() 0 possible 3 present 1 online 1 which is totally wrong: if is_smp() is false, we should not be setting up any possible CPUs. See a patch below to fix that. However, this doesn't matter much, because the code in setup.c won't initialise the SMP operations struct: if (is_smp()) { if (!mdesc->smp_init || !mdesc->smp_init()) { if (psci_smp_available()) smp_set_ops(&psci_smp_ops); else if (mdesc->smp) smp_set_ops(mdesc->smp); } smp_init_cpus(); smp_build_mpidr_hash(); } and this in turn means that __cpu_up() will return -ENOSYS due to this check: if (!smp_ops.smp_boot_secondary) return -ENOSYS; That causes _cpu_up() in kernel/cpu.c to bail out, along with cpu_up(). Notice that the call to smp_init() in kernel/smp.c is silent. Hence, kernel/smp.c will try to bring CPU 1 online (because it is marked present), but it'll _silently_ fail with -ENOSYS. Here's the patch to fix the DT code, which should not be setting present CPUs when is_smp() is false. arch/arm/kernel/devtree.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c index 65addcbf5b30..bd72ce91d7a2 100644 --- a/arch/arm/kernel/devtree.c +++ b/arch/arm/kernel/devtree.c @@ -170,15 +170,18 @@ void __init arm_dt_init_cpu_maps(void) return; } - /* - * Since the boot CPU node contains proper data, and all nodes have - * a reg property, the DT CPU list can be considered valid and the - * logical map created in smp_setup_processor_id() can be overridden - */ - for (i = 0; i < cpuidx; i++) { - set_cpu_possible(i, true); - cpu_logical_map(i) = tmp_map[i]; - pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i)); + if (is_smp()) { + /* + * Since the boot CPU node contains proper data, and all + * nodes have a reg property, the DT CPU list can be + * considered valid and the logical map created in + * smp_setup_processor_id() can be overridden + */ + for (i = 0; i < cpuidx; i++) { + set_cpu_possible(i, true); + cpu_logical_map(i) = tmp_map[i]; + pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i)); + } } }