x86, memblock: Allocate memory near kernel image before SRAT parsed

From: Tang Chen <[email protected]>

This patch imports a new function __memblock_find_range_rev

to factor out of top-down allocation from memblock_find_in_range_node. This is a preparation because we will introduce a new bottom-up allocation mode in the following patch.

Signed-off-by Tang Chen <[email protected]>

Signed-off-by Zhang Yanfei <[email protected]>

Quoted 3 lines · Zhang Yanfei

From: Tang Chen <[email protected]>

This patch imports a new function __memblock_find_range_rev

"imports" sounds a bit weird. I think "creates" or "this patch factors out __memblock_find_range_rev() from memblock_find_in_range_node()" would be better.

Quoted 7 lines

to factor out of top-down allocation from memblock_find_in_range_node. This is a preparation because we will introduce a new bottom-up allocation mode in the following patch.

Signed-off-by Tang Chen <[email protected]>

Signed-off-by Zhang Yanfei <[email protected]>

Acked-by Tejun Heo <[email protected]>

Thanks.

include/linux/memblock.h +7 -0

+/* Allocation direction */
+enum {
+	MEMBLOCK_DIRECTION_TOP_DOWN,
+	MEMBLOCK_DIRECTION_BOTTOM_UP,
+	NR_MEMLBOCK_DIRECTIONS
+};
+
 struct memblock_region {
 	phys_addr_t base;
 	phys_addr_t size;

mm/memblock.c +13 -0

+ *
  * RETURNS:
  * Found address on success, %0 on failure.
  */

@@ -134,6 +173,8 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
 					phys_addr_t end, phys_addr_t size,
 					phys_addr_t align, int nid)
 {
+	int ret;
+
 	/* pump up @end */
 	if (end == MEMBLOCK_ALLOC_ACCESSIBLE)
 		end = memblock.current_limit;

@@ -142,6 +183,28 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
 	start = max_t(phys_addr_t, start, PAGE_SIZE);
 	end = max(start, end);
 
+	if (memblock_bottom_up()) {
+		phys_addr_t bottom_up_start;
+
+		/* make sure we will allocate above the kernel */
+		bottom_up_start = max_t(phys_addr_t, start, __pa_symbol(_end));
+
+		/* ok, try bottom-up allocation first */
+		ret = __memblock_find_range(bottom_up_start, end,
+					    size, align, nid);
+		if (ret)

Quoted 13 lines · Zhang Yanfei

include/linux/memblock.h +7 -0

+/* Allocation direction */
+enum {
+	MEMBLOCK_DIRECTION_TOP_DOWN,
+	MEMBLOCK_DIRECTION_BOTTOM_UP,
+	NR_MEMLBOCK_DIRECTIONS
+};
+
 struct memblock_region {
 	phys_addr_t base;
 	phys_addr_t size;

@@ -35,6 +42,7 @@ struct memblock_type {
 };
 

include/linux/memblock.h struct memblock_type { · line 45

Quoted 2 lines

include/linux/memblock.h +1 -0

 struct memblock {
+	int current_direction;  /* current allocation direction */

Just use boolean bottom_up here too? No need for the constants.

mm/memblock.c line 24

Quoted 7 lines

mm/memblock.c +2 -0

@@ -20,6 +20,8 @@
 #include <linux/seq_file.h>
 #include <linux/memblock.h>
 
+#include <asm-generic/sections.h>
+

Why is the above added by this patch?

mm/memblock.c static long __init_memblock memblock_overlaps_region(struct memblock_type *type, · line 99

Quoted 13 lines

mm/memblock.c +11 -0

 /**
+ * __memblock_find_range - find free area utility
+ * @start: start of candidate range
+ * @end: end of candidate range, can be %MEMBLOCK_ALLOC_{ANYWHERE|ACCESSIBLE}
+ * @size: size of free area to find
+ * @align: alignment of free area to find
+ * @nid: nid of the free area to find, %MAX_NUMNODES for any node
+ *
+ * Utility called from memblock_find_in_range_node(), find free area bottom-up.
+ *
+ * RETURNS:
+ * Found address on success, %0 on failure.

I don't think we prefix numeric literals with %.

...

mm/memblock.c __memblock_find_range_rev(phys_addr_t start, phys_addr_t end, · line 167

Quoted 7 lines

mm/memblock.c +3 -0

@@ -127,6 +162,10 @@ __memblock_find_range_rev(phys_addr_t start, phys_addr_t end,
  *
  * Find @size free area aligned to @align in the specified range and node.
  *
+ * When allocation direction is bottom-up, the @start should be greater
+ * than the end of the kernel image. Otherwise, it will be trimmed. And also,
+ * if bottom-up allocation failed, will try to allocate memory top-down.

It'd be nice to explain that bottom-up allocation is limited to above kernel image and what it's used for here.

Quoted 32 lines

mm/memblock.c +17 -0

+ *
  * RETURNS:
  * Found address on success, %0 on failure.
  */

@@ -134,6 +173,8 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
 					phys_addr_t end, phys_addr_t size,
 					phys_addr_t align, int nid)
 {
+	int ret;
+
 	/* pump up @end */
 	if (end == MEMBLOCK_ALLOC_ACCESSIBLE)
 		end = memblock.current_limit;

@@ -142,6 +183,28 @@ phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start,
 	start = max_t(phys_addr_t, start, PAGE_SIZE);
 	end = max(start, end);
 
+	if (memblock_bottom_up()) {
+		phys_addr_t bottom_up_start;
+
+		/* make sure we will allocate above the kernel */
+		bottom_up_start = max_t(phys_addr_t, start, __pa_symbol(_end));
+
+		/* ok, try bottom-up allocation first */
+		ret = __memblock_find_range(bottom_up_start, end,
+					    size, align, nid);
+		if (ret)
+			return ret;
+
+		/*
+		 * we always limit bottom-up allocation above the kernel,

mm/memblock.c phys_addr_t __init_memblock memblock_find_in_range_node(phys_addr_t start, · line 205

Quoted 6 lines

mm/memblock.c +6 -0

+		 * but top-down allocation doesn't have the limit, so
+		 * retrying top-down allocation may succeed when bottom-up
+		 * allocation failed.
+		 */
+		pr_warn("memblock: Failed to allocate memory in bottom up "
+			"direction. Now try top down direction.\n");

Maybe just print warning only on the first failure?

Otherwise, looks good to me.

Thanks.

Quoted 1 line

Maybe just print warning only on the first failure?

Hmmm... This message is for each memblock allocation, that said, if the allocation this time fails, it prints the message and we use so called top-down. But next time, we still use bottom up first again.

Did you mean if we fail on one bottom-up allocation, then we never try bottom-up again and will always use top-down?

Quoted 6 lines · Zhang Yanfei

Quoted 1 line

Maybe just print warning only on the first failure?

Hmmm... This message is for each memblock allocation, that said, if the allocation this time fails, it prints the message and we use so called top-down. But next time, we still use bottom up first again.

Quoted 2 lines

Did you mean if we fail on one bottom-up allocation, then we never try bottom-up again and will always use top-down?

Nope, it's just that it might end up printing something for each alloc which can end up flooding console / log. The first failure is the most interesting and pretty much defeats the purpose of the whole thing after all. If it's expected to fail very rarely, I'd just stick in WARN_ONCE() there as the stack trace would be interesting too.

I see. I think it is rarely to fail. But here is case that it must fail in the current bottom-up implementation. For example, we allocate memory in reserve_real_mode() by calling this:
memblock_find_in_range(0, 1<<20, size, PAGE_SIZE);

Both the start and end is below the kernel, so trying bottom-up for this must fail. So I am now thinking that if we should take this as the special case for bottom-up. That said, if we limit start and end both below the kernel, we should allocate memory below the kernel instead of make it fail. The cases are also rare, in early boot time, only these two:

 |->early_reserve_e820_mpc_new()   /* allocate memory under 1MB */
 |->reserve_real_mode()            /* allocate memory under 1MB */

How do you think?

Hello,

Quoted 12 lines · Zhang Yanfei

I see. I think it is rarely to fail. But here is case that it must fail in the current bottom-up implementation. For example, we allocate memory in reserve_real_mode() by calling this:
memblock_find_in_range(0, 1<<20, size, PAGE_SIZE);

Both the start and end is below the kernel, so trying bottom-up for this must fail. So I am now thinking that if we should take this as the special case for bottom-up. That said, if we limit start and end both below the kernel, we should allocate memory below the kernel instead of make it fail. The cases are also rare, in early boot time, only these two:

Quoted 4 lines

 |->early_reserve_e820_mpc_new()   /* allocate memory under 1MB */
 |->reserve_real_mode()            /* allocate memory under 1MB */

How do you think?

They need to be special cased regardless, right? It's wrong to print out warning messages for things which are expected to behave that way. Just skip bottom-up allocs if @end is under kernel image?

+/**
+ * memory_map_top_down - Map [map_start, map_end) top down
+ * @map_start: start address of the target memory range
+ * @map_end: end address of the target memory range
+ *
+ * This function will setup direct mapping for memory range [map_start, map_end)
+ * in a heuristic way. In the beginning, step_size is small. The more memory we
+ * map memory in the next loop.
+ */

arch/x86/mm/init.c +19 -6

-	while (last_start > ISA_END_ADDRESS) {
+	while (last_start > map_start) {
 		if (last_start > step_size) {
 			start = round_down(last_start - 1, step_size);
-			if (start < ISA_END_ADDRESS)
-				start = ISA_END_ADDRESS;
+			if (start < map_start)
+				start = map_start;
 		} else
-			start = ISA_END_ADDRESS;
+			start = map_start;
 		new_mapped_ram_size = init_range_memory_mapping(start,
 							last_start);
 		last_start = start;

@@ -453,8 +447,32 @@ void __init init_mem_mapping(void)
 		mapped_ram_size += new_mapped_ram_size;
 	}
 
-	if (real_end < end)
-		init_range_memory_mapping(real_end, end);
+	if (real_end < map_end)
+		init_range_memory_mapping(real_end, map_end);
+}
+
+void __init init_mem_mapping(void)
+{
+	unsigned long end;
+
+	probe_page_size_mask();
+
+#ifdef CONFIG_X86_64
+	end = max_pfn << PAGE_SHIFT;
+#else
+	end = max_low_pfn << PAGE_SHIFT;
+#endif

Quoted 9 lines · Zhang Yanfei

+/**
+ * memory_map_top_down - Map [map_start, map_end) top down
+ * @map_start: start address of the target memory range
+ * @map_end: end address of the target memory range
+ *
+ * This function will setup direct mapping for memory range [map_start, map_end)
+ * in a heuristic way. In the beginning, step_size is small. The more memory we
+ * map memory in the next loop.
+ */

The comment reads a bit weird to me. The step size is increased gradually but that really isn't really a heuristic and it doesn't mention mapping direction.

...

Quoted 4 lines

arch/x86/mm/init.c +0 -0

@@ -430,19 +430,13 @@ void __init init_mem_mapping(void)
 	min_pfn_mapped = real_end >> PAGE_SHIFT;
 	last_start = start = real_end;
 

arch/x86/mm/init.c void __init init_mem_mapping(void) · line 438

Quoted 6 lines

arch/x86/mm/init.c +0 -6

-	/*
-	 * We start from the top (end of memory) and go to the bottom.
-	 * The memblock_find_in_range() gets us a block of RAM from the
-	 * end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
-	 * for page table.
-	 */

I think this comment should stay here with the variable names updated.

Quoted 40 lines

arch/x86/mm/init.c +23 -6

-	while (last_start > ISA_END_ADDRESS) {
+	while (last_start > map_start) {
 		if (last_start > step_size) {
 			start = round_down(last_start - 1, step_size);
-			if (start < ISA_END_ADDRESS)
-				start = ISA_END_ADDRESS;
+			if (start < map_start)
+				start = map_start;
 		} else
-			start = ISA_END_ADDRESS;
+			start = map_start;
 		new_mapped_ram_size = init_range_memory_mapping(start,
 							last_start);
 		last_start = start;

@@ -453,8 +447,32 @@ void __init init_mem_mapping(void)
 		mapped_ram_size += new_mapped_ram_size;
 	}
 
-	if (real_end < end)
-		init_range_memory_mapping(real_end, end);
+	if (real_end < map_end)
+		init_range_memory_mapping(real_end, map_end);
+}
+
+void __init init_mem_mapping(void)
+{
+	unsigned long end;
+
+	probe_page_size_mask();
+
+#ifdef CONFIG_X86_64
+	end = max_pfn << PAGE_SHIFT;
+#else
+	end = max_low_pfn << PAGE_SHIFT;
+#endif
+
+	/* the ISA range is always mapped regardless of memory holes */
+	init_memory_mapping(0, ISA_END_ADDRESS);
+

arch/x86/mm/init.c void __init init_mem_mapping(void) · line 474

Quoted 6 lines

arch/x86/mm/init.c +6 -0

+	/*
+	 * We start from the top (end of memory) and go to the bottom.
+	 * The memblock_find_in_range() gets us a block of RAM from the
+	 * end of RAM in [min_pfn_mapped, max_pfn_mapped) used as new pages
+	 * for page table.
+	 */

And just mention the range and direction in the comment here?

arch/x86/mm/init.c void __init init_mem_mapping(void) · line 475

Quoted 2 lines

arch/x86/mm/init.c +1 -0

+	memory_map_top_down(ISA_END_ADDRESS, end);

Thanks.

+#ifdef CONFIG_MOVABLE_NODE

+/**
+ * memory_map_bottom_up - Map [map_start, map_end) bottom up
+ * @map_start: start address of the target memory range
+ * @map_end: end address of the target memory range
+ *
+ * This function will setup direct mapping for memory range [map_start, map_end)
+ * in a heuristic way. In the beginning, step_size is small. The more memory we
+ * map memory in the next loop.

arch/x86/mm/init.c +5 -0

+	 * If the allocation is in bottom-up direction, we start from the
+	 * bottom and go to the top: first [kernel_end, end) and then
+	 * [ISA_END_ADDRESS, kernel_end). Otherwise, we start from the top
+	 * (end of memory) and go to the bottom.
+	 *

Hello,

Quoted 1 line · Zhang Yanfei

+#ifdef CONFIG_MOVABLE_NODE

You don't need the above ifdef. The compiler will be able to cull the code as long as memblock_bottom_up() is defined as constant expression when !MOVABLE_NODE.

Quoted 9 lines

+/**
+ * memory_map_bottom_up - Map [map_start, map_end) bottom up
+ * @map_start: start address of the target memory range
+ * @map_end: end address of the target memory range
+ *
+ * This function will setup direct mapping for memory range [map_start, map_end)
+ * in a heuristic way. In the beginning, step_size is small. The more memory we
+ * map memory in the next loop.

The same comment as before. Now we have two function with the identical comment but behaving differently, which isn't nice.

...

Quoted 13 lines

arch/x86/mm/init.c +11 -1

+	 * If the allocation is in bottom-up direction, we start from the
+	 * bottom and go to the top: first [kernel_end, end) and then
+	 * [ISA_END_ADDRESS, kernel_end). Otherwise, we start from the top
+	 * (end of memory) and go to the bottom.
+	 *
+	 * The memblock_find_in_range() gets us a block of RAM in
+	 * [min_pfn_mapped, max_pfn_mapped) used as new pages for page table.
 	 */
-	memory_map_top_down(ISA_END_ADDRESS, end);
+	if (memblock_bottom_up()) {
+		unsigned long kernel_end;
+
+		kernel_end = round_up(__pa_symbol(_end), PMD_SIZE);

arch/x86/mm/init.c void __init init_mem_mapping(void) · line 527

Quoted 2 lines

arch/x86/mm/init.c +2 -0

+		memory_map_bottom_up(kernel_end, end);
+		memory_map_bottom_up(ISA_END_ADDRESS, kernel_end);

Hmm... so, this is kinda weird. We're doing it in two chunks and mapping memory between ISA_END_ADDRESS and kernel_end right on top of ISA_END_ADDRESS? Can't you give enough information to the mapping function so that it can map everything on top of kernel_end in single go?

Thanks.

Quoted 5 lines

Hmm... so, this is kinda weird. We're doing it in two chunks and mapping memory between ISA_END_ADDRESS and kernel_end right on top of ISA_END_ADDRESS? Can't you give enough information to the mapping function so that it can map everything on top of kernel_end in single go?

You mean we should call memory_map_bottom_up like this:

memory_map_bottom_up(ISA_END_ADDRESS, end)

right?

Quoted 6 lines · Zhang Yanfei

Quoted 5 lines

Hmm... so, this is kinda weird. We're doing it in two chunks and mapping memory between ISA_END_ADDRESS and kernel_end right on top of ISA_END_ADDRESS? Can't you give enough information to the mapping function so that it can map everything on top of kernel_end in single go?

Quoted 5 lines

You mean we should call memory_map_bottom_up like this:

memory_map_bottom_up(ISA_END_ADDRESS, end)

right?

But that wouldn't be ideal as we want the page tables above kernel image and the above would allocate it above ISA_END_ADDRESS, right?

Maybe memory_map_bottom_up() should take extra parameters for where to allocate page tables at separately from the mapping range and treat it specially? Would that make the function a lot more complex?

Thanks.

Quoted 2 lines

But that wouldn't be ideal as we want the page tables above kernel image and the above would allocate it above ISA_END_ADDRESS, right?

The original idea is we will allocate everything above the kernel. So the pagetables for [ISA_END_ADDRESS, kernel_end) will be also located above the kernel.

Hello,

Quoted 3 lines · Zhang Yanfei

Quoted 2 lines

But that wouldn't be ideal as we want the page tables above kernel image and the above would allocate it above ISA_END_ADDRESS, right?

Quoted 3 lines

The original idea is we will allocate everything above the kernel. So the pagetables for [ISA_END_ADDRESS, kernel_end) will be also located above the kernel.

I'm a bit confused why we need two separate calls then. What's the difference from calling with the whole range?

Thanks.

OK, this is just because we allocate pagtables just above the kernel. And if we use up the BRK space that is reserved for inital pagetables, we will use memblock to allocate memory for pagetables, and the memory allocated here should be mapped already. So we first map [kernel_end, end) to make memory above the kernel be mapped as soon as possible. And then use pagetables allocated above the kernel to map [ISA_END_ADDRESS, kernel_end).

Quoted 6 lines · Zhang Yanfei

OK, this is just because we allocate pagtables just above the kernel. And if we use up the BRK space that is reserved for inital pagetables, we will use memblock to allocate memory for pagetables, and the memory allocated here should be mapped already. So we first map [kernel_end, end) to make memory above the kernel be mapped as soon as possible. And then use pagetables allocated above the kernel to map [ISA_END_ADDRESS, kernel_end).

I see. The code seems fine to me then. Can you please add comment explaining why the split calls are necessary?

From: Tang Chen <[email protected]>

Memory reserved for crashkernel could be large. So we should not allocate this memory bottom up from the end of kernel image.

When SRAT is parsed, we will be able to know whihc memory is hotpluggable, and we can avoid allocating this memory for the kernel. So reorder reserve_crashkernel() after SRAT is parsed.

Signed-off-by Tang Chen <[email protected]>

Reviewed-by Zhang Yanfei <[email protected]>

Quoted 9 lines · Zhang Yanfei

From: Tang Chen <[email protected]>

Memory reserved for crashkernel could be large. So we should not allocate this memory bottom up from the end of kernel image.

When SRAT is parsed, we will be able to know whihc memory is hotpluggable, and we can avoid allocating this memory for the kernel. So reorder reserve_crashkernel() after SRAT is parsed.

Quoted 2 lines

Signed-off-by Tang Chen <[email protected]>

Reviewed-by Zhang Yanfei <[email protected]>

Assuming this was tested to work.

Acked-by Tejun Heo <[email protected]>

Thanks.

arch/x86/kernel/setup.c +0 -0

index 36cfce3..2cf04fd 100644

arch/x86/kernel/setup.c +15 -0

+static int __init cmdline_parse_movablenode(char *p)
+{
+	/*
+	 * Memory used by the kernel cannot be hot-removed because Linux
+	 * cannot migrate the kernel pages. When memory hotplug is
+	 * enabled, we should prevent memblock from allocating memory
+	 * for the kernel.
+	 *
+	 * ACPI SRAT records all hotpluggable memory ranges. But before
+	 * SRAT is parsed, we don't know about it.
+	 *
+	 * The kernel image is loaded into memory at very early time. We
+	 * cannot prevent this anyway. So on NUMA system, we set any
+	 * node the kernel resides in as un-hotpluggable.
+	 *

Hello,

Quoted 7 lines · Zhang Yanfei

arch/x86/kernel/setup.c +0 -0

index 36cfce3..2cf04fd 100644
@@ -1132,6 +1132,14 @@ void __init setup_arch(char **cmdline_p)
 	early_acpi_boot_init();
 

arch/x86/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 1141

Quoted 8 lines

arch/x86/kernel/setup.c +7 -0

 	initmem_init();
+
+	/*
+	 * When ACPI SRAT is parsed, which is done in initmem_init(),
+	 * set memblock back to the top-down direction.
+	 */
+	if (memblock_bottom_up())
+		memblock_set_bottom_up(false);

I don't think you need the if (). Just call
memblock_set_bottom_up(false).

Quoted 24 lines

arch/x86/kernel/setup.c +23 -0

+static int __init cmdline_parse_movablenode(char *p)
+{
+	/*
+	 * Memory used by the kernel cannot be hot-removed because Linux
+	 * cannot migrate the kernel pages. When memory hotplug is
+	 * enabled, we should prevent memblock from allocating memory
+	 * for the kernel.
+	 *
+	 * ACPI SRAT records all hotpluggable memory ranges. But before
+	 * SRAT is parsed, we don't know about it.
+	 *
+	 * The kernel image is loaded into memory at very early time. We
+	 * cannot prevent this anyway. So on NUMA system, we set any
+	 * node the kernel resides in as un-hotpluggable.
+	 *
+	 * Since on modern servers, one node could have double-digit
+	 * gigabytes memory, we can assume the memory around the kernel
+	 * image is also un-hotpluggable. So before SRAT is parsed, just
+	 * allocate memory near the kernel image to try the best to keep
+	 * the kernel away from hotpluggable memory.
+	 */
+	memblock_set_bottom_up(true);
+	return 0;

arch/x86/kernel/setup.c

Quoted 2 lines

arch/x86/kernel/setup.c +2 -0

+}
+early_param("movablenode", cmdline_parse_movablenode);

This came up during earlier review but never was addressed. Is "movablenode" the right name? Shouldn't it be something which explicitly shows that it's to prepare for memory hotplug? Also, maybe the above param should generate warning if CONFIG_MOVABLE_NODE isn't enabled?

Thanks.

Quoted 5 lines

This came up during earlier review but never was addressed. Is

Quoted 4 lines

"movablenode" the right name? Shouldn't it be something which explicitly shows that it's to prepare for memory hotplug? Also, maybe the above param should generate warning if CONFIG_MOVABLE_NODE isn't enabled?

hmmm...as for the option name, if this option is set, it means, the kernel could support the functionality that a whole node is the so called movable node, which only has ZONE MOVABLE zone in it. So we choose to name the parameter "movablenode".

As for the warning, will add it.

I am now preparing the v5 version. Only in this patch we haven't come to an agreement. So as for the boot option name, after my explanation, do you still have the objection? Or you could suggest a good name for us, that'll be very thankful:)

Hello,

Quoted 4 lines · Zhang Yanfei

I am now preparing the v5 version. Only in this patch we haven't come to an agreement. So as for the boot option name, after my explanation, do you still have the objection? Or you could suggest a good name for us, that'll be very thankful:)

No particular idea and my concern about the name isn't very strong. It just doesn't seem like a good name to me. It's a bit obscure and we may want to do more things for hotplug later which may not fit the param name. If you guys think it's good enough, please go ahead.

Hello tejun,

Quoted 10 lines · Zhang Yanfei

Quoted 5 lines

This came up during earlier review but never was addressed. Is

Quoted 4 lines

"movablenode" the right name? Shouldn't it be something which explicitly shows that it's to prepare for memory hotplug? Also, maybe the above param should generate warning if CONFIG_MOVABLE_NODE isn't enabled?

hmmm...as for the option name, if this option is set, it means, the kernel could support the functionality that a whole node is the so called movable node, which only has ZONE MOVABLE zone in it. So we choose to name the parameter "movablenode".

Quoted 1 line

As for the warning, will add it.

I am now preparing the v5 version. Only in this patch we haven't come to an agreement. So as for the boot option name, after my explanation, do you still have the objection? Or you could suggest a good name for us, that'll be very thankful:)

Quoted 15 lines · Zhang Yanfei

Hello tejun,

Quoted 11 lines · Zhang Yanfei

Quoted 5 lines

This came up during earlier review but never was addressed. Is

Quoted 4 lines

"movablenode" the right name? Shouldn't it be something which explicitly shows that it's to prepare for memory hotplug? Also, maybe the above param should generate warning if CONFIG_MOVABLE_NODE isn't enabled?

hmmm...as for the option name, if this option is set, it means, the kernel could support the functionality that a whole node is the so called movable node, which only has ZONE MOVABLE zone in it. So we choose to name the parameter "movablenode".

As for the warning, will add it.

Quoted 4 lines

I am now preparing the v5 version. Only in this patch we haven't come to an agreement. So as for the boot option name, after my explanation, do you still have the objection? Or you could suggest a good name for us, that'll be very thankful:)

I do not think the granularity has to stay as a node, and this option does nothing to with other devices that may be included in a node. So, how about using "movablemem"?

Hello toshi-san

Quoted 21 lines · Toshi Kani

Quoted 19 lines · Zhang Yanfei

Hello tejun,

Quoted 11 lines · Zhang Yanfei

Quoted 5 lines

This came up during earlier review but never was addressed. Is

Quoted 4 lines

"movablenode" the right name? Shouldn't it be something which explicitly shows that it's to prepare for memory hotplug? Also, maybe the above param should generate warning if CONFIG_MOVABLE_NODE isn't enabled?

hmmm...as for the option name, if this option is set, it means, the kernel could support the functionality that a whole node is the so called movable node, which only has ZONE MOVABLE zone in it. So we choose to name the parameter "movablenode".

As for the warning, will add it.

I am now preparing the v5 version. Only in this patch we haven't come to an agreement. So as for the boot option name, after my explanation, do you still have the objection? Or you could suggest a good name for us, that'll be very thankful:)

Quoted 4 lines

I do not think the granularity has to stay as a node, and this option does nothing to with other devices that may be included in a node. So, how about using "movablemem"?

As I explained before, we use movablenode to mean a node could only have a MOVABLE zone from the memory aspect. So I still think movablenode seems better than movablemem. movablemem seems vaguer here....