Re: [Patch 2/2]: powerpc/hotplug/mm: Fix hot-add memory node assoc

[Balbir Singh <bsingharora@gmail.com>]

On Thu, 2017-06-01 at 16:33 -0500, Reza Arbab wrote:
> On Thu, Jun 01, 2017 at 07:36:31PM +1000, Michael Ellerman wrote:
> > I don't think that's what the patch does. It just marks 32 (!?) nodes 
> > as online. Or if you're talking about reverting 3af229f2071f that 
> > leaves you with 256 possible nodes. Both of which are wasteful.
> 
> To be clear, with Balbir's set the latter is no longer wasteful.
> 
> > The right fix is to make sure any nodes which are present at boot 
> > remain in the possible map, even if they don't have memory/CPUs 
> > assigned at boot.
> 
> I'm still hoping 3af229f2071f could indeed be reverted some day, but 
> until then the following would follow your suggestion for our GPU nodes.  
> What do you think?
> 
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -895,6 +895,7 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
>  void __init initmem_init(void)
>  {
>  	int nid, cpu;
> +	struct device_node *dn;
>  
>  	max_low_pfn = memblock_end_of_DRAM() >> PAGE_SHIFT;
>  	max_pfn = max_low_pfn;
> @@ -911,6 +912,18 @@ void __init initmem_init(void)
>  	 */
>  	nodes_and(node_possible_map, node_possible_map, node_online_map);
>  
> +	/*
> +	 * Consider an ibm,coherent-device-memory node possible. Even though
> +	 * it is not online at boot, it may be hotplugged later.
> +	 */
> +	for_each_compatible_node(dn, NULL, "ibm,coherent-device-memory") {
> +		nid = of_node_to_nid_single(dn);
> +		if (nid < 0)
> +			continue;
> +
> +		node_set(nid, node_possible_map);
> +	}
> +

I think it looks reasonable, although I'd really like to set a limit
in firmware on the number of nodes and fix memcg hotplug correctly in
the medium term.

Balbir Singh.

>  	for_each_online_node(nid) {
>  		unsigned long start_pfn, end_pfn;
>  
>