Re: [PATCH 2/2] powerpc/numa: Return the first online node if device tree mapping returns a not online node

[Srikar Dronamraju <srikar@linux.vnet.ibm.com>]

* Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> [2022-06-23 18:24:42]:

> While building the cpu_to_node map make sure we always use the online node
> to build the mapping table. In general this should not be an issue
> because the kernel use similar lookup mechanism (vphn_get_nid()) to mark
> nodes online (setup_node_data()). Hence NUMA nodes we find during
> lookup in numa_setup_cpu() will always be found online.
> 
> To keep logic simpler/correct, make sure that if the hypervisor
> or device tree returned a not online node, don't use that to build
> the map table. Instead, use the first_online_node.

Why should the returned nid be already online. Are we facing any problem
with the current code?

Since general idea is to keep cpu_to_node constant, By assigining it the
first_online_node, we may be ending up assigning a wrong node, resulting a
performance penalty later on. i.e CPU may actually belong to node 4 but we
assigned it to node 1. Also the coregroup id is derived from associavity
array. If there is a mismatch between the coregroup id and nid,
scheduler will crib.

> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  arch/powerpc/mm/numa.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 0801b2ce9b7d..f387b9eb9dc9 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -741,7 +741,7 @@ static int numa_setup_cpu(unsigned long lcpu)
>  	of_node_put(cpu);
> 
>  out_present:
> -	if (nid < 0 || !node_possible(nid))
> +	if (nid < 0 || !node_online(nid))
>  		nid = first_online_node;
> 
>  	/*
> -- 
> 2.36.1
> 

-- 
Thanks and Regards
Srikar Dronamraju