| Message ID | 20190822144235.19398-3-srikar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Early node associativity | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (0e4523c0b4f64eaf7abe59e143e6bdf8f972acff) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 1 checks, 57 lines checked |
Hi Srikar, Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes: > Currently the kernel detects if its running on a shared lpar platform > and requests home node associativity before the scheduler sched_domains > are setup. However between the time NUMA setup is initialized and the > request for home node associativity, workqueue initializes its per node > cpumask. The per node workqueue possible cpumask may turn invalid > after home node associativity resulting in weird situations like > workqueue possible cpumask being a subset of workqueue online cpumask. > > This can be fixed by requesting home node associativity earlier just > before NUMA setup. However at the NUMA setup time, kernel may not be in > a position to detect if its running on a shared lpar platform. So > request for home node associativity and if the request fails, fallback > on the device tree property. I think this is generally sound at the conceptual level. > However home node associativity requires cpu's hwid which is set in > smp_setup_pacas. Hence call smp_setup_pacas before numa_setup_cpus. But this seems like it would negatively affect pacas' NUMA placements? Would it be less risky to figure out a way to do "early" VPHN hcalls before mem_topology_setup, getting the hwids from the cpu_to_phys_id array perhaps? > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c > index 88b5157..7965d3b 100644 > --- a/arch/powerpc/mm/numa.c > +++ b/arch/powerpc/mm/numa.c > @@ -461,6 +461,21 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb) > return nid; > } > > +static int vphn_get_nid(unsigned long cpu) > +{ > + __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0}; > + long rc; > + > + /* Use associativity from first thread for all siblings */ I don't understand how this comment corresponds to the code it accompanies. > + rc = hcall_vphn(get_hard_smp_processor_id(cpu), > + VPHN_FLAG_VCPU, associativity); > + > + if (rc == H_SUCCESS) > + return associativity_to_nid(associativity); ^^ extra space > @@ -490,7 +505,18 @@ static int numa_setup_cpu(unsigned long lcpu) > goto out; > } > > - nid = of_node_to_nid_single(cpu); > + /* > + * On a shared lpar, the device tree might not have the correct node > + * associativity. At this time lppaca, or its __old_status field Sorry but I'm going to quibble with this phrasing a bit. On SPLPAR the CPU nodes have no affinity information in the device tree at all. This comment implies that they may have incorrect information, which is AFAIK not the case.
* Nathan Lynch <nathanl@linux.ibm.com> [2019-08-22 12:17:48]: > Hi Srikar, Thanks Nathan for the review. > > > However home node associativity requires cpu's hwid which is set in > > smp_setup_pacas. Hence call smp_setup_pacas before numa_setup_cpus. > > But this seems like it would negatively affect pacas' NUMA placements? > > Would it be less risky to figure out a way to do "early" VPHN hcalls > before mem_topology_setup, getting the hwids from the cpu_to_phys_id > array perhaps? > Do you mean for calls from mem_topology_setup(), stuff we use cpu_to_phys_id but for the calls from ppc_numa_cpu_prepare() we use the get_hard_smp_processor_id()? Thats doable. > > > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c > > index 88b5157..7965d3b 100644 > > --- a/arch/powerpc/mm/numa.c > > +++ b/arch/powerpc/mm/numa.c > > @@ -461,6 +461,21 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb) > > return nid; > > } > > > > +static int vphn_get_nid(unsigned long cpu) > > +{ > > + __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0}; > > + long rc; > > + > > + /* Use associativity from first thread for all siblings */ > > I don't understand how this comment corresponds to the code it > accompanies. Okay will rephrase > > > > + rc = hcall_vphn(get_hard_smp_processor_id(cpu), > > + VPHN_FLAG_VCPU, associativity); > > + > > + if (rc == H_SUCCESS) > > + return associativity_to_nid(associativity); > ^^ extra space > > > @@ -490,7 +505,18 @@ static int numa_setup_cpu(unsigned long lcpu) > > goto out; > > } > > > > - nid = of_node_to_nid_single(cpu); > > + /* > > + * On a shared lpar, the device tree might not have the correct node > > + * associativity. At this time lppaca, or its __old_status field > > Sorry but I'm going to quibble with this phrasing a bit. On SPLPAR the > CPU nodes have no affinity information in the device tree at all. This > comment implies that they may have incorrect information, which is > AFAIK not the case. > Okay will clarify.
Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes: > * Nathan Lynch <nathanl@linux.ibm.com> [2019-08-22 12:17:48]: >> > However home node associativity requires cpu's hwid which is set in >> > smp_setup_pacas. Hence call smp_setup_pacas before numa_setup_cpus. >> >> But this seems like it would negatively affect pacas' NUMA placements? >> >> Would it be less risky to figure out a way to do "early" VPHN hcalls >> before mem_topology_setup, getting the hwids from the cpu_to_phys_id >> array perhaps? >> > > Do you mean for calls from mem_topology_setup(), stuff we use cpu_to_phys_id > but for the calls from ppc_numa_cpu_prepare() we use the > get_hard_smp_processor_id()? Yes, something like that, I think. Although numa_setup_cpu() is used in both contexts.
On Thu, Aug 22, 2019 at 08:12:34PM +0530, Srikar Dronamraju wrote: > Currently the kernel detects if its running on a shared lpar platform > and requests home node associativity before the scheduler sched_domains > are setup. However between the time NUMA setup is initialized and the > request for home node associativity, workqueue initializes its per node > cpumask. The per node workqueue possible cpumask may turn invalid > after home node associativity resulting in weird situations like > workqueue possible cpumask being a subset of workqueue online cpumask. Tested this series on Power KVM guest and expected that it fixes https://github.com/linuxppc/issues/issues/167 but am able to see the below warning still while doing vcpu hotplug with numa nodes, Advise if am missing anything or this is not the intended series to fix above issue. Env: HW: Power8 Host/Guest Kernel: 5.3.0-rc5-00172-g13e3f1076e29 (linux master + this series) Qemu: 4.0.90 (v4.1.0-rc3) Guest Config: .. <vcpu placement='static' current='2'>4</vcpu> ... <kernel>/home/kvmci/linux/vmlinux</kernel> <cmdline>root=/dev/sda2 rw console=tty0 console=ttyS0,115200 init=/sbin/init initcall_debug numa=debug crashkernel=1024M selinux=0</cmdline> ... <topology sockets='1' cores='2' threads='2'/> <numa> <cell id='0' cpus='0-1' memory='2097152' unit='KiB'/> <cell id='1' cpus='2-3' memory='2097152' unit='KiB'/> </numa> Event: vcpu hotplug [root@atest-guest ~]# [ 41.447170] random: crng init done [ 41.448153] random: 7 urandom warning(s) missed due to ratelimiting [ 51.727256] VPHN hcall succeeded. Reset polling... [ 51.826301] adding cpu 2 to node 1 [ 51.856238] WARNING: workqueue cpumask: online intersect > possible intersect [ 51.916297] VPHN hcall succeeded. Reset polling... [ 52.036272] adding cpu 3 to node 1 Regards, -Satheesh. > > This can be fixed by requesting home node associativity earlier just > before NUMA setup. However at the NUMA setup time, kernel may not be in > a position to detect if its running on a shared lpar platform. So > request for home node associativity and if the request fails, fallback > on the device tree property. > > However home node associativity requires cpu's hwid which is set in > smp_setup_pacas. Hence call smp_setup_pacas before numa_setup_cpus. > > Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > Cc: Michael Ellerman <mpe@ellerman.id.au> > Cc: Nicholas Piggin <npiggin@gmail.com> > Cc: Nathan Lynch <nathanl@linux.ibm.com> > Cc: linuxppc-dev@lists.ozlabs.org > Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> > Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com> > --- > arch/powerpc/kernel/setup-common.c | 5 +++-- > arch/powerpc/mm/numa.c | 28 +++++++++++++++++++++++++++- > 2 files changed, 30 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c > index 1f8db66..9135dba 100644 > --- a/arch/powerpc/kernel/setup-common.c > +++ b/arch/powerpc/kernel/setup-common.c > @@ -888,6 +888,9 @@ void __init setup_arch(char **cmdline_p) > /* Check the SMT related command line arguments (ppc64). */ > check_smt_enabled(); > > +#ifdef CONFIG_SMP > + smp_setup_pacas(); > +#endif > /* Parse memory topology */ > mem_topology_setup(); > > @@ -899,8 +902,6 @@ void __init setup_arch(char **cmdline_p) > * so smp_release_cpus() does nothing for them. > */ > #ifdef CONFIG_SMP > - smp_setup_pacas(); > - > /* On BookE, setup per-core TLB data structures. */ > setup_tlb_core_data(); > > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c > index 88b5157..7965d3b 100644 > --- a/arch/powerpc/mm/numa.c > +++ b/arch/powerpc/mm/numa.c > @@ -461,6 +461,21 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb) > return nid; > } > > +static int vphn_get_nid(unsigned long cpu) > +{ > + __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0}; > + long rc; > + > + /* Use associativity from first thread for all siblings */ > + rc = hcall_vphn(get_hard_smp_processor_id(cpu), > + VPHN_FLAG_VCPU, associativity); > + > + if (rc == H_SUCCESS) > + return associativity_to_nid(associativity); > + > + return NUMA_NO_NODE; > +} > + > /* > * Figure out to which domain a cpu belongs and stick it there. > * Return the id of the domain used. > @@ -490,7 +505,18 @@ static int numa_setup_cpu(unsigned long lcpu) > goto out; > } > > - nid = of_node_to_nid_single(cpu); > + /* > + * On a shared lpar, the device tree might not have the correct node > + * associativity. At this time lppaca, or its __old_status field > + * may not be updated. Hence request an explicit associativity > + * irrespective of whether the lpar is shared or dedicated. Use the > + * device tree property as a fallback. > + */ > + if (firmware_has_feature(FW_FEATURE_VPHN)) > + nid = vphn_get_nid(lcpu); > + > + if (nid == NUMA_NO_NODE) > + nid = of_node_to_nid_single(cpu); > > out_present: > if (nid < 0 || !node_possible(nid)) > -- > 1.8.3.1 >
Hi Satheesh, > > Currently the kernel detects if its running on a shared lpar platform > > and requests home node associativity before the scheduler sched_domains > > are setup. However between the time NUMA setup is initialized and the > > request for home node associativity, workqueue initializes its per node > > cpumask. The per node workqueue possible cpumask may turn invalid > > after home node associativity resulting in weird situations like > > workqueue possible cpumask being a subset of workqueue online cpumask. > > Env: > HW: Power8 > Host/Guest Kernel: 5.3.0-rc5-00172-g13e3f1076e29 (linux master + this series) > Qemu: 4.0.90 (v4.1.0-rc3) > > Guest Config: > .. > <vcpu placement='static' current='2'>4</vcpu> > ... > <kernel>/home/kvmci/linux/vmlinux</kernel> > <cmdline>root=/dev/sda2 rw console=tty0 console=ttyS0,115200 init=/sbin/init initcall_debug numa=debug crashkernel=1024M selinux=0</cmdline> > ... > <topology sockets='1' cores='2' threads='2'/> > <numa> > <cell id='0' cpus='0-1' memory='2097152' unit='KiB'/> > <cell id='1' cpus='2-3' memory='2097152' unit='KiB'/> > </numa> > > Event: > vcpu hotplug > > [root@atest-guest ~]# [ 41.447170] random: crng init done > [ 41.448153] random: 7 urandom warning(s) missed due to ratelimiting > [ 51.727256] VPHN hcall succeeded. Reset polling... > [ 51.826301] adding cpu 2 to node 1 > [ 51.856238] WARNING: workqueue cpumask: online intersect > possible intersect > [ 51.916297] VPHN hcall succeeded. Reset polling... > [ 52.036272] adding cpu 3 to node 1 > Thanks for testing. The fix for this patch series was to make sure per node workqueue possible cpus is updated correctly at boot. However Node hotplug on KVM guests and dlpar on PowerVM lpars aren't covered by this patch series. On systems that support shared processor, the associativity of the possible cpus is not known at boot time. Hence we will not be able to update the per node workquque possible cpumask.
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index 1f8db66..9135dba 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -888,6 +888,9 @@ void __init setup_arch(char **cmdline_p) /* Check the SMT related command line arguments (ppc64). */ check_smt_enabled(); +#ifdef CONFIG_SMP + smp_setup_pacas(); +#endif /* Parse memory topology */ mem_topology_setup(); @@ -899,8 +902,6 @@ void __init setup_arch(char **cmdline_p) * so smp_release_cpus() does nothing for them. */ #ifdef CONFIG_SMP - smp_setup_pacas(); - /* On BookE, setup per-core TLB data structures. */ setup_tlb_core_data(); diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c index 88b5157..7965d3b 100644 --- a/arch/powerpc/mm/numa.c +++ b/arch/powerpc/mm/numa.c @@ -461,6 +461,21 @@ static int of_drconf_to_nid_single(struct drmem_lmb *lmb) return nid; } +static int vphn_get_nid(unsigned long cpu) +{ + __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0}; + long rc; + + /* Use associativity from first thread for all siblings */ + rc = hcall_vphn(get_hard_smp_processor_id(cpu), + VPHN_FLAG_VCPU, associativity); + + if (rc == H_SUCCESS) + return associativity_to_nid(associativity); + + return NUMA_NO_NODE; +} + /* * Figure out to which domain a cpu belongs and stick it there. * Return the id of the domain used. @@ -490,7 +505,18 @@ static int numa_setup_cpu(unsigned long lcpu) goto out; } - nid = of_node_to_nid_single(cpu); + /* + * On a shared lpar, the device tree might not have the correct node + * associativity. At this time lppaca, or its __old_status field + * may not be updated. Hence request an explicit associativity + * irrespective of whether the lpar is shared or dedicated. Use the + * device tree property as a fallback. + */ + if (firmware_has_feature(FW_FEATURE_VPHN)) + nid = vphn_get_nid(lcpu); + + if (nid == NUMA_NO_NODE) + nid = of_node_to_nid_single(cpu); out_present: if (nid < 0 || !node_possible(nid))
Currently the kernel detects if its running on a shared lpar platform and requests home node associativity before the scheduler sched_domains are setup. However between the time NUMA setup is initialized and the request for home node associativity, workqueue initializes its per node cpumask. The per node workqueue possible cpumask may turn invalid after home node associativity resulting in weird situations like workqueue possible cpumask being a subset of workqueue online cpumask. This can be fixed by requesting home node associativity earlier just before NUMA setup. However at the NUMA setup time, kernel may not be in a position to detect if its running on a shared lpar platform. So request for home node associativity and if the request fails, fallback on the device tree property. However home node associativity requires cpu's hwid which is set in smp_setup_pacas. Hence call smp_setup_pacas before numa_setup_cpus. Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Nicholas Piggin <npiggin@gmail.com> Cc: Nathan Lynch <nathanl@linux.ibm.com> Cc: linuxppc-dev@lists.ozlabs.org Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com> Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com> --- arch/powerpc/kernel/setup-common.c | 5 +++-- arch/powerpc/mm/numa.c | 28 +++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-)