Re: [PATCH V7 1/3] powerpc/nodes: Ensure enough nodes avail for operations
Nathan Fontenot writes: > On 11/16/2017 11:24 AM, Michael Bringmann wrote: ... >> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c >> index eb604b3..334a1ff 100644 >> --- a/arch/powerpc/mm/numa.c >> +++ b/arch/powerpc/mm/numa.c >> @@ -892,6 +892,37 @@ static void __init setup_node_data(int nid, u64 >> start_pfn, u64 end_pfn) >> NODE_DATA(nid)->node_spanned_pages = spanned_pages; >> } >> >> +static void __init find_possible_nodes(void) >> +{ >> +struct device_node *rtas; >> +u32 numnodes, i; >> + >> +if (min_common_depth <= 0) >> +return; >> + >> +rtas = of_find_node_by_path("/rtas"); >> +if (!rtas) >> +return; >> + >> +if (of_property_read_u32_index(rtas, >> +"ibm,max-associativity-domains", >> +min_common_depth, &numnodes)) >> +goto out; >> + >> +pr_info("numa: Nodes = %d (mcd = %d)\n", numnodes, >> +min_common_depth); > > numa.c already has a pr_fmt define, no need to pre-pend "numa:" to the > information message. And in fact no need to print that out here at all, it's covered elsewhere. So just drop that pr_info() entirely. cheers
Re: [PATCH V7 1/3] powerpc/nodes: Ensure enough nodes avail for operations
See below. On 11/20/2017 10:33 AM, Nathan Fontenot wrote: > > > On 11/16/2017 11:24 AM, Michael Bringmann wrote: >> On powerpc systems which allow 'hot-add' of CPU or memory resources, >> it may occur that the new resources are to be inserted into nodes >> that were not used for these resources at bootup. In the kernel, >> any node that is used must be defined and initialized. These empty >> nodes may occur when, >> >> * Dedicated vs. shared resources. Shared resources require >> information such as the VPHN hcall for CPU assignment to nodes. >> Associativity decisions made based on dedicated resource rules, >> such as associativity properties in the device tree, may vary >> from decisions made using the values returned by the VPHN hcall. >> * memoryless nodes at boot. Nodes need to be defined as 'possible' >> at boot for operation with other code modules. Previously, the >> powerpc code would limit the set of possible nodes to those which >> have memory assigned at boot, and were thus online. Subsequent >> add/remove of CPUs or memory would only work with this subset of >> possible nodes. >> * memoryless nodes with CPUs at boot. Due to the previous restriction >> on nodes, nodes that had CPUs but no memory were being collapsed >> into other nodes that did have memory at boot. In practice this >> meant that the node assignment presented by the runtime kernel >> differed from the affinity and associativity attributes presented >> by the device tree or VPHN hcalls. Nodes that might be known to >> the pHyp were not 'possible' in the runtime kernel because they did >> not have memory at boot. >> >> This patch ensures that sufficient nodes are defined to support >> configuration requirements after boot, as well as at boot. This >> patch set fixes a couple of problems. >> >> * Nodes known to powerpc to be memoryless at boot, but to have >> CPUs in them are allowed to be 'possible' and 'online'. Memory >> allocations for those nodes are taken from another node that does >> have memory until and if memory is hot-added to the node. >> * Nodes which have no resources assigned at boot, but which may still >> be referenced subsequently by affinity or associativity attributes, >> are kept in the list of 'possible' nodes for powerpc. Hot-add of >> memory or CPUs to the system can reference these nodes and bring >> them online instead of redirecting to one of the set of nodes that >> were known to have memory at boot. >> >> This patch extracts the value of the lowest domain level (number of >> allocable resources) from the device tree property >> "ibm,max-associativity-domains" to use as the maximum number of nodes >> to setup as possibly available in the system. This new setting will >> override the instruction, >> >> nodes_and(node_possible_map, node_possible_map, node_online_map); >> >> presently seen in the function arch/powerpc/mm/numa.c:initmem_init(). >> >> If the "ibm,max-associativity-domains" property is not present at boot, >> no operation will be performed to define or enable additional nodes, or >> enable the above 'nodes_and()'. >> >> Signed-off-by: Michael Bringmann >> --- >> Changes in V6: >> -- Remove some node initialization/allocation from boot setup >> to later in runtime to try to limit memory needs early on >> -- Augment descriptive documentation for patch >> --- >> arch/powerpc/mm/numa.c | 40 +--- >> 1 file changed, 37 insertions(+), 3 deletions(-) >> >> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c >> index eb604b3..334a1ff 100644 >> --- a/arch/powerpc/mm/numa.c >> +++ b/arch/powerpc/mm/numa.c >> @@ -892,6 +892,37 @@ static void __init setup_node_data(int nid, u64 >> start_pfn, u64 end_pfn) >> NODE_DATA(nid)->node_spanned_pages = spanned_pages; >> } >> >> +static void __init find_possible_nodes(void) >> +{ >> +struct device_node *rtas; >> +u32 numnodes, i; >> + >> +if (min_common_depth <= 0) >> +return; >> + >> +rtas = of_find_node_by_path("/rtas"); >> +if (!rtas) >> +return; >> + >> +if (of_property_read_u32_index(rtas, >> +"ibm,max-associativity-domains", >> +min_common_depth, &numnodes)) >> +goto out; >> + >> +pr_info("numa: Nodes = %d (mcd = %d)\n", numnodes, >> +min_common_depth); > > numa.c already has a pr_fmt define, no need to pre-pend "numa:" to the > information message. > > -Nathan Okay. > >> + >> +for (i = 0; i < numnodes; i++) { >> +if (!node_possible(i)) { >> +setup_node_data(i, 0, 0); >> +node_set(i, node_possible_map); >> +} >> +} >> + >> +out: >> +of_node_put(rtas); >> +} >> + >> void __init initmem_init(void) >> { >> int nid, cpu; >> @@ -905,12 +936,15 @@ void __init initmem_init(void) >> memblock_dump_all(); >> >>
Re: [PATCH V7 1/3] powerpc/nodes: Ensure enough nodes avail for operations
See below. On 11/22/2017 05:17 AM, Michael Ellerman wrote: > Nathan Fontenot writes: >> On 11/16/2017 11:24 AM, Michael Bringmann wrote: > ... >>> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c >>> index eb604b3..334a1ff 100644 >>> --- a/arch/powerpc/mm/numa.c >>> +++ b/arch/powerpc/mm/numa.c >>> @@ -892,6 +892,37 @@ static void __init setup_node_data(int nid, u64 >>> start_pfn, u64 end_pfn) >>> NODE_DATA(nid)->node_spanned_pages = spanned_pages; >>> } >>> >>> +static void __init find_possible_nodes(void) >>> +{ >>> + struct device_node *rtas; >>> + u32 numnodes, i; >>> + >>> + if (min_common_depth <= 0) >>> + return; >>> + >>> + rtas = of_find_node_by_path("/rtas"); >>> + if (!rtas) >>> + return; >>> + >>> + if (of_property_read_u32_index(rtas, >>> + "ibm,max-associativity-domains", >>> + min_common_depth, &numnodes)) >>> + goto out; >>> + >>> + pr_info("numa: Nodes = %d (mcd = %d)\n", numnodes, >>> + min_common_depth); >> >> numa.c already has a pr_fmt define, no need to pre-pend "numa:" to the >> information message. > > And in fact no need to print that out here at all, it's covered > elsewhere. So just drop that pr_info() entirely. > > cheers > > Okay. pr_info() removed. -- Michael W. Bringmann Linux Technology Center IBM Corporation Tie-Line 363-5196 External: (512) 286-5196 Cell: (512) 466-0650 m...@linux.vnet.ibm.com
Re: [PATCH V7 1/3] powerpc/nodes: Ensure enough nodes avail for operations
On 11/16/2017 11:24 AM, Michael Bringmann wrote: > On powerpc systems which allow 'hot-add' of CPU or memory resources, > it may occur that the new resources are to be inserted into nodes > that were not used for these resources at bootup. In the kernel, > any node that is used must be defined and initialized. These empty > nodes may occur when, > > * Dedicated vs. shared resources. Shared resources require > information such as the VPHN hcall for CPU assignment to nodes. > Associativity decisions made based on dedicated resource rules, > such as associativity properties in the device tree, may vary > from decisions made using the values returned by the VPHN hcall. > * memoryless nodes at boot. Nodes need to be defined as 'possible' > at boot for operation with other code modules. Previously, the > powerpc code would limit the set of possible nodes to those which > have memory assigned at boot, and were thus online. Subsequent > add/remove of CPUs or memory would only work with this subset of > possible nodes. > * memoryless nodes with CPUs at boot. Due to the previous restriction > on nodes, nodes that had CPUs but no memory were being collapsed > into other nodes that did have memory at boot. In practice this > meant that the node assignment presented by the runtime kernel > differed from the affinity and associativity attributes presented > by the device tree or VPHN hcalls. Nodes that might be known to > the pHyp were not 'possible' in the runtime kernel because they did > not have memory at boot. > > This patch ensures that sufficient nodes are defined to support > configuration requirements after boot, as well as at boot. This > patch set fixes a couple of problems. > > * Nodes known to powerpc to be memoryless at boot, but to have > CPUs in them are allowed to be 'possible' and 'online'. Memory > allocations for those nodes are taken from another node that does > have memory until and if memory is hot-added to the node. > * Nodes which have no resources assigned at boot, but which may still > be referenced subsequently by affinity or associativity attributes, > are kept in the list of 'possible' nodes for powerpc. Hot-add of > memory or CPUs to the system can reference these nodes and bring > them online instead of redirecting to one of the set of nodes that > were known to have memory at boot. > > This patch extracts the value of the lowest domain level (number of > allocable resources) from the device tree property > "ibm,max-associativity-domains" to use as the maximum number of nodes > to setup as possibly available in the system. This new setting will > override the instruction, > > nodes_and(node_possible_map, node_possible_map, node_online_map); > > presently seen in the function arch/powerpc/mm/numa.c:initmem_init(). > > If the "ibm,max-associativity-domains" property is not present at boot, > no operation will be performed to define or enable additional nodes, or > enable the above 'nodes_and()'. > > Signed-off-by: Michael Bringmann > --- > Changes in V6: > -- Remove some node initialization/allocation from boot setup > to later in runtime to try to limit memory needs early on > -- Augment descriptive documentation for patch > --- > arch/powerpc/mm/numa.c | 40 +--- > 1 file changed, 37 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c > index eb604b3..334a1ff 100644 > --- a/arch/powerpc/mm/numa.c > +++ b/arch/powerpc/mm/numa.c > @@ -892,6 +892,37 @@ static void __init setup_node_data(int nid, u64 > start_pfn, u64 end_pfn) > NODE_DATA(nid)->node_spanned_pages = spanned_pages; > } > > +static void __init find_possible_nodes(void) > +{ > + struct device_node *rtas; > + u32 numnodes, i; > + > + if (min_common_depth <= 0) > + return; > + > + rtas = of_find_node_by_path("/rtas"); > + if (!rtas) > + return; > + > + if (of_property_read_u32_index(rtas, > + "ibm,max-associativity-domains", > + min_common_depth, &numnodes)) > + goto out; > + > + pr_info("numa: Nodes = %d (mcd = %d)\n", numnodes, > + min_common_depth); numa.c already has a pr_fmt define, no need to pre-pend "numa:" to the information message. -Nathan > + > + for (i = 0; i < numnodes; i++) { > + if (!node_possible(i)) { > + setup_node_data(i, 0, 0); > + node_set(i, node_possible_map); > + } > + } > + > +out: > + of_node_put(rtas); > +} > + > void __init initmem_init(void) > { > int nid, cpu; > @@ -905,12 +936,15 @@ void __init initmem_init(void) > memblock_dump_all(); > > /* > - * Reduce the possible NUMA nodes to the online NUMA nodes, > - * since we do not support node hotplug. This ensures that we > - * lower the