Re: [PATCH v9 7/7] spapr_numa.c: handle auto NUMA node with no distance info

[David Gibson]

On Mon, Sep 20, 2021 at 02:49:47PM -0300, Daniel Henrique Barboza wrote:
> numa_complete_configuration() in hw/core/numa.c always adds a NUMA node
> for the pSeries machine if none was specified, but without node distance
> information for the single node created.
> 
> NUMA FORM1 affinity code didn't rely on numa_state information to do its
> job, but FORM2 does. As is now, this is the result of a pSeries guest
> with NUMA FORM2 affinity when no NUMA nodes is specified:
> 
> $ numactl -H
> available: 1 nodes (0)
> node 0 cpus: 0
> node 0 size: 16222 MB
> node 0 free: 15681 MB
> No distance information available.
> 
> This can be amended in spapr_numa_FORM2_write_rtas_tables(). We're
> enforcing that the local distance (the distance to the node to itself) is
> always 10. This allows for the proper creation of the NUMA distance tables,
> fixing the output of 'numactl -H' in the guest:
> 
> $ numactl -H
> available: 1 nodes (0)
> node 0 cpus: 0
> node 0 size: 16222 MB
> node 0 free: 15685 MB
> node distances:
> node   0
>   0:  10
> 
> CC: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/ppc/spapr_numa.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/hw/ppc/spapr_numa.c b/hw/ppc/spapr_numa.c
> index 13db321997..58d5dc7084 100644
> --- a/hw/ppc/spapr_numa.c
> +++ b/hw/ppc/spapr_numa.c
> @@ -539,6 +539,17 @@ static void 
> spapr_numa_FORM2_write_rtas_tables(SpaprMachineState *spapr,
>  
>      for (src = 0; src < nb_numa_nodes; src++) {
>          for (dst = 0; dst < nb_numa_nodes; dst++) {
> +            /*
> +             * We need to be explicit with the local distance
> +             * value to cover the case where the user didn't added any
> +             * NUMA nodes, but QEMU adds the default NUMA node without
> +             * adding the numa_info to retrieve distance info from.
> +             */
> +            if (src == dst) {
> +                node_distances[i++] = 10;

Would it make sense to use NUMA_DISTANCE_MIN here, rather than a fixed
value?

Again, simple enough that it can be fixed in followup.

> +                continue;
> +            }
> +
>              node_distances[i++] = numa_info[src].distance[dst];
>          }
>      }

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature