"Aneesh Kumar K.V" <aneesh.ku...@linux.ibm.com> writes: > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c > index e437a9ac4956..6c659aada55b 100644 > --- a/arch/powerpc/mm/numa.c > +++ b/arch/powerpc/mm/numa.c > @@ -221,25 +221,51 @@ static void initialize_distance_lookup_table(int nid, > } > } > > +static u32 nid_map[MAX_NUMNODES] = {[0 ... MAX_NUMNODES - 1] = > NUMA_NO_NODE};
It's odd to me to use MAX_NUMNODES for this array when it's going to be indexed not by Linux's logical node IDs but by the platform-provided domain number, which has no relation to MAX_NUMNODES. > + > +int firmware_group_id_to_nid(int firmware_gid) > +{ > + static int last_nid = 0; > + > + /* > + * For PowerNV we don't change the node id. This helps to avoid > + * confusion w.r.t the expected node ids. On pseries, node numbers > + * are virtualized. Hence do logical node id for pseries. > + */ > + if (!firmware_has_feature(FW_FEATURE_LPAR)) > + return firmware_gid; > + > + if (firmware_gid == -1) > + return NUMA_NO_NODE; > + > + if (nid_map[firmware_gid] == NUMA_NO_NODE) > + nid_map[firmware_gid] = last_nid++; This should at least be bounds-checked in case of domain numbering in excess of MAX_NUMNODES. Or a different data structure should be used? Not sure. I'd prefer Linux's logical node type not be easily interchangeable with the firmware node/group id type. The firmware type could be something like: struct affinity_domain { u32 val; }; typedef struct affinity_domain affinity_domain_t; with appropriate accessors/APIs. This can prevent a whole class of errors that is currently possible with CPUs, since the logical and "hardware" ID types are both simple integers.