Messages in this thread | | | Subject | Re: [PATCH 2/2] powerpc/hotplug: Ensure nodes initialized for hotplug | From | Michael Bringmann <> | Date | Tue, 17 Oct 2017 10:08:51 -0500 |
| |
On 10/16/2017 07:54 AM, Michael Ellerman wrote: > Michael Bringmann <mwb@linux.vnet.ibm.com> writes: > >> powerpc/hotplug: On systems like PowerPC which allow 'hot-add' of CPU, >> it may occur that the new resources are to be inserted into nodes >> that were not used for memory resources at bootup. Many different >> configurations of PowerPC resources may need to be supported depending >> upon the environment. > > Give me some detail please?!
Configurations that demonstrated problems included 'memoryless' nodes that possessed only CPUs at boot, and nodes that contained neither CPUs nor memory at boot. The calculations in the kernel resulted in a different node use layout on many SAP HANA configured systems.
> >> This patch fixes some problems encountered at > > What problems?
The previous implementation collapsed all node assignments after affinity calculations to use only those nodes that had memory at boot. This resulted in calculation and configuration differences between the FSP code and the Linux kernel.
> >> runtime with configurations that support memory-less nodes, but which >> allow CPUs to be added at and after boot. > > How does it fix those problems?
The change involves completing the initialization of nodes that were not used at boot, but which were selected by VPHN affinity calculations during subsequent hotplug operations.
Michael
> >> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c >> index b385cd0..e811dd1 100644 >> --- a/arch/powerpc/mm/numa.c >> +++ b/arch/powerpc/mm/numa.c >> @@ -1325,6 +1325,17 @@ static long vphn_get_associativity(unsigned long cpu, >> return rc; >> } >> >> +static int verify_node_preparation(int nid) >> +{ > > I would not expect a function called "verify" ... > >> + if ((NODE_DATA(nid) == NULL) || >> + (NODE_DATA(nid)->node_spanned_pages == 0)) { >> + if (try_online_node(nid)) > > .. to do something like online a node. > >> + return first_online_node; >> + } >> + >> + return nid; >> +} >> + >> /* >> * Update the CPU maps and sysfs entries for a single CPU when its NUMA >> * characteristics change. This function doesn't perform any locking and is >> @@ -1433,9 +1444,11 @@ int numa_update_cpu_topology(bool cpus_locked) >> /* Use associativity from first thread for all siblings */ >> vphn_get_associativity(cpu, associativity); >> new_nid = associativity_to_nid(associativity); >> - if (new_nid < 0 || !node_online(new_nid)) >> + if (new_nid < 0 || !node_possible(new_nid)) >> new_nid = first_online_node; >> >> + new_nid = verify_node_preparation(new_nid); > > You're being called part-way through CPU hotplug here, are we sure it's > safe to go and do memory hotplug from there? What's the locking > situation? > > cheers > >
-- Michael W. Bringmann Linux Technology Center IBM Corporation Tie-Line 363-5196 External: (512) 286-5196 Cell: (512) 466-0650 mwb@linux.vnet.ibm.com
| |