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

[Michael Ellerman]

Reza Arbab <arbab@xxxxxxxxxxxxxxxxxx> writes:

> 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.

AFAIK that series has been rejected.

And even so, it only fixes one known case where having a very large
possible map is wasteful, there could be others we haven't found, or
there could be more introduced in future.

So we will always prefer to keep the possible map as small as it can be.

>>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?

Yeah we can do something like that.

We might want to come up with a generic property, rather than looking
specifically for ibm,coherent-device-memory.

And we might also need a way for skiboot to tell us "in addition to
what is in the device tree at boot, up to N more nodes could be hot
plugged".

cheers

> --- 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);
> +	}
> +
>  	for_each_online_node(nid) {
>  		unsigned long start_pfn, end_pfn;
>  
>
> -- 
> Reza Arbab