[PATCH v3] powerpc/smp: Use nid as fallback for package_id
Michael Ellerman
mpe at ellerman.id.au
Mon Jan 13 23:40:41 AEDT 2020
Hi Srikar,
Still some comments sorry ...
Srikar Dronamraju <srikar at linux.vnet.ibm.com> writes:
> Package_id is to find out all cores that are part of the same chip. On
> PowerNV machines, package_id defaults to chip_id. However ibm,chip_id
> property is not present in device-tree of PowerVM Lpars. Hence lscpu
> output shows one core per socket and multiple cores.
>
> To overcome this, use nid as the package_id on PowerVM Lpars.
>
> Before the patch.
> ---------------
> Architecture: ppc64le
> Byte Order: Little Endian
> CPU(s): 128
> On-line CPU(s) list: 0-127
> Thread(s) per core: 8
> Core(s) per socket: 1 <----------------------
> Socket(s): 16 <----------------------
> NUMA node(s): 2
> Model: 2.2 (pvr 004e 0202)
> Model name: POWER9 (architected), altivec supported
> Hypervisor vendor: pHyp
> Virtualization type: para
> L1d cache: 32K
> L1i cache: 32K
> L2 cache: 512K
> L3 cache: 10240K
> NUMA node0 CPU(s): 0-63
> NUMA node1 CPU(s): 64-127
> #
> # cat /sys/devices/system/cpu/cpu0/topology/physical_package_id
> -1
> #
>
> After the patch
> ---------------
> Architecture: ppc64le
> Byte Order: Little Endian
> CPU(s): 128
> On-line CPU(s) list: 0-127
> Thread(s) per core: 8 <------------------------------
> Core(s) per socket: 8 <------------------------------
> Socket(s): 2
> NUMA node(s): 2
> Model: 2.2 (pvr 004e 0202)
> Model name: POWER9 (architected), altivec supported
> Hypervisor vendor: pHyp
> Virtualization type: para
> L1d cache: 32K
> L1i cache: 32K
> L2 cache: 512K
> L3 cache: 10240K
> NUMA node0 CPU(s): 0-63
> NUMA node1 CPU(s): 64-127
> #
> # cat /sys/devices/system/cpu/cpu0/topology/physical_package_id
> 0
> #
> Now lscpu output is more in line with the system configuration.
>
> Signed-off-by: Srikar Dronamraju <srikar at linux.vnet.ibm.com>
> Cc: linuxppc-dev at lists.ozlabs.org
> Cc: Michael Ellerman <mpe at ellerman.id.au>
> Cc: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
> Cc: Vaidyanathan Srinivasan <svaidy at linux.vnet.ibm.com>
> ---
> Changelog from v2: https://patchwork.ozlabs.org/patch/1151649
> - Rebased to v5.5-rc2
> - Renamed the new function to cpu_to_nid
> - Removed checks to fadump property. (Looked too excessive)
> - Moved pseries specific code to pseries/lpar.c
>
> Changelog from v1: https://patchwork.ozlabs.org/patch/1126145
> - In v1 cpu_to_chip_id was overloaded to fallback on nid. Michael
> Ellerman wasn't comfortable with nid being shown up as chip_id.
>
> arch/powerpc/include/asm/topology.h | 7 ++++++-
> arch/powerpc/kernel/smp.c | 6 +++---
> arch/powerpc/platforms/pseries/lpar.c | 22 ++++++++++++++++++++++
> 3 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/topology.h b/arch/powerpc/include/asm/topology.h
> index 2f7e1ea5089e..7422ef913c75 100644
> --- a/arch/powerpc/include/asm/topology.h
> +++ b/arch/powerpc/include/asm/topology.h
> @@ -130,11 +130,16 @@ static inline void shared_proc_topology_init(void) {}
>
> #ifdef CONFIG_SMP
> #include <asm/cputable.h>
> +#ifdef CONFIG_PPC_SPLPAR
> +int cpu_to_nid(int);
> +#else
> +#define cpu_to_nid(cpu) cpu_to_chip_id(cpu)
> +#endif
We already have cpu_to_node(), which returns the numa node (nid) for a
given CPU, so I think introducing a new accessor with an almost
identical name is going to cause confusion. ie. when should code use
cpu_to_node() and when should it use cpu_to_nid()?
> #ifdef CONFIG_PPC64
> #include <asm/smp.h>
>
> -#define topology_physical_package_id(cpu) (cpu_to_chip_id(cpu))
> +#define topology_physical_package_id(cpu) (cpu_to_nid(cpu))
I think you should be overriding topology_physical_package_id() instead
of introducing cpu_to_nid().
> #define topology_sibling_cpumask(cpu) (per_cpu(cpu_sibling_map, cpu))
> #define topology_core_cpumask(cpu) (per_cpu(cpu_core_map, cpu))
> #define topology_core_id(cpu) (cpu_to_core_id(cpu))
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index ea6adbf6a221..b0c1438d8d9a 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1188,7 +1188,7 @@ static inline void add_cpu_to_smallcore_masks(int cpu)
> static void add_cpu_to_masks(int cpu)
> {
> int first_thread = cpu_first_thread_sibling(cpu);
> - int chipid = cpu_to_chip_id(cpu);
> + int nid = cpu_to_nid(cpu);
And here you should be using topology_physical_package_id(), rather than
cpu_to_nid() directly.
> int i;
>
> /*
> @@ -1217,11 +1217,11 @@ static void add_cpu_to_masks(int cpu)
> for_each_cpu(i, cpu_l2_cache_mask(cpu))
> set_cpus_related(cpu, i, cpu_core_mask);
>
> - if (chipid == -1)
> + if (nid == -1)
> return;
>
> for_each_cpu(i, cpu_online_mask)
> - if (cpu_to_chip_id(i) == chipid)
> + if (cpu_to_nid(i) == nid)
> set_cpus_related(cpu, i, cpu_core_mask);
> }
>
> diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
> index 60cb29ae4739..99583b9f00e4 100644
> --- a/arch/powerpc/platforms/pseries/lpar.c
> +++ b/arch/powerpc/platforms/pseries/lpar.c
> @@ -650,6 +650,28 @@ static int __init vcpudispatch_stats_procfs_init(void)
I think this function should actually be in smp.c or numa.c
That's because it will be called on non-pseries platforms when we build
a multi platform kernel (pseries & powernv). So it's not really pseries
specific code.
> }
>
> machine_device_initcall(pseries, vcpudispatch_stats_procfs_init);
> +
> +int cpu_to_nid(cpu)
> +{
> + int nid = cpu_to_chip_id(cpu);
> +
> + /*
> + * If the platform is PowerNV or Guest on KVM, ibm,chip-id is
> + * defined. Hence we would return the chip-id as the
> + * cpu_to_nid.
> + */
> + if (nid == -1 && firmware_has_feature(FW_FEATURE_LPAR)) {
> + struct device_node *np = of_get_cpu_node(cpu, NULL);
> +
> + if (np) {
> + nid = of_node_to_nid(np);
> + of_node_put(np);
> + }
> + }
> + return nid;
> +}
> +EXPORT_SYMBOL(cpu_to_nid);
Should be GPL.
cheers
More information about the Linuxppc-dev
mailing list