From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_2 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 36EA6C4363D for ; Fri, 25 Sep 2020 11:35:50 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DF846206C9 for ; Fri, 25 Sep 2020 11:35:49 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="ecFmGb8o" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DF846206C9 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=Huawei.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=G2PKh6xKV01mOD9t1zPY8B4D0gu0K7Ep9xHLu8xz9Yo=; b=ecFmGb8o8BwImbS5wfs0Ohk2Q nqQLUFIyXk61gttSbU+vGqFaF9QqNbEc1o0DYuAJjpH9Crh3aJYL7UlqD0thxEvVRzq900m7wGEO2 MFwc483GeCtc14jQ4fpMbH8D7OXR3E50Z2+xgazB+I02TCjc4zz21sbAfhcGXrwOfgfBCHbjrRFr7 mWAOLue7J8UUTGZQ30OlXMjpgy7g7RyBuJ2alZmSYDsHSWZdGkOTbanT7fIDoJ520vgTuO+qLA/VK BbCMxpJXmweYowJNSTFQvsTzD4qHhV7E31P6jd/XV1tnU379gmUdb7tWvAR1Zr/ba9rhXgSkX9zlZ YE76XflPQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kLlzf-0005MG-R1; Fri, 25 Sep 2020 11:34:27 +0000 Received: from lhrrgout.huawei.com ([185.176.76.210] helo=huawei.com) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kLlzd-0005Jw-AR for linux-arm-kernel@lists.infradead.org; Fri, 25 Sep 2020 11:34:26 +0000 Received: from lhreml710-chm.china.huawei.com (unknown [172.18.7.106]) by Forcepoint Email with ESMTP id 23D44ACD6EC795CB14F6; Fri, 25 Sep 2020 12:34:09 +0100 (IST) Received: from localhost (10.52.122.107) by lhreml710-chm.china.huawei.com (10.201.108.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1913.5; Fri, 25 Sep 2020 12:34:08 +0100 Date: Fri, 25 Sep 2020 12:32:26 +0100 From: Jonathan Cameron To: Borislav Petkov Subject: Re: [PATCH v10 2/6] x86: Support Generic Initiator only proximity domains Message-ID: <20200925123226.0000636a@Huawei.com> In-Reply-To: <20200923160609.GO28545@zn.tnic> References: <20200907140307.571932-1-Jonathan.Cameron@huawei.com> <20200907140307.571932-3-Jonathan.Cameron@huawei.com> <20200923160609.GO28545@zn.tnic> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 3.17.4 (GTK+ 2.24.32; i686-w64-mingw32) MIME-Version: 1.0 X-Originating-IP: [10.52.122.107] X-ClientProxiedBy: lhreml736-chm.china.huawei.com (10.201.108.87) To lhreml710-chm.china.huawei.com (10.201.108.61) X-CFilter-Loop: Reflected X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200925_073425_459633_5BA088F8 X-CRM114-Status: GOOD ( 32.17 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lorenzo Pieralisi , linux-acpi@vger.kernel.org, rafael@kernel.org, linux-api@vger.kernel.org, x86@kernel.org, Hanjun Guo , linux-kernel@vger.kernel.org, linuxarm@huawei.com, linux-mm@kvack.org, Ingo Molnar , Brice Goglin , Bjorn Helgaas , Thomas Gleixner , Dan Williams , linux-arm-kernel@lists.infradead.org, Sean V Kelley Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, 23 Sep 2020 18:06:09 +0200 Borislav Petkov wrote: > On Mon, Sep 07, 2020 at 10:03:03PM +0800, Jonathan Cameron wrote: > > In common with memoryless domains we only register GI domains > > if the proximity node is not online. If a domain is already > > a memory containing domain, or a memoryless domain there is > > nothing to do just because it also contains a Generic Initiator. > > > > Signed-off-by: Jonathan Cameron Hi Borislav, Thanks for taking a look. Answers inline. > > --- > > arch/x86/include/asm/numa.h | 2 ++ > > arch/x86/kernel/setup.c | 1 + > > arch/x86/mm/numa.c | 14 ++++++++++++++ > > 3 files changed, 17 insertions(+) > > > > diff --git a/arch/x86/include/asm/numa.h b/arch/x86/include/asm/numa.h > > index bbfde3d2662f..f631467272a3 100644 > > --- a/arch/x86/include/asm/numa.h > > +++ b/arch/x86/include/asm/numa.h > > @@ -62,12 +62,14 @@ extern void numa_clear_node(int cpu); > > extern void __init init_cpu_to_node(void); > > extern void numa_add_cpu(int cpu); > > extern void numa_remove_cpu(int cpu); > > +extern void init_gi_nodes(void); > > #else /* CONFIG_NUMA */ > > static inline void numa_set_node(int cpu, int node) { } > > static inline void numa_clear_node(int cpu) { } > > static inline void init_cpu_to_node(void) { } > > static inline void numa_add_cpu(int cpu) { } > > static inline void numa_remove_cpu(int cpu) { } > > +static inline void init_gi_nodes(void) { } > > #endif /* CONFIG_NUMA */ > > > > #ifdef CONFIG_DEBUG_PER_CPU_MAPS > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > > index 3511736fbc74..9062c146f03a 100644 > > --- a/arch/x86/kernel/setup.c > > +++ b/arch/x86/kernel/setup.c > > @@ -1218,6 +1218,7 @@ void __init setup_arch(char **cmdline_p) > > prefill_possible_map(); > > > > init_cpu_to_node(); > > + init_gi_nodes(); > > Can this function be an early_initcall() or so instead which you can > call in numa.c directly instead of exporting it and calling it here? I don't think we can. This is doing the same operation as is done for memoryless cpu nodes in the init_cpu_to_node() call above so it would make little sense from a code flow point of view, even if it were possible. However it isn't possible as far as I can see. It is called after init_cpu_to_node() because... * A GI node may also be a CPU node and / or a Memory Node, but we only have to do anything extra if it has neither of these. Easiest way to do that is use the same logic init_cpu_to_node() does and rely on ordering wrt to the other two types of nodes that have already been handled. We could have could just call it at the end of init_cpu_to_node() but I'd not be happy doing so without renaming that function and then we'd end up with a horrible name like init_cpu_to_node_and_gi() as they are rather different things. It needs to happen before use is made of the node_data which is allocated in init_gi_nodes() / init_memoryless_node() / alloc_node_data() I think the first call that uses node_data is build_all_zonelists() (there might be one hiding earlier but it's definitely needed by this call). We need the fallback lists to be setup correctly for the GI node, just as they are for the memoryless node that has CPUs. So there is a narrow window in which we need to call this. As mentioned I could just call it from init_cpu_to_node() but that would make the code harder to understand given it's nothing to do with cpus. It might be possible to allocate the zonelists for this special case later in the boot flow, but it seems like we would be adding a lot of complexity to avoid a single function call. Just to check my logic, I gave using early_initcall() a go and it blows up spectacularly when allocations start happening for Generic Initiators. > > > io_apic_init_mappings(); > > > > diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c > > index aa76ec2d359b..fc630dc6764e 100644 > > --- a/arch/x86/mm/numa.c > > +++ b/arch/x86/mm/numa.c > > @@ -747,6 +747,20 @@ static void __init init_memory_less_node(int nid) > > */ > > } > > > > +/* > > + * Generic Initiator Nodes may have neither CPU nor Memory. > > + * At this stage if either of the others were present we would > > Who's "we"? And what is "either of the others"? The other nodes? "We" is the kernel. Silly idiom, I'll rephrase. "Either of the others" refers to CPU or memory as per the line above. I'll state that explicitly. How about this? +/* + * A node may exist which has one or more Generic Initiators but no + * CPUs and no memory. + * When this function is called, any nodes containing either memory + * and/or CPUs will already be online and there is no need to do + * anything extra, even if they also contain one or more Generic + * Initiators. + */ > Thanks, Jonathan _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel