Re: [PATCH 1/2] mm/memblock: use NUMA_NO_NODE instead of MAX_NUMNODES as default node_id

[Wei Yang]

Hello, everyone,

By deeper thinking, I am willing to split these two patches into two patch
set, since they are trying to address two different things.

The first one [Patch 1] is trying to use NUMA_NO_NODE as the default node_id in
memblock_region.

Current implementation use MAX_NUMNODES as the default nid in several
situations:

    * when it adds a range from e820 to memblock 
    * when it returns an allocated range, it sets nid to MAX_NUMNODES 
    * on x86 before initialize the numa info, it set all nid to MAX_NUMNODES

The usage of MAX_NUMNODES here is not accurate, and NUMA_NO_NODE should be
used here.

When looking at the allocation procedure of memblock, it translate
MAX_NUMNODES to NUMA_NO_NODE and mentioned MAX_NUMNODES is deprecated. So I
think it is reasonable to do this refactor here.

The second one [Patch 2] is trying to address similar issue in
for_each_mem_pfn_range(). The patch here is the first step. I have searched
out all related functions and relpaces MAX_NUMNODES with NUMA_NO_NODE. While
the warning here will still be seen when just this patch applies. While after
all patches applied, we won't see the warning again.

Hmm... it looks like some dirty work, while I still think it worth the efforts
to use the correct macro.

Willing to get some feedback :-)


On Fri, Jan 27, 2017 at 09:59:21AM +0800, Wei Yang wrote:
>According to commit <b115423357e0> ('mm/memblock: switch to use
>NUMA_NO_NODE instead of MAX_NUMNODES'), MAX_NUMNODES is not preferred as an
>node_id indicator.
>
>This patch use NUMA_NO_NODE as the default node_id for memblock.
>
>Signed-off-by: Wei Yang <richard.weiyang@xxxxxxxxx>
>---
> arch/x86/mm/numa.c | 6 +++---
> mm/memblock.c      | 8 ++++----
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
>index 3f35b48d1d9d..4366242356c5 100644
>--- a/arch/x86/mm/numa.c
>+++ b/arch/x86/mm/numa.c
>@@ -506,7 +506,7 @@ static void __init numa_clear_kernel_node_hotplug(void)
> 	 *   reserve specific pages for Sandy Bridge graphics. ]
> 	 */
> 	for_each_memblock(reserved, mb_region) {
>-		if (mb_region->nid != MAX_NUMNODES)
>+		if (mb_region->nid != NUMA_NO_NODE)
> 			node_set(mb_region->nid, reserved_nodemask);
> 	}
> 
>@@ -633,9 +633,9 @@ static int __init numa_init(int (*init_func)(void))
> 	nodes_clear(node_online_map);
> 	memset(&numa_meminfo, 0, sizeof(numa_meminfo));
> 	WARN_ON(memblock_set_node(0, ULLONG_MAX, &memblock.memory,
>-				  MAX_NUMNODES));
>+				  NUMA_NO_NODE));
> 	WARN_ON(memblock_set_node(0, ULLONG_MAX, &memblock.reserved,
>-				  MAX_NUMNODES));
>+				  NUMA_NO_NODE));
> 	/* In case that parsing SRAT failed. */
> 	WARN_ON(memblock_clear_hotplug(0, ULLONG_MAX));
> 	numa_reset_distance();
>diff --git a/mm/memblock.c b/mm/memblock.c
>index d0f2c9632187..7d27566cee11 100644
>--- a/mm/memblock.c
>+++ b/mm/memblock.c
>@@ -292,7 +292,7 @@ static void __init_memblock memblock_remove_region(struct memblock_type *type, u
> 		type->regions[0].base = 0;
> 		type->regions[0].size = 0;
> 		type->regions[0].flags = 0;
>-		memblock_set_region_node(&type->regions[0], MAX_NUMNODES);
>+		memblock_set_region_node(&type->regions[0], NUMA_NO_NODE);
> 	}
> }
> 
>@@ -616,7 +616,7 @@ int __init_memblock memblock_add(phys_addr_t base, phys_addr_t size)
> 		     (unsigned long long)base + size - 1,
> 		     0UL, (void *)_RET_IP_);
> 
>-	return memblock_add_range(&memblock.memory, base, size, MAX_NUMNODES, 0);
>+	return memblock_add_range(&memblock.memory, base, size, NUMA_NO_NODE, 0);
> }
> 
> /**
>@@ -734,7 +734,7 @@ int __init_memblock memblock_reserve(phys_addr_t base, phys_addr_t size)
> 		     (unsigned long long)base + size - 1,
> 		     0UL, (void *)_RET_IP_);
> 
>-	return memblock_add_range(&memblock.reserved, base, size, MAX_NUMNODES, 0);
>+	return memblock_add_range(&memblock.reserved, base, size, NUMA_NO_NODE, 0);
> }
> 
> /**
>@@ -1684,7 +1684,7 @@ static void __init_memblock memblock_dump(struct memblock_type *type, char *name
> 		size = rgn->size;
> 		flags = rgn->flags;
> #ifdef CONFIG_HAVE_MEMBLOCK_NODE_MAP
>-		if (memblock_get_region_node(rgn) != MAX_NUMNODES)
>+		if (memblock_get_region_node(rgn) != NUMA_NO_NODE)
> 			snprintf(nid_buf, sizeof(nid_buf), " on node %d",
> 				 memblock_get_region_node(rgn));
> #endif
>-- 
>2.11.0

-- 
Wei Yang
Help you, Help me

Attachment: signature.asc
Description: PGP signature