Re: [RFC PATCH 1/2] powerpc/numa: Introduce logical numa id

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

* Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com> [2020-08-06 16:14:21]:

> >
> > associativity_to_nid gets called the first time a cpu is being made present
> > from offline. So it need not be in boot path. We may to verify if cpu
> > hotplug, dlpar, operations are synchronized. For example a memory hotadd and
> > cpu hotplug are they synchronized? I am not sure if they are synchronized at
> > this time.
> 
> But you don't online cpu or memory to a non existent node post boot
> right?. If the node is existent we have already initialized the nid_map.
> 

Not sure what you mean by existent and non-existent. Are you referring to
online / offline?

> However i am not sure whether we do a parallel initialization of devices. ie,
> of_device_add getting called in parallel. if it can then we need the
> below?
> 
> @@ -226,6 +226,7 @@ static u32 nid_map[MAX_NUMNODES] = {[0 ... MAX_NUMNODES - 1] =  NUMA_NO_NODE};
>  int firmware_group_id_to_nid(int firmware_gid)
>  {
>         static int last_nid = 0;
> +       static DEFINE_SPINLOCK(node_id_lock);
> 
>         /*
>          * For PowerNV we don't change the node id. This helps to avoid
> @@ -238,8 +239,13 @@ int firmware_group_id_to_nid(int firmware_gid)
>         if (firmware_gid ==  -1)
>                 return NUMA_NO_NODE;
> 
> -       if (nid_map[firmware_gid] == NUMA_NO_NODE)
> -               nid_map[firmware_gid] = last_nid++;
> +       if (nid_map[firmware_gid] == NUMA_NO_NODE) {
> +               spin_lock(&node_id_lock);
> +               /*  recheck with lock held */
> +               if (nid_map[firmware_gid] == NUMA_NO_NODE)
> +                       nid_map[firmware_gid] = last_nid++;
> +               spin_unlock(&node_id_lock);
> +       }
> 
>         return nid_map[firmware_gid];
>  }
> 

This should help.


> 
> I will also add a las_nid > MAX_NUMNODES check in
> firmware_group_id_to_nid() to handle the case where we find more numa
> nodes than MAX_NUMANODES in device tree.
> 

Okay, 

Whats your plan to handle the node distances?
Currently the node distances we compute from the device tree properties are
based on distance from node 0.  If you rename a different node as node 0,
how do you plan to remap the node distances?

> -aneesh

-- 
Thanks and Regards
Srikar Dronamraju