Re: [Qemu-ppc] [PATCH] spapr_pci: advertise explicit numa IDs even when there's 1 node

[David Gibson]

On Tue, Oct 18, 2016 at 03:50:23PM -0500, Michael Roth wrote:
> With the addition of "numa_node" properties for PHBs we began
> advertising NUMA affinity in cases where nb_numa_nodes > 1.
> 
> Since the default on the guest side is to make no assumptions about
> PHB NUMA affinity (defaulting to -1), there is still a valid use-case
> for explicitly defining a PHB's NUMA affinity even when there's just
> one node. In particular, some workloads make faulty assumptions about
> /sys/bus/pci/<devid>/numa_node being >= 0, warranting the use of
> this property as a workaround even if there's just 1 PHB or NUMA
> node.
> 
> Enable this use-case by always advertising the PHB's NUMA affinity
> if "numa_node" has been explicitly set.
> 
> We could achieve this by relaxing the check to simply be
> nb_numa_nodes > 0, but even safer would be to check
> numa_info[nodeid].present explicitly, and to fail at start time
> for cases where it does not exist.
> 
> This has an additional affect of no longer advertising PHB NUMA
> affinity unconditionally if nb_numa_nodes > 1 and "numa_node"
> property is unset/-1, but since the default value on the guest
> side for each PHB is also -1, the behavior should be the same for
> that situation. We could still retain the old behavior if desired,
> but the decision seems arbitrary, so we take the simpler route.
> 
> Cc: Alexey Kardashevskiy <address@hidden>
> Cc: Shivaprasad G. Bhat <address@hidden>
> Signed-off-by: Michael Roth <address@hidden>

Applied to ppc-for-2.8, thanks.

> ---
>  hw/ppc/spapr_pci.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2a1ccf5..7cde30e 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1392,6 +1392,12 @@ static void spapr_phb_realize(DeviceState *dev, Error 
> **errp)
>          return;
>      }
>  
> +    if (sphb->numa_node != -1 &&
> +        (sphb->numa_node >= MAX_NODES || 
> !numa_info[sphb->numa_node].present)) {
> +        error_setg(errp, "Invalid NUMA node ID for PCI host bridge");
> +        return;
> +    }
> +
>      sphb->dtbusname = g_strdup_printf("address@hidden" PRIx64, sphb->buid);
>  
>      namebuf = alloca(strlen(sphb->dtbusname) + 32);
> @@ -1880,7 +1886,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
>      }
>  
>      /* Advertise NUMA via ibm,associativity */
> -    if (nb_numa_nodes > 1) {
> +    if (phb->numa_node != -1) {
>          _FDT(fdt_setprop(fdt, bus_off, "ibm,associativity", associativity,
>                           sizeof(associativity)));
>      }

-- 
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