Re: [PATCH v4] powerpc/smp: Use nid as fallback for package_id
On Wed, 2020-01-29 at 13:51:21 UTC, Srikar Dronamraju wrote: > 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 > Cc: linuxppc-dev@lists.ozlabs.org > Cc: Michael Ellerman > Cc: Vasant Hegde > Cc: Vaidyanathan Srinivasan Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/a05f0e5be4e81e4977d3f92aaf7688ee0cb7d5db cheers
Re: [PATCH v4] powerpc/smp: Use nid as fallback for package_id
* Gautham R Shenoy [2020-01-30 16:27:47]:
> On Wed, Jan 29, 2020 at 07:21:21PM +0530, Srikar Dronamraju wrote:
>
> [..snip..]
>
> > --- a/arch/powerpc/kernel/smp.c
> > +++ b/arch/powerpc/kernel/smp.c
> > @@ -1185,10 +1185,34 @@ static inline void add_cpu_to_smallcore_masks(int
> > cpu)
> > }
> > }
> >
> > +int get_physical_package_id(int cpu)
> > +{
> > + int ppid = cpu_to_chip_id(cpu);
> > +
> > +#ifdef CONFIG_PPC_SPLPAR
> > + /*
> > +* If the platform is PowerNV or Guest on KVM, ibm,chip-id is
> > +* defined. Hence we would return the chip-id as the
> > +* get_physical_package_id.
> > +*/
> > + if (ppid == -1 && firmware_has_feature(FW_FEATURE_LPAR)) {
> > + struct device_node *np = of_get_cpu_node(cpu, NULL);
> > +
> > + if (np) {
> > + ppid = of_node_to_nid(np);
> > + of_node_put(np);
> > + }
> > + }
> > +#endif /* CONFIG_PPC_SPLPAR */
> > +
> > + return ppid;
> > +}
> > static void add_cpu_to_masks(int cpu)
> > {
> > int first_thread = cpu_first_thread_sibling(cpu);
> > - int chipid = cpu_to_chip_id(cpu);
> > + int ppid = get_physical_package_id(cpu);
> > int i;
> >
> > /*
> > @@ -1217,11 +1241,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 (ppid == -1)
> > return;
>
> Can get_physical_package_id() return -1 ?
>
Yes, it can return -1,
1. A System doesnt have CONFIG_PPC_SPLPAR: cpu_to_chip_id() might return -1.
2. A System with CONFIG_PPC_SPLPAR: Still can return -1 for following cases.
a. Not having firmware feature FW_FEATURE_LPAR
b. If for some reason, of_get_cpu_node property is not present.
> >
> > for_each_cpu(i, cpu_online_mask)
> > - if (cpu_to_chip_id(i) == chipid)
> > + if (get_physical_package_id(i) == ppid)
> > set_cpus_related(cpu, i, cpu_core_mask);
> > }
--
Thanks and Regards
Srikar Dronamraju
Re: [PATCH v4] powerpc/smp: Use nid as fallback for package_id
Hello Srikar,
On Wed, Jan 29, 2020 at 07:21:21PM +0530, Srikar Dronamraju wrote:
[..snip..]
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1185,10 +1185,34 @@ static inline void add_cpu_to_smallcore_masks(int cpu)
> }
> }
>
> +int get_physical_package_id(int cpu)
> +{
> + int ppid = cpu_to_chip_id(cpu);
> +
> +#ifdef CONFIG_PPC_SPLPAR
> + /*
> + * If the platform is PowerNV or Guest on KVM, ibm,chip-id is
> + * defined. Hence we would return the chip-id as the
> + * get_physical_package_id.
> + */
> + if (ppid == -1 && firmware_has_feature(FW_FEATURE_LPAR)) {
> + struct device_node *np = of_get_cpu_node(cpu, NULL);
> +
> + if (np) {
> + ppid = of_node_to_nid(np);
> + of_node_put(np);
> + }
> + }
> +#endif /* CONFIG_PPC_SPLPAR */
> +
> + return ppid;
> +}
> +EXPORT_SYMBOL_GPL(get_physical_package_id);
> +
> static void add_cpu_to_masks(int cpu)
> {
> int first_thread = cpu_first_thread_sibling(cpu);
> - int chipid = cpu_to_chip_id(cpu);
> + int ppid = get_physical_package_id(cpu);
> int i;
>
> /*
> @@ -1217,11 +1241,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 (ppid == -1)
> return;
Can get_physical_package_id() return -1 ?
>
> for_each_cpu(i, cpu_online_mask)
> - if (cpu_to_chip_id(i) == chipid)
> + if (get_physical_package_id(i) == ppid)
> set_cpus_related(cpu, i, cpu_core_mask);
> }
>
> --
> 2.18.1
>
--
Thanks and Regards
gautham.
