From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934592AbbI1Rcy (ORCPT ); Mon, 28 Sep 2015 13:32:54 -0400 Received: from e19.ny.us.ibm.com ([129.33.205.209]:32776 "EHLO e19.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933540AbbI1Rcx (ORCPT ); Mon, 28 Sep 2015 13:32:53 -0400 X-IBM-Helo: d01dlp03.pok.ibm.com X-IBM-MailFrom: nacc@linux.vnet.ibm.com X-IBM-RcptTo: linux-kernel@vger.kernel.org Date: Mon, 28 Sep 2015 10:32:45 -0700 From: Nishanth Aravamudan To: Raghavendra K T Cc: benh@kernel.crashing.org, paulus@samba.org, mpe@ellerman.id.au, anton@samba.org, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, cl@linux.com, gkurz@linux.vnet.ibm.com, grant.likely@linaro.org, nikunj@linux.vnet.ibm.com, khandual@linux.vnet.ibm.com Subject: Re: [PATCH RFC 4/5] powerpc:numa Add helper functions to maintain chipid to nid mapping Message-ID: <20150928173245.GD48470@linux.vnet.ibm.com> References: <1443378553-2146-1-git-send-email-raghavendra.kt@linux.vnet.ibm.com> <1443378553-2146-5-git-send-email-raghavendra.kt@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1443378553-2146-5-git-send-email-raghavendra.kt@linux.vnet.ibm.com> X-Operating-System: Linux 3.13.0-40-generic (x86_64) User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15092817-0057-0000-0000-000001AEA7DC Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 27.09.2015 [23:59:12 +0530], Raghavendra K T wrote: > Create arrays that maps serial nids and sparse chipids. > > Note: My original idea had only two arrays of chipid to nid map. Final > code is inspired by driver/acpi/numa.c that maps a proximity node with > a logical node by Takayoshi Kochi , and thus > uses an additional chipid_map nodemask. The mask helps in first unused > nid easily by knowing first unset bit in the mask. > > No change in functionality. > > Signed-off-by: Raghavendra K T > --- > arch/powerpc/mm/numa.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 47 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c > index dd2073b..f015cad 100644 > --- a/arch/powerpc/mm/numa.c > +++ b/arch/powerpc/mm/numa.c > @@ -63,6 +63,11 @@ static int form1_affinity; > static int distance_ref_points_depth; > static const __be32 *distance_ref_points; > static int distance_lookup_table[MAX_NUMNODES][MAX_DISTANCE_REF_POINTS]; > +static nodemask_t chipid_map = NODE_MASK_NONE; > +static int chipid_to_nid_map[MAX_NUMNODES] > + = { [0 ... MAX_NUMNODES - 1] = NUMA_NO_NODE }; Hrm, conceptually there are *more* chips than nodes, right? So what guarantees we won't see > MAX_NUMNODES chips? > +static int nid_to_chipid_map[MAX_NUMNODES] > + = { [0 ... MAX_NUMNODES - 1] = NUMA_NO_NODE }; > > /* > * Allocate node_to_cpumask_map based on number of available nodes > @@ -133,6 +138,48 @@ static int __init fake_numa_create_new_node(unsigned long end_pfn, > return 0; > } > > +int chipid_to_nid(int chipid) > +{ > + if (chipid < 0) > + return NUMA_NO_NODE; Do you really want to support these cases? Or should they be bugs/warnings indicating that you got an unexpected input? Or at least WARN_ON_ONCE? > + return chipid_to_nid_map[chipid]; > +} > + > +int nid_to_chipid(int nid) > +{ > + if (nid < 0) > + return NUMA_NO_NODE; > + return nid_to_chipid_map[nid]; > +} > + > +static void __map_chipid_to_nid(int chipid, int nid) > +{ > + if (chipid_to_nid_map[chipid] == NUMA_NO_NODE > + || nid < chipid_to_nid_map[chipid]) > + chipid_to_nid_map[chipid] = nid; > + if (nid_to_chipid_map[nid] == NUMA_NO_NODE > + || chipid < nid_to_chipid_map[nid]) > + nid_to_chipid_map[nid] = chipid; > +} chip <-> node mapping is a static (physical) concept, right? Should we emit some debugging if for some reason we get a runtime call to remap an already mapped chip to a new node? > + > +int map_chipid_to_nid(int chipid) > +{ > + int nid; > + > + if (chipid < 0 || chipid >= MAX_NUMNODES) > + return NUMA_NO_NODE; > + > + nid = chipid_to_nid_map[chipid]; > + if (nid == NUMA_NO_NODE) { > + if (nodes_weight(chipid_map) >= MAX_NUMNODES) > + return NUMA_NO_NODE; If you create a KVM guest with a bogus topology, doesn't this just start losing NUMA information for very high-noded guests? > + nid = first_unset_node(chipid_map); > + __map_chipid_to_nid(chipid, nid); > + node_set(nid, chipid_map); > + } > + return nid; > +} > + > int numa_cpu_lookup(int cpu) > { > return numa_cpu_lookup_table[cpu]; > @@ -264,7 +311,6 @@ out: > return chipid; > } > > - stray change? > /* Return the nid from associativity */ > static int associativity_to_nid(const __be32 *associativity) > { > -- > 1.7.11.7 >