[PATCH v3 0/3] mm: fix memory to node bad links in sysfs

[PATCH v3 0/3] mm: fix memory to node bad links in sysfs

[Laurent Dufour]

Sometimes, firmware may expose interleaved memory layout like this:
 Early memory node ranges
   node   1: [mem 0x0000000000000000-0x000000011fffffff]
   node   2: [mem 0x0000000120000000-0x000000014fffffff]
   node   1: [mem 0x0000000150000000-0x00000001ffffffff]
   node   0: [mem 0x0000000200000000-0x000000048fffffff]
   node   2: [mem 0x0000000490000000-0x00000007ffffffff]

In that case, we can see memory blocks assigned to multiple nodes in sysfs:

$ ls -l /sys/devices/system/memory/memory21
total 0
lrwxrwxrwx 1 root root     0 Aug 24 05:27 node1 -> ../../node/node1
lrwxrwxrwx 1 root root     0 Aug 24 05:27 node2 -> ../../node/node2
-rw-r--r-- 1 root root 65536 Aug 24 05:27 online
-r--r--r-- 1 root root 65536 Aug 24 05:27 phys_device
-r--r--r-- 1 root root 65536 Aug 24 05:27 phys_index
drwxr-xr-x 2 root root     0 Aug 24 05:27 power
-r--r--r-- 1 root root 65536 Aug 24 05:27 removable
-rw-r--r-- 1 root root 65536 Aug 24 05:27 state
lrwxrwxrwx 1 root root     0 Aug 24 05:25 subsystem -> ../../../../bus/memory
-rw-r--r-- 1 root root 65536 Aug 24 05:25 uevent
-r--r--r-- 1 root root 65536 Aug 24 05:27 valid_zones

The same applies in the node's directory with a memory21 link in both the
node1 and node2's directory.

This is wrong but doesn't prevent the system to run. However when later,
one of these memory blocks is hot-unplugged and then hot-plugged, the
system is detecting an inconsistency in the sysfs layout and a BUG_ON() is
raised:

------------[ cut here ]------------
kernel BUG at /Users/laurent/src/linux-ppc/mm/memory_hotplug.c:1084!
Oops: Exception in kernel mode, sig: 5 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: rpadlpar_io rpaphp pseries_rng rng_core vmx_crypto gf128mul binfmt_misc ip_tables x_tables xfs libcrc32c crc32c_vpmsum autofs4
CPU: 8 PID: 10256 Comm: drmgr Not tainted 5.9.0-rc1+ #25
NIP:  c000000000403f34 LR: c000000000403f2c CTR: 0000000000000000
REGS: c0000004876e3660 TRAP: 0700   Not tainted  (5.9.0-rc1+)
MSR:  800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24000448  XER: 20040000
CFAR: c000000000846d20 IRQMASK: 0
GPR00: c000000000403f2c c0000004876e38f0 c0000000012f6f00 ffffffffffffffef
GPR04: 0000000000000227 c0000004805ae680 0000000000000000 00000004886f0000
GPR08: 0000000000000226 0000000000000003 0000000000000002 fffffffffffffffd
GPR12: 0000000088000484 c00000001ec96280 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000004 0000000000000003
GPR20: c00000047814ffe0 c0000007ffff7c08 0000000000000010 c0000000013332c8
GPR24: 0000000000000000 c0000000011f6cc0 0000000000000000 0000000000000000
GPR28: ffffffffffffffef 0000000000000001 0000000150000000 0000000010000000
NIP [c000000000403f34] add_memory_resource+0x244/0x340
LR [c000000000403f2c] add_memory_resource+0x23c/0x340
Call Trace:
[c0000004876e38f0] [c000000000403f2c] add_memory_resource+0x23c/0x340 (unreliable)
[c0000004876e39c0] [c00000000040408c] __add_memory+0x5c/0xf0
[c0000004876e39f0] [c0000000000e2b94] dlpar_add_lmb+0x1b4/0x500
[c0000004876e3ad0] [c0000000000e3888] dlpar_memory+0x1f8/0xb80
[c0000004876e3b60] [c0000000000dc0d0] handle_dlpar_errorlog+0xc0/0x190
[c0000004876e3bd0] [c0000000000dc398] dlpar_store+0x198/0x4a0
[c0000004876e3c90] [c00000000072e630] kobj_attr_store+0x30/0x50
[c0000004876e3cb0] [c00000000051f954] sysfs_kf_write+0x64/0x90
[c0000004876e3cd0] [c00000000051ee40] kernfs_fop_write+0x1b0/0x290
[c0000004876e3d20] [c000000000438dd8] vfs_write+0xe8/0x290
[c0000004876e3d70] [c0000000004391ac] ksys_write+0xdc/0x130
[c0000004876e3dc0] [c000000000034e40] system_call_exception+0x160/0x270
[c0000004876e3e20] [c00000000000d740] system_call_common+0xf0/0x27c
Instruction dump:
48442e35 60000000 0b030000 3cbe0001 7fa3eb78 7bc48402 38a5fffe 7ca5fa14
78a58402 48442db1 60000000 7c7c1b78 <0b030000> 7f23cb78 4bda371d 60000000
---[ end trace 562fd6c109cd0fb2 ]---

This has been seen on PowerPC LPAR.

The root cause of this issue is that when node's memory is registered, the
range used can overlap another node's range, thus the memory block is
registered to multiple nodes in sysfs.

There are 2 issues here:

a. The sysfs memory and node's layouts are broken due to these multiple
   links

b. The link errors in link_mem_sections() should not lead to a system
   panic.

To address a. register_mem_sect_under_node should not rely on the system
state to detect whether the link operation is triggered by a hot plug
operation or not. This is addressed by the patches 1 and 2 of this series.

The patch 3 is addressing the point b.

Thanks,
Laurent

Since v2:
- Address David's comments
- Fix stupid build errors in patch 1
Since v1:
- change context enum's name from Michal's comment
- use 2 callbacks in link_mem_sections from David's comment
- use dev_err_ratelimited from Greg's comment

Laurent Dufour (3):
  mm: replace memmap_context by memplug_context
  mm: don't rely on system state to detect hot-plug operations
  mm: don't panic when links can't be created in sysfs

 arch/ia64/mm/init.c    |  6 +--
 drivers/base/node.c    | 98 ++++++++++++++++++++++++++++--------------
 include/linux/mm.h     |  2 +-
 include/linux/mmzone.h | 11 +++--
 include/linux/node.h   | 13 +++---
 mm/memory_hotplug.c    |  6 +--
 mm/page_alloc.c        | 10 ++---
 7 files changed, 93 insertions(+), 53 deletions(-)

-- 
2.28.0

[PATCH v3 0/3] mm: fix memory to node bad links in sysfs

[Laurent Dufour]

Sometimes, firmware may expose interleaved memory layout like this:
 Early memory node ranges
   node   1: [mem 0x0000000000000000-0x000000011fffffff]
   node   2: [mem 0x0000000120000000-0x000000014fffffff]
   node   1: [mem 0x0000000150000000-0x00000001ffffffff]
   node   0: [mem 0x0000000200000000-0x000000048fffffff]
   node   2: [mem 0x0000000490000000-0x00000007ffffffff]

In that case, we can see memory blocks assigned to multiple nodes in sysfs:

$ ls -l /sys/devices/system/memory/memory21
total 0
lrwxrwxrwx 1 root root     0 Aug 24 05:27 node1 -> ../../node/node1
lrwxrwxrwx 1 root root     0 Aug 24 05:27 node2 -> ../../node/node2
-rw-r--r-- 1 root root 65536 Aug 24 05:27 online
-r--r--r-- 1 root root 65536 Aug 24 05:27 phys_device
-r--r--r-- 1 root root 65536 Aug 24 05:27 phys_index
drwxr-xr-x 2 root root     0 Aug 24 05:27 power
-r--r--r-- 1 root root 65536 Aug 24 05:27 removable
-rw-r--r-- 1 root root 65536 Aug 24 05:27 state
lrwxrwxrwx 1 root root     0 Aug 24 05:25 subsystem -> ../../../../bus/memory
-rw-r--r-- 1 root root 65536 Aug 24 05:25 uevent
-r--r--r-- 1 root root 65536 Aug 24 05:27 valid_zones

The same applies in the node's directory with a memory21 link in both the
node1 and node2's directory.

This is wrong but doesn't prevent the system to run. However when later,
one of these memory blocks is hot-unplugged and then hot-plugged, the
system is detecting an inconsistency in the sysfs layout and a BUG_ON() is
raised:

------------[ cut here ]------------
kernel BUG at /Users/laurent/src/linux-ppc/mm/memory_hotplug.c:1084!
Oops: Exception in kernel mode, sig: 5 [#1]
LE PAGE_SIZEdK MMU=Hash SMP NR_CPUS 48 NUMA pSeries
Modules linked in: rpadlpar_io rpaphp pseries_rng rng_core vmx_crypto gf128mul binfmt_misc ip_tables x_tables xfs libcrc32c crc32c_vpmsum autofs4
CPU: 8 PID: 10256 Comm: drmgr Not tainted 5.9.0-rc1+ #25
NIP:  c000000000403f34 LR: c000000000403f2c CTR: 0000000000000000
REGS: c0000004876e3660 TRAP: 0700   Not tainted  (5.9.0-rc1+)
MSR:  800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24000448  XER: 20040000
CFAR: c000000000846d20 IRQMASK: 0
GPR00: c000000000403f2c c0000004876e38f0 c0000000012f6f00 ffffffffffffffef
GPR04: 0000000000000227 c0000004805ae680 0000000000000000 00000004886f0000
GPR08: 0000000000000226 0000000000000003 0000000000000002 fffffffffffffffd
GPR12: 0000000088000484 c00000001ec96280 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000004 0000000000000003
GPR20: c00000047814ffe0 c0000007ffff7c08 0000000000000010 c0000000013332c8
GPR24: 0000000000000000 c0000000011f6cc0 0000000000000000 0000000000000000
GPR28: ffffffffffffffef 0000000000000001 0000000150000000 0000000010000000
NIP [c000000000403f34] add_memory_resource+0x244/0x340
LR [c000000000403f2c] add_memory_resource+0x23c/0x340
Call Trace:
[c0000004876e38f0] [c000000000403f2c] add_memory_resource+0x23c/0x340 (unreliable)
[c0000004876e39c0] [c00000000040408c] __add_memory+0x5c/0xf0
[c0000004876e39f0] [c0000000000e2b94] dlpar_add_lmb+0x1b4/0x500
[c0000004876e3ad0] [c0000000000e3888] dlpar_memory+0x1f8/0xb80
[c0000004876e3b60] [c0000000000dc0d0] handle_dlpar_errorlog+0xc0/0x190
[c0000004876e3bd0] [c0000000000dc398] dlpar_store+0x198/0x4a0
[c0000004876e3c90] [c00000000072e630] kobj_attr_store+0x30/0x50
[c0000004876e3cb0] [c00000000051f954] sysfs_kf_write+0x64/0x90
[c0000004876e3cd0] [c00000000051ee40] kernfs_fop_write+0x1b0/0x290
[c0000004876e3d20] [c000000000438dd8] vfs_write+0xe8/0x290
[c0000004876e3d70] [c0000000004391ac] ksys_write+0xdc/0x130
[c0000004876e3dc0] [c000000000034e40] system_call_exception+0x160/0x270
[c0000004876e3e20] [c00000000000d740] system_call_common+0xf0/0x27c
Instruction dump:
48442e35 60000000 0b030000 3cbe0001 7fa3eb78 7bc48402 38a5fffe 7ca5fa14
78a58402 48442db1 60000000 7c7c1b78 <0b030000> 7f23cb78 4bda371d 60000000
---[ end trace 562fd6c109cd0fb2 ]---

This has been seen on PowerPC LPAR.

The root cause of this issue is that when node's memory is registered, the
range used can overlap another node's range, thus the memory block is
registered to multiple nodes in sysfs.

There are 2 issues here:

a. The sysfs memory and node's layouts are broken due to these multiple
   links

b. The link errors in link_mem_sections() should not lead to a system
   panic.

To address a. register_mem_sect_under_node should not rely on the system
state to detect whether the link operation is triggered by a hot plug
operation or not. This is addressed by the patches 1 and 2 of this series.

The patch 3 is addressing the point b.

Thanks,
Laurent

Since v2:
- Address David's comments
- Fix stupid build errors in patch 1
Since v1:
- change context enum's name from Michal's comment
- use 2 callbacks in link_mem_sections from David's comment
- use dev_err_ratelimited from Greg's comment

Laurent Dufour (3):
  mm: replace memmap_context by memplug_context
  mm: don't rely on system state to detect hot-plug operations
  mm: don't panic when links can't be created in sysfs

 arch/ia64/mm/init.c    |  6 +--
 drivers/base/node.c    | 98 ++++++++++++++++++++++++++++--------------
 include/linux/mm.h     |  2 +-
 include/linux/mmzone.h | 11 +++--
 include/linux/node.h   | 13 +++---
 mm/memory_hotplug.c    |  6 +--
 mm/page_alloc.c        | 10 ++---
 7 files changed, 93 insertions(+), 53 deletions(-)

-- 
2.28.0

[PATCH v3 1/3] mm: replace memmap_context by memplug_context

[Laurent Dufour]

The memmap_context is used to detect whether a memory operation is due to a
hot-add operation or happening at boot time.

Make it general to the hotplug operation and rename it at memplug_context.

There is no functional change introduced by this patch

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
 arch/ia64/mm/init.c    |  6 +++---
 include/linux/mm.h     |  2 +-
 include/linux/mmzone.h | 11 ++++++++---
 mm/memory_hotplug.c    |  2 +-
 mm/page_alloc.c        | 10 +++++-----
 5 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 0b3fb4c7af29..8e7b8c6c576e 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
 	if (map_start < map_end)
 		memmap_init_zone((unsigned long)(map_end - map_start),
 				 args->nid, args->zone, page_to_pfn(map_start),
-				 MEMMAP_EARLY, NULL);
+				 MEMINIT_EARLY, NULL);
 	return 0;
 }
 
@@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
 	     unsigned long start_pfn)
 {
 	if (!vmem_map) {
-		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
-				NULL);
+		memmap_init_zone(size, nid, zone, start_pfn,
+				 MEMINIT_EARLY, NULL);
 	} else {
 		struct page *start;
 		struct memmap_init_callback_data args;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1983e08f5906..e942f91ed155 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
 
 extern void set_dma_reserve(unsigned long new_dma_reserve);
 extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
-		enum memmap_context, struct vmem_altmap *);
+		enum meminit_context, struct vmem_altmap *);
 extern void setup_per_zone_wmarks(void);
 extern int __meminit init_per_zone_wmark_min(void);
 extern void mem_init(void);
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 8379432f4f2f..0f7a4ff4b059 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
 		unsigned int alloc_flags);
 bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
 		unsigned long mark, int highest_zoneidx);
-enum memmap_context {
-	MEMMAP_EARLY,
-	MEMMAP_HOTPLUG,
+/*
+ * Memory initialization context, use to differentiate memory added by
+ * the platform statically or via memory hotplug interface.
+ */
+enum meminit_context {
+	MEMINIT_EARLY,
+	MEMINIT_HOTPLUG,
 };
+
 extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
 				     unsigned long size);
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e9d5ab5d3ca0..fc25886ad719 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 	 * are reserved so nobody should be touching them so we should be safe
 	 */
 	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
-			MEMMAP_HOTPLUG, altmap);
+			 MEMINIT_HOTPLUG, altmap);
 
 	set_zone_contiguous(zone);
 }
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fab5e97dc9ca..5661fa164f13 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
  * done. Non-atomic initialization, single-pass.
  */
 void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
-		unsigned long start_pfn, enum memmap_context context,
+		unsigned long start_pfn, enum meminit_context context,
 		struct vmem_altmap *altmap)
 {
 	unsigned long pfn, end_pfn = start_pfn + size;
@@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 		 * There can be holes in boot-time mem_map[]s handed to this
 		 * function.  They do not exist on hotplugged memory.
 		 */
-		if (context == MEMMAP_EARLY) {
+		if (context == MEMINIT_EARLY) {
 			if (overlap_memmap_init(zone, &pfn))
 				continue;
 			if (defer_init(nid, pfn, end_pfn))
@@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 
 		page = pfn_to_page(pfn);
 		__init_single_page(page, pfn, zone, nid);
-		if (context == MEMMAP_HOTPLUG)
+		if (context == MEMINIT_HOTPLUG)
 			__SetPageReserved(page);
 
 		/*
@@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
 		 * check here not to call set_pageblock_migratetype() against
 		 * pfn out of zone.
 		 *
-		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
+		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
 		 * because this is done early in section_activate()
 		 */
 		if (!(pfn & (pageblock_nr_pages - 1))) {
@@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
 		if (end_pfn > start_pfn) {
 			size = end_pfn - start_pfn;
 			memmap_init_zone(size, nid, zone, start_pfn,
-					 MEMMAP_EARLY, NULL);
+					 MEMINIT_EARLY, NULL);
 		}
 	}
 }
-- 
2.28.0

[PATCH v3 1/3] mm: replace memmap_context by memplug_context

[Laurent Dufour]

The memmap_context is used to detect whether a memory operation is due to a
hot-add operation or happening at boot time.

Make it general to the hotplug operation and rename it at memplug_context.

There is no functional change introduced by this patch

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
 arch/ia64/mm/init.c    |  6 +++---
 include/linux/mm.h     |  2 +-
 include/linux/mmzone.h | 11 ++++++++---
 mm/memory_hotplug.c    |  2 +-
 mm/page_alloc.c        | 10 +++++-----
 5 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 0b3fb4c7af29..8e7b8c6c576e 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
 	if (map_start < map_end)
 		memmap_init_zone((unsigned long)(map_end - map_start),
 				 args->nid, args->zone, page_to_pfn(map_start),
-				 MEMMAP_EARLY, NULL);
+				 MEMINIT_EARLY, NULL);
 	return 0;
 }
 
@@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
 	     unsigned long start_pfn)
 {
 	if (!vmem_map) {
-		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
-				NULL);
+		memmap_init_zone(size, nid, zone, start_pfn,
+				 MEMINIT_EARLY, NULL);
 	} else {
 		struct page *start;
 		struct memmap_init_callback_data args;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1983e08f5906..e942f91ed155 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
 
 extern void set_dma_reserve(unsigned long new_dma_reserve);
 extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
-		enum memmap_context, struct vmem_altmap *);
+		enum meminit_context, struct vmem_altmap *);
 extern void setup_per_zone_wmarks(void);
 extern int __meminit init_per_zone_wmark_min(void);
 extern void mem_init(void);
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 8379432f4f2f..0f7a4ff4b059 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
 		unsigned int alloc_flags);
 bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
 		unsigned long mark, int highest_zoneidx);
-enum memmap_context {
-	MEMMAP_EARLY,
-	MEMMAP_HOTPLUG,
+/*
+ * Memory initialization context, use to differentiate memory added by
+ * the platform statically or via memory hotplug interface.
+ */
+enum meminit_context {
+	MEMINIT_EARLY,
+	MEMINIT_HOTPLUG,
 };
+
 extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
 				     unsigned long size);
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e9d5ab5d3ca0..fc25886ad719 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 	 * are reserved so nobody should be touching them so we should be safe
 	 */
 	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
-			MEMMAP_HOTPLUG, altmap);
+			 MEMINIT_HOTPLUG, altmap);
 
 	set_zone_contiguous(zone);
 }
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fab5e97dc9ca..5661fa164f13 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
  * done. Non-atomic initialization, single-pass.
  */
 void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
-		unsigned long start_pfn, enum memmap_context context,
+		unsigned long start_pfn, enum meminit_context context,
 		struct vmem_altmap *altmap)
 {
 	unsigned long pfn, end_pfn = start_pfn + size;
@@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 		 * There can be holes in boot-time mem_map[]s handed to this
 		 * function.  They do not exist on hotplugged memory.
 		 */
-		if (context = MEMMAP_EARLY) {
+		if (context = MEMINIT_EARLY) {
 			if (overlap_memmap_init(zone, &pfn))
 				continue;
 			if (defer_init(nid, pfn, end_pfn))
@@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 
 		page = pfn_to_page(pfn);
 		__init_single_page(page, pfn, zone, nid);
-		if (context = MEMMAP_HOTPLUG)
+		if (context = MEMINIT_HOTPLUG)
 			__SetPageReserved(page);
 
 		/*
@@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
 		 * check here not to call set_pageblock_migratetype() against
 		 * pfn out of zone.
 		 *
-		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
+		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
 		 * because this is done early in section_activate()
 		 */
 		if (!(pfn & (pageblock_nr_pages - 1))) {
@@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
 		if (end_pfn > start_pfn) {
 			size = end_pfn - start_pfn;
 			memmap_init_zone(size, nid, zone, start_pfn,
-					 MEMMAP_EARLY, NULL);
+					 MEMINIT_EARLY, NULL);
 		}
 	}
 }
-- 
2.28.0

[PATCH v3 2/3] mm: don't rely on system state to detect hot-plug operations

[Laurent Dufour]

In register_mem_sect_under_node() the system_state’s value is checked to
detect whether the call is made during boot time or during an hot-plug
operation. Unfortunately, that check against SYSTEM_BOOTING is wrong
because regular memory is registered at SYSTEM_SCHEDULING state. In
addition, memory hot-plug operation can be triggered at this system state
by the ACPI [1]. So checking against the system state is not enough.

The consequence is that on system with interleaved node's ranges like this:
 Early memory node ranges
   node   1: [mem 0x0000000000000000-0x000000011fffffff]
   node   2: [mem 0x0000000120000000-0x000000014fffffff]
   node   1: [mem 0x0000000150000000-0x00000001ffffffff]
   node   0: [mem 0x0000000200000000-0x000000048fffffff]
   node   2: [mem 0x0000000490000000-0x00000007ffffffff]

This can be seen on PowerPC LPAR after multiple memory hot-plug and
hot-unplug operations are done. At the next reboot the node's memory ranges
can be interleaved and since the call to link_mem_sections() is made in
topology_init() while the system is in the SYSTEM_SCHEDULING state, the
node's id is not checked, and the sections registered to multiple nodes:

$ ls -l /sys/devices/system/memory/memory21/node*
total 0
lrwxrwxrwx 1 root root     0 Aug 24 05:27 node1 -> ../../node/node1
lrwxrwxrwx 1 root root     0 Aug 24 05:27 node2 -> ../../node/node2

In that case, the system is able to boot but if later one of theses memory
blocks is hot-unplugged and then hot-plugged, the sysfs inconsistency is
detected and this is triggering a BUG_ON():

------------[ cut here ]------------
kernel BUG at /Users/laurent/src/linux-ppc/mm/memory_hotplug.c:1084!
Oops: Exception in kernel mode, sig: 5 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: rpadlpar_io rpaphp pseries_rng rng_core vmx_crypto gf128mul binfmt_misc ip_tables x_tables xfs libcrc32c crc32c_vpmsum autofs4
CPU: 8 PID: 10256 Comm: drmgr Not tainted 5.9.0-rc1+ #25
NIP:  c000000000403f34 LR: c000000000403f2c CTR: 0000000000000000
REGS: c0000004876e3660 TRAP: 0700   Not tainted  (5.9.0-rc1+)
MSR:  800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24000448  XER: 20040000
CFAR: c000000000846d20 IRQMASK: 0
GPR00: c000000000403f2c c0000004876e38f0 c0000000012f6f00 ffffffffffffffef
GPR04: 0000000000000227 c0000004805ae680 0000000000000000 00000004886f0000
GPR08: 0000000000000226 0000000000000003 0000000000000002 fffffffffffffffd
GPR12: 0000000088000484 c00000001ec96280 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000004 0000000000000003
GPR20: c00000047814ffe0 c0000007ffff7c08 0000000000000010 c0000000013332c8
GPR24: 0000000000000000 c0000000011f6cc0 0000000000000000 0000000000000000
GPR28: ffffffffffffffef 0000000000000001 0000000150000000 0000000010000000
NIP [c000000000403f34] add_memory_resource+0x244/0x340
LR [c000000000403f2c] add_memory_resource+0x23c/0x340
Call Trace:
[c0000004876e38f0] [c000000000403f2c] add_memory_resource+0x23c/0x340 (unreliable)
[c0000004876e39c0] [c00000000040408c] __add_memory+0x5c/0xf0
[c0000004876e39f0] [c0000000000e2b94] dlpar_add_lmb+0x1b4/0x500
[c0000004876e3ad0] [c0000000000e3888] dlpar_memory+0x1f8/0xb80
[c0000004876e3b60] [c0000000000dc0d0] handle_dlpar_errorlog+0xc0/0x190
[c0000004876e3bd0] [c0000000000dc398] dlpar_store+0x198/0x4a0
[c0000004876e3c90] [c00000000072e630] kobj_attr_store+0x30/0x50
[c0000004876e3cb0] [c00000000051f954] sysfs_kf_write+0x64/0x90
[c0000004876e3cd0] [c00000000051ee40] kernfs_fop_write+0x1b0/0x290
[c0000004876e3d20] [c000000000438dd8] vfs_write+0xe8/0x290
[c0000004876e3d70] [c0000000004391ac] ksys_write+0xdc/0x130
[c0000004876e3dc0] [c000000000034e40] system_call_exception+0x160/0x270
[c0000004876e3e20] [c00000000000d740] system_call_common+0xf0/0x27c
Instruction dump:
48442e35 60000000 0b030000 3cbe0001 7fa3eb78 7bc48402 38a5fffe 7ca5fa14
78a58402 48442db1 60000000 7c7c1b78 <0b030000> 7f23cb78 4bda371d 60000000
---[ end trace 562fd6c109cd0fb2 ]---

This patch addresses the root cause by not relying on the system_state
value to detect whether the call is due to a hot-plug operation. An extra
parameter is added to link_mem_sections() detailing whether the operation
is due to a hot-plug operation.

[1] According to Oscar Salvador, using this qemu command line, ACPI memory
hotplug operations are raised at SYSTEM_SCHEDULING state:

$QEMU -enable-kvm -machine pc -smp 4,sockets=4,cores=1,threads=1 -cpu host -monitor pty \
        -m size=$MEM,slots=255,maxmem=4294967296k  \
        -numa node,nodeid=0,cpus=0-3,mem=512 -numa node,nodeid=1,mem=512 \
        -object memory-backend-ram,id=memdimm0,size=134217728 -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \
        -object memory-backend-ram,id=memdimm1,size=134217728 -device pc-dimm,node=0,memdev=memdimm1,id=dimm1,slot=1 \
        -object memory-backend-ram,id=memdimm2,size=134217728 -device pc-dimm,node=0,memdev=memdimm2,id=dimm2,slot=2 \
        -object memory-backend-ram,id=memdimm3,size=134217728 -device pc-dimm,node=0,memdev=memdimm3,id=dimm3,slot=3 \
        -object memory-backend-ram,id=memdimm4,size=134217728 -device pc-dimm,node=1,memdev=memdimm4,id=dimm4,slot=4 \
        -object memory-backend-ram,id=memdimm5,size=134217728 -device pc-dimm,node=1,memdev=memdimm5,id=dimm5,slot=5 \
        -object memory-backend-ram,id=memdimm6,size=134217728 -device pc-dimm,node=1,memdev=memdimm6,id=dimm6,slot=6 \

Fixes: 4fbce633910e ("mm/memory_hotplug.c: make register_mem_sect_under_node() a callback of walk_memory_range()")
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Cc: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oscar Salvador <osalvador@suse.de>
---
 drivers/base/node.c  | 85 ++++++++++++++++++++++++++++----------------
 include/linux/node.h | 11 +++---
 mm/memory_hotplug.c  |  3 +-
 3 files changed, 64 insertions(+), 35 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 508b80f6329b..50af16e68d98 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -761,14 +761,36 @@ static int __ref get_nid_for_pfn(unsigned long pfn)
 	return pfn_to_nid(pfn);
 }
 
+static int do_register_memory_block_under_node(int nid,
+					       struct memory_block *mem_blk)
+{
+	int ret;
+
+	/*
+	 * If this memory block spans multiple nodes, we only indicate
+	 * the last processed node.
+	 */
+	mem_blk->nid = nid;
+
+	ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
+				       &mem_blk->dev.kobj,
+				       kobject_name(&mem_blk->dev.kobj));
+	if (ret)
+		return ret;
+
+	return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
+				&node_devices[nid]->dev.kobj,
+				kobject_name(&node_devices[nid]->dev.kobj));
+}
+
 /* register memory section under specified node if it spans that node */
-static int register_mem_sect_under_node(struct memory_block *mem_blk,
-					 void *arg)
+static int register_mem_block_under_node_early(struct memory_block *mem_blk,
+					       void *arg)
 {
 	unsigned long memory_block_pfns = memory_block_size_bytes() / PAGE_SIZE;
 	unsigned long start_pfn = section_nr_to_pfn(mem_blk->start_section_nr);
 	unsigned long end_pfn = start_pfn + memory_block_pfns - 1;
-	int ret, nid = *(int *)arg;
+	int nid = *(int *)arg;
 	unsigned long pfn;
 
 	for (pfn = start_pfn; pfn <= end_pfn; pfn++) {
@@ -785,38 +807,33 @@ static int register_mem_sect_under_node(struct memory_block *mem_blk,
 		}
 
 		/*
-		 * We need to check if page belongs to nid only for the boot
-		 * case, during hotplug we know that all pages in the memory
-		 * block belong to the same node.
-		 */
-		if (system_state == SYSTEM_BOOTING) {
-			page_nid = get_nid_for_pfn(pfn);
-			if (page_nid < 0)
-				continue;
-			if (page_nid != nid)
-				continue;
-		}
-
-		/*
-		 * If this memory block spans multiple nodes, we only indicate
-		 * the last processed node.
+		 * We need to check if page belongs to nid only at the boot
+		 * case because node's ranges can be interleaved.
 		 */
-		mem_blk->nid = nid;
-
-		ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
-					&mem_blk->dev.kobj,
-					kobject_name(&mem_blk->dev.kobj));
-		if (ret)
-			return ret;
+		page_nid = get_nid_for_pfn(pfn);
+		if (page_nid < 0)
+			continue;
+		if (page_nid != nid)
+			continue;
 
-		return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
-				&node_devices[nid]->dev.kobj,
-				kobject_name(&node_devices[nid]->dev.kobj));
+		return do_register_memory_block_under_node(nid, mem_blk);
 	}
 	/* mem section does not span the specified node */
 	return 0;
 }
 
+/*
+ * During hotplug we know that all pages in the memory block belong to the same
+ * node.
+ */
+static int register_mem_block_under_node_hotplug(struct memory_block *mem_blk,
+						 void *arg)
+{
+	int nid = *(int *)arg;
+
+	return do_register_memory_block_under_node(nid, mem_blk);
+}
+
 /*
  * Unregister a memory block device under the node it spans. Memory blocks
  * with multiple nodes cannot be offlined and therefore also never be removed.
@@ -832,11 +849,19 @@ void unregister_memory_block_under_nodes(struct memory_block *mem_blk)
 			  kobject_name(&node_devices[mem_blk->nid]->dev.kobj));
 }
 
-int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn)
+int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
+		      enum meminit_context context)
 {
+	walk_memory_blocks_func_t func;
+
+	if (context == MEMINIT_HOTPLUG)
+		func = register_mem_block_under_node_hotplug;
+	else
+		func = register_mem_block_under_node_early;
+
 	return walk_memory_blocks(PFN_PHYS(start_pfn),
 				  PFN_PHYS(end_pfn - start_pfn), (void *)&nid,
-				  register_mem_sect_under_node);
+				  func);
 }
 
 #ifdef CONFIG_HUGETLBFS
diff --git a/include/linux/node.h b/include/linux/node.h
index 4866f32a02d8..014ba3ab2efd 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -99,11 +99,13 @@ extern struct node *node_devices[];
 typedef  void (*node_registration_func_t)(struct node *);
 
 #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
-extern int link_mem_sections(int nid, unsigned long start_pfn,
-			     unsigned long end_pfn);
+int link_mem_sections(int nid, unsigned long start_pfn,
+		      unsigned long end_pfn,
+		      enum meminit_context context);
 #else
 static inline int link_mem_sections(int nid, unsigned long start_pfn,
-				    unsigned long end_pfn)
+				    unsigned long end_pfn,
+				    enum meminit_context context)
 {
 	return 0;
 }
@@ -128,7 +130,8 @@ static inline int register_one_node(int nid)
 		if (error)
 			return error;
 		/* link memory sections under this node */
-		error = link_mem_sections(nid, start_pfn, end_pfn);
+		error = link_mem_sections(nid, start_pfn, end_pfn,
+					  MEMINIT_EARLY);
 	}
 
 	return error;
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index fc25886ad719..03df20078827 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1080,7 +1080,8 @@ int __ref add_memory_resource(int nid, struct resource *res)
 	}
 
 	/* link memory sections under this node.*/
-	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1));
+	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1),
+				MEMINIT_HOTPLUG);
 	BUG_ON(ret);
 
 	/* create new memmap entry */
-- 
2.28.0

[PATCH v3 2/3] mm: don't rely on system state to detect hot-plug operations

[Laurent Dufour]

In register_mem_sect_under_node() the system_state’s value is checked to
detect whether the call is made during boot time or during an hot-plug
operation. Unfortunately, that check against SYSTEM_BOOTING is wrong
because regular memory is registered at SYSTEM_SCHEDULING state. In
addition, memory hot-plug operation can be triggered at this system state
by the ACPI [1]. So checking against the system state is not enough.

The consequence is that on system with interleaved node's ranges like this:
 Early memory node ranges
   node   1: [mem 0x0000000000000000-0x000000011fffffff]
   node   2: [mem 0x0000000120000000-0x000000014fffffff]
   node   1: [mem 0x0000000150000000-0x00000001ffffffff]
   node   0: [mem 0x0000000200000000-0x000000048fffffff]
   node   2: [mem 0x0000000490000000-0x00000007ffffffff]

This can be seen on PowerPC LPAR after multiple memory hot-plug and
hot-unplug operations are done. At the next reboot the node's memory ranges
can be interleaved and since the call to link_mem_sections() is made in
topology_init() while the system is in the SYSTEM_SCHEDULING state, the
node's id is not checked, and the sections registered to multiple nodes:

$ ls -l /sys/devices/system/memory/memory21/node*
total 0
lrwxrwxrwx 1 root root     0 Aug 24 05:27 node1 -> ../../node/node1
lrwxrwxrwx 1 root root     0 Aug 24 05:27 node2 -> ../../node/node2

In that case, the system is able to boot but if later one of theses memory
blocks is hot-unplugged and then hot-plugged, the sysfs inconsistency is
detected and this is triggering a BUG_ON():

------------[ cut here ]------------
kernel BUG at /Users/laurent/src/linux-ppc/mm/memory_hotplug.c:1084!
Oops: Exception in kernel mode, sig: 5 [#1]
LE PAGE_SIZEdK MMU=Hash SMP NR_CPUS 48 NUMA pSeries
Modules linked in: rpadlpar_io rpaphp pseries_rng rng_core vmx_crypto gf128mul binfmt_misc ip_tables x_tables xfs libcrc32c crc32c_vpmsum autofs4
CPU: 8 PID: 10256 Comm: drmgr Not tainted 5.9.0-rc1+ #25
NIP:  c000000000403f34 LR: c000000000403f2c CTR: 0000000000000000
REGS: c0000004876e3660 TRAP: 0700   Not tainted  (5.9.0-rc1+)
MSR:  800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24000448  XER: 20040000
CFAR: c000000000846d20 IRQMASK: 0
GPR00: c000000000403f2c c0000004876e38f0 c0000000012f6f00 ffffffffffffffef
GPR04: 0000000000000227 c0000004805ae680 0000000000000000 00000004886f0000
GPR08: 0000000000000226 0000000000000003 0000000000000002 fffffffffffffffd
GPR12: 0000000088000484 c00000001ec96280 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000004 0000000000000003
GPR20: c00000047814ffe0 c0000007ffff7c08 0000000000000010 c0000000013332c8
GPR24: 0000000000000000 c0000000011f6cc0 0000000000000000 0000000000000000
GPR28: ffffffffffffffef 0000000000000001 0000000150000000 0000000010000000
NIP [c000000000403f34] add_memory_resource+0x244/0x340
LR [c000000000403f2c] add_memory_resource+0x23c/0x340
Call Trace:
[c0000004876e38f0] [c000000000403f2c] add_memory_resource+0x23c/0x340 (unreliable)
[c0000004876e39c0] [c00000000040408c] __add_memory+0x5c/0xf0
[c0000004876e39f0] [c0000000000e2b94] dlpar_add_lmb+0x1b4/0x500
[c0000004876e3ad0] [c0000000000e3888] dlpar_memory+0x1f8/0xb80
[c0000004876e3b60] [c0000000000dc0d0] handle_dlpar_errorlog+0xc0/0x190
[c0000004876e3bd0] [c0000000000dc398] dlpar_store+0x198/0x4a0
[c0000004876e3c90] [c00000000072e630] kobj_attr_store+0x30/0x50
[c0000004876e3cb0] [c00000000051f954] sysfs_kf_write+0x64/0x90
[c0000004876e3cd0] [c00000000051ee40] kernfs_fop_write+0x1b0/0x290
[c0000004876e3d20] [c000000000438dd8] vfs_write+0xe8/0x290
[c0000004876e3d70] [c0000000004391ac] ksys_write+0xdc/0x130
[c0000004876e3dc0] [c000000000034e40] system_call_exception+0x160/0x270
[c0000004876e3e20] [c00000000000d740] system_call_common+0xf0/0x27c
Instruction dump:
48442e35 60000000 0b030000 3cbe0001 7fa3eb78 7bc48402 38a5fffe 7ca5fa14
78a58402 48442db1 60000000 7c7c1b78 <0b030000> 7f23cb78 4bda371d 60000000
---[ end trace 562fd6c109cd0fb2 ]---

This patch addresses the root cause by not relying on the system_state
value to detect whether the call is due to a hot-plug operation. An extra
parameter is added to link_mem_sections() detailing whether the operation
is due to a hot-plug operation.

[1] According to Oscar Salvador, using this qemu command line, ACPI memory
hotplug operations are raised at SYSTEM_SCHEDULING state:

$QEMU -enable-kvm -machine pc -smp 4,sockets=4,cores=1,threads=1 -cpu host -monitor pty \
        -m size=$MEM,slots%5,maxmemB94967296k  \
        -numa node,nodeid=0,cpus=0-3,memQ2 -numa node,nodeid=1,memQ2 \
        -object memory-backend-ram,id=memdimm0,size\x134217728 -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \
        -object memory-backend-ram,id=memdimm1,size\x134217728 -device pc-dimm,node=0,memdev=memdimm1,id=dimm1,slot=1 \
        -object memory-backend-ram,id=memdimm2,size\x134217728 -device pc-dimm,node=0,memdev=memdimm2,id=dimm2,slot=2 \
        -object memory-backend-ram,id=memdimm3,size\x134217728 -device pc-dimm,node=0,memdev=memdimm3,id=dimm3,slot=3 \
        -object memory-backend-ram,id=memdimm4,size\x134217728 -device pc-dimm,node=1,memdev=memdimm4,id=dimm4,slot=4 \
        -object memory-backend-ram,id=memdimm5,size\x134217728 -device pc-dimm,node=1,memdev=memdimm5,id=dimm5,slot=5 \
        -object memory-backend-ram,id=memdimm6,size\x134217728 -device pc-dimm,node=1,memdev=memdimm6,id=dimm6,slot=6 \

Fixes: 4fbce633910e ("mm/memory_hotplug.c: make register_mem_sect_under_node() a callback of walk_memory_range()")
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Cc: stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oscar Salvador <osalvador@suse.de>
---
 drivers/base/node.c  | 85 ++++++++++++++++++++++++++++----------------
 include/linux/node.h | 11 +++---
 mm/memory_hotplug.c  |  3 +-
 3 files changed, 64 insertions(+), 35 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 508b80f6329b..50af16e68d98 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -761,14 +761,36 @@ static int __ref get_nid_for_pfn(unsigned long pfn)
 	return pfn_to_nid(pfn);
 }
 
+static int do_register_memory_block_under_node(int nid,
+					       struct memory_block *mem_blk)
+{
+	int ret;
+
+	/*
+	 * If this memory block spans multiple nodes, we only indicate
+	 * the last processed node.
+	 */
+	mem_blk->nid = nid;
+
+	ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
+				       &mem_blk->dev.kobj,
+				       kobject_name(&mem_blk->dev.kobj));
+	if (ret)
+		return ret;
+
+	return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
+				&node_devices[nid]->dev.kobj,
+				kobject_name(&node_devices[nid]->dev.kobj));
+}
+
 /* register memory section under specified node if it spans that node */
-static int register_mem_sect_under_node(struct memory_block *mem_blk,
-					 void *arg)
+static int register_mem_block_under_node_early(struct memory_block *mem_blk,
+					       void *arg)
 {
 	unsigned long memory_block_pfns = memory_block_size_bytes() / PAGE_SIZE;
 	unsigned long start_pfn = section_nr_to_pfn(mem_blk->start_section_nr);
 	unsigned long end_pfn = start_pfn + memory_block_pfns - 1;
-	int ret, nid = *(int *)arg;
+	int nid = *(int *)arg;
 	unsigned long pfn;
 
 	for (pfn = start_pfn; pfn <= end_pfn; pfn++) {
@@ -785,38 +807,33 @@ static int register_mem_sect_under_node(struct memory_block *mem_blk,
 		}
 
 		/*
-		 * We need to check if page belongs to nid only for the boot
-		 * case, during hotplug we know that all pages in the memory
-		 * block belong to the same node.
-		 */
-		if (system_state = SYSTEM_BOOTING) {
-			page_nid = get_nid_for_pfn(pfn);
-			if (page_nid < 0)
-				continue;
-			if (page_nid != nid)
-				continue;
-		}
-
-		/*
-		 * If this memory block spans multiple nodes, we only indicate
-		 * the last processed node.
+		 * We need to check if page belongs to nid only at the boot
+		 * case because node's ranges can be interleaved.
 		 */
-		mem_blk->nid = nid;
-
-		ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
-					&mem_blk->dev.kobj,
-					kobject_name(&mem_blk->dev.kobj));
-		if (ret)
-			return ret;
+		page_nid = get_nid_for_pfn(pfn);
+		if (page_nid < 0)
+			continue;
+		if (page_nid != nid)
+			continue;
 
-		return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
-				&node_devices[nid]->dev.kobj,
-				kobject_name(&node_devices[nid]->dev.kobj));
+		return do_register_memory_block_under_node(nid, mem_blk);
 	}
 	/* mem section does not span the specified node */
 	return 0;
 }
 
+/*
+ * During hotplug we know that all pages in the memory block belong to the same
+ * node.
+ */
+static int register_mem_block_under_node_hotplug(struct memory_block *mem_blk,
+						 void *arg)
+{
+	int nid = *(int *)arg;
+
+	return do_register_memory_block_under_node(nid, mem_blk);
+}
+
 /*
  * Unregister a memory block device under the node it spans. Memory blocks
  * with multiple nodes cannot be offlined and therefore also never be removed.
@@ -832,11 +849,19 @@ void unregister_memory_block_under_nodes(struct memory_block *mem_blk)
 			  kobject_name(&node_devices[mem_blk->nid]->dev.kobj));
 }
 
-int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn)
+int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
+		      enum meminit_context context)
 {
+	walk_memory_blocks_func_t func;
+
+	if (context = MEMINIT_HOTPLUG)
+		func = register_mem_block_under_node_hotplug;
+	else
+		func = register_mem_block_under_node_early;
+
 	return walk_memory_blocks(PFN_PHYS(start_pfn),
 				  PFN_PHYS(end_pfn - start_pfn), (void *)&nid,
-				  register_mem_sect_under_node);
+				  func);
 }
 
 #ifdef CONFIG_HUGETLBFS
diff --git a/include/linux/node.h b/include/linux/node.h
index 4866f32a02d8..014ba3ab2efd 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -99,11 +99,13 @@ extern struct node *node_devices[];
 typedef  void (*node_registration_func_t)(struct node *);
 
 #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
-extern int link_mem_sections(int nid, unsigned long start_pfn,
-			     unsigned long end_pfn);
+int link_mem_sections(int nid, unsigned long start_pfn,
+		      unsigned long end_pfn,
+		      enum meminit_context context);
 #else
 static inline int link_mem_sections(int nid, unsigned long start_pfn,
-				    unsigned long end_pfn)
+				    unsigned long end_pfn,
+				    enum meminit_context context)
 {
 	return 0;
 }
@@ -128,7 +130,8 @@ static inline int register_one_node(int nid)
 		if (error)
 			return error;
 		/* link memory sections under this node */
-		error = link_mem_sections(nid, start_pfn, end_pfn);
+		error = link_mem_sections(nid, start_pfn, end_pfn,
+					  MEMINIT_EARLY);
 	}
 
 	return error;
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index fc25886ad719..03df20078827 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1080,7 +1080,8 @@ int __ref add_memory_resource(int nid, struct resource *res)
 	}
 
 	/* link memory sections under this node.*/
-	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1));
+	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1),
+				MEMINIT_HOTPLUG);
 	BUG_ON(ret);
 
 	/* create new memmap entry */
-- 
2.28.0

[PATCH v3 3/3] mm: don't panic when links can't be created in sysfs

[Laurent Dufour]

At boot time, or when doing memory hot-add operations, if the links in
sysfs can't be created, the system is still able to run, so just report the
error in the kernel log rather than BUG_ON and potentially make system
unusable because the callpath can be called with locks held.

Since the number of memory blocks managed could be high, the messages are
rate limited.

As a consequence, link_mem_sections() has no status to report anymore.

Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: David Hildenbrand <david@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/node.c  | 33 +++++++++++++++++++++------------
 include/linux/node.h | 16 +++++++---------
 mm/memory_hotplug.c  |  5 ++---
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 50af16e68d98..9426b0f1f660 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -761,8 +761,8 @@ static int __ref get_nid_for_pfn(unsigned long pfn)
 	return pfn_to_nid(pfn);
 }
 
-static int do_register_memory_block_under_node(int nid,
-					       struct memory_block *mem_blk)
+static void do_register_memory_block_under_node(int nid,
+						struct memory_block *mem_blk)
 {
 	int ret;
 
@@ -775,12 +775,19 @@ static int do_register_memory_block_under_node(int nid,
 	ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
 				       &mem_blk->dev.kobj,
 				       kobject_name(&mem_blk->dev.kobj));
-	if (ret)
-		return ret;
+	if (ret && ret != -EEXIST)
+		dev_err_ratelimited(&node_devices[nid]->dev,
+				    "can't create link to %s in sysfs (%d)\n",
+				    kobject_name(&mem_blk->dev.kobj), ret);
 
-	return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
+	ret = sysfs_create_link_nowarn(&mem_blk->dev.kobj,
 				&node_devices[nid]->dev.kobj,
 				kobject_name(&node_devices[nid]->dev.kobj));
+	if (ret && ret != -EEXIST)
+		dev_err_ratelimited(&mem_blk->dev,
+				    "can't create link to %s in sysfs (%d)\n",
+				    kobject_name(&node_devices[nid]->dev.kobj),
+				    ret);
 }
 
 /* register memory section under specified node if it spans that node */
@@ -816,7 +823,8 @@ static int register_mem_block_under_node_early(struct memory_block *mem_blk,
 		if (page_nid != nid)
 			continue;
 
-		return do_register_memory_block_under_node(nid, mem_blk);
+		do_register_memory_block_under_node(nid, mem_blk);
+		return 0;
 	}
 	/* mem section does not span the specified node */
 	return 0;
@@ -831,7 +839,8 @@ static int register_mem_block_under_node_hotplug(struct memory_block *mem_blk,
 {
 	int nid = *(int *)arg;
 
-	return do_register_memory_block_under_node(nid, mem_blk);
+	do_register_memory_block_under_node(nid, mem_blk);
+	return 0;
 }
 
 /*
@@ -849,8 +858,8 @@ void unregister_memory_block_under_nodes(struct memory_block *mem_blk)
 			  kobject_name(&node_devices[mem_blk->nid]->dev.kobj));
 }
 
-int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
-		      enum meminit_context context)
+void link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
+		       enum meminit_context context)
 {
 	walk_memory_blocks_func_t func;
 
@@ -859,9 +868,9 @@ int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
 	else
 		func = register_mem_block_under_node_early;
 
-	return walk_memory_blocks(PFN_PHYS(start_pfn),
-				  PFN_PHYS(end_pfn - start_pfn), (void *)&nid,
-				  func);
+	walk_memory_blocks(PFN_PHYS(start_pfn), PFN_PHYS(end_pfn - start_pfn),
+			   (void *)&nid, func);
+	return;
 }
 
 #ifdef CONFIG_HUGETLBFS
diff --git a/include/linux/node.h b/include/linux/node.h
index 014ba3ab2efd..8e5a29897936 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -99,15 +99,14 @@ extern struct node *node_devices[];
 typedef  void (*node_registration_func_t)(struct node *);
 
 #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
-int link_mem_sections(int nid, unsigned long start_pfn,
-		      unsigned long end_pfn,
-		      enum meminit_context context);
+void link_mem_sections(int nid, unsigned long start_pfn,
+		       unsigned long end_pfn,
+		       enum meminit_context context);
 #else
-static inline int link_mem_sections(int nid, unsigned long start_pfn,
-				    unsigned long end_pfn,
-				    enum meminit_context context)
+static inline void link_mem_sections(int nid, unsigned long start_pfn,
+				     unsigned long end_pfn,
+				     enum meminit_context context)
 {
-	return 0;
 }
 #endif
 
@@ -130,8 +129,7 @@ static inline int register_one_node(int nid)
 		if (error)
 			return error;
 		/* link memory sections under this node */
-		error = link_mem_sections(nid, start_pfn, end_pfn,
-					  MEMINIT_EARLY);
+		link_mem_sections(nid, start_pfn, end_pfn, MEMINIT_EARLY);
 	}
 
 	return error;
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 03df20078827..01e01a530d38 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1080,9 +1080,8 @@ int __ref add_memory_resource(int nid, struct resource *res)
 	}
 
 	/* link memory sections under this node.*/
-	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1),
-				MEMINIT_HOTPLUG);
-	BUG_ON(ret);
+	link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1),
+			  MEMINIT_HOTPLUG);
 
 	/* create new memmap entry */
 	if (!strcmp(res->name, "System RAM"))
-- 
2.28.0

[PATCH v3 3/3] mm: don't panic when links can't be created in sysfs

[Laurent Dufour]

At boot time, or when doing memory hot-add operations, if the links in
sysfs can't be created, the system is still able to run, so just report the
error in the kernel log rather than BUG_ON and potentially make system
unusable because the callpath can be called with locks held.

Since the number of memory blocks managed could be high, the messages are
rate limited.

As a consequence, link_mem_sections() has no status to report anymore.

Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: David Hildenbrand <david@redhat.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/base/node.c  | 33 +++++++++++++++++++++------------
 include/linux/node.h | 16 +++++++---------
 mm/memory_hotplug.c  |  5 ++---
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 50af16e68d98..9426b0f1f660 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -761,8 +761,8 @@ static int __ref get_nid_for_pfn(unsigned long pfn)
 	return pfn_to_nid(pfn);
 }
 
-static int do_register_memory_block_under_node(int nid,
-					       struct memory_block *mem_blk)
+static void do_register_memory_block_under_node(int nid,
+						struct memory_block *mem_blk)
 {
 	int ret;
 
@@ -775,12 +775,19 @@ static int do_register_memory_block_under_node(int nid,
 	ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
 				       &mem_blk->dev.kobj,
 				       kobject_name(&mem_blk->dev.kobj));
-	if (ret)
-		return ret;
+	if (ret && ret != -EEXIST)
+		dev_err_ratelimited(&node_devices[nid]->dev,
+				    "can't create link to %s in sysfs (%d)\n",
+				    kobject_name(&mem_blk->dev.kobj), ret);
 
-	return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
+	ret = sysfs_create_link_nowarn(&mem_blk->dev.kobj,
 				&node_devices[nid]->dev.kobj,
 				kobject_name(&node_devices[nid]->dev.kobj));
+	if (ret && ret != -EEXIST)
+		dev_err_ratelimited(&mem_blk->dev,
+				    "can't create link to %s in sysfs (%d)\n",
+				    kobject_name(&node_devices[nid]->dev.kobj),
+				    ret);
 }
 
 /* register memory section under specified node if it spans that node */
@@ -816,7 +823,8 @@ static int register_mem_block_under_node_early(struct memory_block *mem_blk,
 		if (page_nid != nid)
 			continue;
 
-		return do_register_memory_block_under_node(nid, mem_blk);
+		do_register_memory_block_under_node(nid, mem_blk);
+		return 0;
 	}
 	/* mem section does not span the specified node */
 	return 0;
@@ -831,7 +839,8 @@ static int register_mem_block_under_node_hotplug(struct memory_block *mem_blk,
 {
 	int nid = *(int *)arg;
 
-	return do_register_memory_block_under_node(nid, mem_blk);
+	do_register_memory_block_under_node(nid, mem_blk);
+	return 0;
 }
 
 /*
@@ -849,8 +858,8 @@ void unregister_memory_block_under_nodes(struct memory_block *mem_blk)
 			  kobject_name(&node_devices[mem_blk->nid]->dev.kobj));
 }
 
-int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
-		      enum meminit_context context)
+void link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
+		       enum meminit_context context)
 {
 	walk_memory_blocks_func_t func;
 
@@ -859,9 +868,9 @@ int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
 	else
 		func = register_mem_block_under_node_early;
 
-	return walk_memory_blocks(PFN_PHYS(start_pfn),
-				  PFN_PHYS(end_pfn - start_pfn), (void *)&nid,
-				  func);
+	walk_memory_blocks(PFN_PHYS(start_pfn), PFN_PHYS(end_pfn - start_pfn),
+			   (void *)&nid, func);
+	return;
 }
 
 #ifdef CONFIG_HUGETLBFS
diff --git a/include/linux/node.h b/include/linux/node.h
index 014ba3ab2efd..8e5a29897936 100644
--- a/include/linux/node.h
+++ b/include/linux/node.h
@@ -99,15 +99,14 @@ extern struct node *node_devices[];
 typedef  void (*node_registration_func_t)(struct node *);
 
 #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
-int link_mem_sections(int nid, unsigned long start_pfn,
-		      unsigned long end_pfn,
-		      enum meminit_context context);
+void link_mem_sections(int nid, unsigned long start_pfn,
+		       unsigned long end_pfn,
+		       enum meminit_context context);
 #else
-static inline int link_mem_sections(int nid, unsigned long start_pfn,
-				    unsigned long end_pfn,
-				    enum meminit_context context)
+static inline void link_mem_sections(int nid, unsigned long start_pfn,
+				     unsigned long end_pfn,
+				     enum meminit_context context)
 {
-	return 0;
 }
 #endif
 
@@ -130,8 +129,7 @@ static inline int register_one_node(int nid)
 		if (error)
 			return error;
 		/* link memory sections under this node */
-		error = link_mem_sections(nid, start_pfn, end_pfn,
-					  MEMINIT_EARLY);
+		link_mem_sections(nid, start_pfn, end_pfn, MEMINIT_EARLY);
 	}
 
 	return error;
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 03df20078827..01e01a530d38 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1080,9 +1080,8 @@ int __ref add_memory_resource(int nid, struct resource *res)
 	}
 
 	/* link memory sections under this node.*/
-	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1),
-				MEMINIT_HOTPLUG);
-	BUG_ON(ret);
+	link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1),
+			  MEMINIT_HOTPLUG);
 
 	/* create new memmap entry */
 	if (!strcmp(res->name, "System RAM"))
-- 
2.28.0

Re: [PATCH v3 1/3] mm: replace memmap_context by memplug_context

[David Hildenbrand]

On 15.09.20 11:41, Laurent Dufour wrote:
> The memmap_context is used to detect whether a memory operation is due to a
> hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it at memplug_context.
> 
> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>  arch/ia64/mm/init.c    |  6 +++---
>  include/linux/mm.h     |  2 +-
>  include/linux/mmzone.h | 11 ++++++++---
>  mm/memory_hotplug.c    |  2 +-
>  mm/page_alloc.c        | 10 +++++-----
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 0b3fb4c7af29..8e7b8c6c576e 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
>  	if (map_start < map_end)
>  		memmap_init_zone((unsigned long)(map_end - map_start),
>  				 args->nid, args->zone, page_to_pfn(map_start),
> -				 MEMMAP_EARLY, NULL);
> +				 MEMINIT_EARLY, NULL);
>  	return 0;
>  }
>  
> @@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
>  	     unsigned long start_pfn)
>  {
>  	if (!vmem_map) {
> -		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
> -				NULL);
> +		memmap_init_zone(size, nid, zone, start_pfn,
> +				 MEMINIT_EARLY, NULL);
>  	} else {
>  		struct page *start;
>  		struct memmap_init_callback_data args;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1983e08f5906..e942f91ed155 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
>  
>  extern void set_dma_reserve(unsigned long new_dma_reserve);
>  extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
> -		enum memmap_context, struct vmem_altmap *);
> +		enum meminit_context, struct vmem_altmap *);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
>  extern void mem_init(void);
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 8379432f4f2f..0f7a4ff4b059 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
>  		unsigned int alloc_flags);
>  bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
>  		unsigned long mark, int highest_zoneidx);
> -enum memmap_context {
> -	MEMMAP_EARLY,
> -	MEMMAP_HOTPLUG,
> +/*
> + * Memory initialization context, use to differentiate memory added by
> + * the platform statically or via memory hotplug interface.
> + */
> +enum meminit_context {
> +	MEMINIT_EARLY,
> +	MEMINIT_HOTPLUG,
>  };
> +
>  extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
>  				     unsigned long size);
>  
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e9d5ab5d3ca0..fc25886ad719 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  	 * are reserved so nobody should be touching them so we should be safe
>  	 */
>  	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
> -			MEMMAP_HOTPLUG, altmap);
> +			 MEMINIT_HOTPLUG, altmap);
>  
>  	set_zone_contiguous(zone);
>  }
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fab5e97dc9ca..5661fa164f13 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
>   * done. Non-atomic initialization, single-pass.
>   */
>  void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> -		unsigned long start_pfn, enum memmap_context context,
> +		unsigned long start_pfn, enum meminit_context context,
>  		struct vmem_altmap *altmap)
>  {
>  	unsigned long pfn, end_pfn = start_pfn + size;
> @@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  		 * There can be holes in boot-time mem_map[]s handed to this
>  		 * function.  They do not exist on hotplugged memory.
>  		 */
> -		if (context == MEMMAP_EARLY) {
> +		if (context == MEMINIT_EARLY) {
>  			if (overlap_memmap_init(zone, &pfn))
>  				continue;
>  			if (defer_init(nid, pfn, end_pfn))
> @@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  
>  		page = pfn_to_page(pfn);
>  		__init_single_page(page, pfn, zone, nid);
> -		if (context == MEMMAP_HOTPLUG)
> +		if (context == MEMINIT_HOTPLUG)
>  			__SetPageReserved(page);
>  
>  		/*
> @@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  		 * check here not to call set_pageblock_migratetype() against
>  		 * pfn out of zone.
>  		 *
> -		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
> +		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
>  		 * because this is done early in section_activate()
>  		 */
>  		if (!(pfn & (pageblock_nr_pages - 1))) {
> @@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
>  		if (end_pfn > start_pfn) {
>  			size = end_pfn - start_pfn;
>  			memmap_init_zone(size, nid, zone, start_pfn,
> -					 MEMMAP_EARLY, NULL);
> +					 MEMINIT_EARLY, NULL);
>  		}
>  	}
>  }
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v3 1/3] mm: replace memmap_context by memplug_context

[David Hildenbrand]

On 15.09.20 11:41, Laurent Dufour wrote:
> The memmap_context is used to detect whether a memory operation is due to a
> hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it at memplug_context.
> 
> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>  arch/ia64/mm/init.c    |  6 +++---
>  include/linux/mm.h     |  2 +-
>  include/linux/mmzone.h | 11 ++++++++---
>  mm/memory_hotplug.c    |  2 +-
>  mm/page_alloc.c        | 10 +++++-----
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 0b3fb4c7af29..8e7b8c6c576e 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
>  	if (map_start < map_end)
>  		memmap_init_zone((unsigned long)(map_end - map_start),
>  				 args->nid, args->zone, page_to_pfn(map_start),
> -				 MEMMAP_EARLY, NULL);
> +				 MEMINIT_EARLY, NULL);
>  	return 0;
>  }
>  
> @@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
>  	     unsigned long start_pfn)
>  {
>  	if (!vmem_map) {
> -		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
> -				NULL);
> +		memmap_init_zone(size, nid, zone, start_pfn,
> +				 MEMINIT_EARLY, NULL);
>  	} else {
>  		struct page *start;
>  		struct memmap_init_callback_data args;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1983e08f5906..e942f91ed155 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
>  
>  extern void set_dma_reserve(unsigned long new_dma_reserve);
>  extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
> -		enum memmap_context, struct vmem_altmap *);
> +		enum meminit_context, struct vmem_altmap *);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
>  extern void mem_init(void);
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 8379432f4f2f..0f7a4ff4b059 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
>  		unsigned int alloc_flags);
>  bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
>  		unsigned long mark, int highest_zoneidx);
> -enum memmap_context {
> -	MEMMAP_EARLY,
> -	MEMMAP_HOTPLUG,
> +/*
> + * Memory initialization context, use to differentiate memory added by
> + * the platform statically or via memory hotplug interface.
> + */
> +enum meminit_context {
> +	MEMINIT_EARLY,
> +	MEMINIT_HOTPLUG,
>  };
> +
>  extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
>  				     unsigned long size);
>  
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e9d5ab5d3ca0..fc25886ad719 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  	 * are reserved so nobody should be touching them so we should be safe
>  	 */
>  	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
> -			MEMMAP_HOTPLUG, altmap);
> +			 MEMINIT_HOTPLUG, altmap);
>  
>  	set_zone_contiguous(zone);
>  }
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fab5e97dc9ca..5661fa164f13 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
>   * done. Non-atomic initialization, single-pass.
>   */
>  void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> -		unsigned long start_pfn, enum memmap_context context,
> +		unsigned long start_pfn, enum meminit_context context,
>  		struct vmem_altmap *altmap)
>  {
>  	unsigned long pfn, end_pfn = start_pfn + size;
> @@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  		 * There can be holes in boot-time mem_map[]s handed to this
>  		 * function.  They do not exist on hotplugged memory.
>  		 */
> -		if (context = MEMMAP_EARLY) {
> +		if (context = MEMINIT_EARLY) {
>  			if (overlap_memmap_init(zone, &pfn))
>  				continue;
>  			if (defer_init(nid, pfn, end_pfn))
> @@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  
>  		page = pfn_to_page(pfn);
>  		__init_single_page(page, pfn, zone, nid);
> -		if (context = MEMMAP_HOTPLUG)
> +		if (context = MEMINIT_HOTPLUG)
>  			__SetPageReserved(page);
>  
>  		/*
> @@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  		 * check here not to call set_pageblock_migratetype() against
>  		 * pfn out of zone.
>  		 *
> -		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
> +		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
>  		 * because this is done early in section_activate()
>  		 */
>  		if (!(pfn & (pageblock_nr_pages - 1))) {
> @@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
>  		if (end_pfn > start_pfn) {
>  			size = end_pfn - start_pfn;
>  			memmap_init_zone(size, nid, zone, start_pfn,
> -					 MEMMAP_EARLY, NULL);
> +					 MEMINIT_EARLY, NULL);
>  		}
>  	}
>  }
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v3 1/3] mm: replace memmap_context by memplug_context

[Oscar Salvador]

On Tue, Sep 15, 2020 at 11:41:41AM +0200, Laurent Dufour wrote:
> The memmap_context is used to detect whether a memory operation is due to a
> hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it at memplug_context.
> 
> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v3 1/3] mm: replace memmap_context by memplug_context

[Oscar Salvador]

On Tue, Sep 15, 2020 at 11:41:41AM +0200, Laurent Dufour wrote:
> The memmap_context is used to detect whether a memory operation is due to a
> hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it at memplug_context.
> 
> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v3 2/3] mm: don't rely on system state to detect hot-plug operations

[Oscar Salvador]

On Tue, Sep 15, 2020 at 11:41:42AM +0200, Laurent Dufour wrote:
> [1] According to Oscar Salvador, using this qemu command line, ACPI memory
> hotplug operations are raised at SYSTEM_SCHEDULING state:

I would like to stress that this is not the only way we can end up
hotplugging memor while state = SYSTEM_SCHEDULING.
According to David, we can end up doing this if we reboot a VM
with hotplugged memory.
(And I have seen other virtualization technologies do the same)

 
> Fixes: 4fbce633910e ("mm/memory_hotplug.c: make register_mem_sect_under_node() a callback of walk_memory_range()")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Cc: stable@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oscar Salvador <osalvador@suse.de>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v3 2/3] mm: don't rely on system state to detect hot-plug operations

[Oscar Salvador]

On Tue, Sep 15, 2020 at 11:41:42AM +0200, Laurent Dufour wrote:
> [1] According to Oscar Salvador, using this qemu command line, ACPI memory
> hotplug operations are raised at SYSTEM_SCHEDULING state:

I would like to stress that this is not the only way we can end up
hotplugging memor while state = SYSTEM_SCHEDULING.
According to David, we can end up doing this if we reboot a VM
with hotplugged memory.
(And I have seen other virtualization technologies do the same)

 
> Fixes: 4fbce633910e ("mm/memory_hotplug.c: make register_mem_sect_under_node() a callback of walk_memory_range()")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Cc: stable@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oscar Salvador <osalvador@suse.de>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v3 3/3] mm: don't panic when links can't be created in sysfs

[Oscar Salvador]

On Tue, Sep 15, 2020 at 11:41:43AM +0200, Laurent Dufour wrote:
> At boot time, or when doing memory hot-add operations, if the links in
> sysfs can't be created, the system is still able to run, so just report the
> error in the kernel log rather than BUG_ON and potentially make system
> unusable because the callpath can be called with locks held.
> 
> Since the number of memory blocks managed could be high, the messages are
> rate limited.
> 
> As a consequence, link_mem_sections() has no status to report anymore.
> 
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v3 3/3] mm: don't panic when links can't be created in sysfs

[Oscar Salvador]

On Tue, Sep 15, 2020 at 11:41:43AM +0200, Laurent Dufour wrote:
> At boot time, or when doing memory hot-add operations, if the links in
> sysfs can't be created, the system is still able to run, so just report the
> error in the kernel log rather than BUG_ON and potentially make system
> unusable because the callpath can be called with locks held.
> 
> Since the number of memory blocks managed could be high, the messages are
> rate limited.
> 
> As a consequence, link_mem_sections() has no status to report anymore.
> 
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Acked-by: David Hildenbrand <david@redhat.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Oscar Salvador <osalvador@suse.de>

-- 
Oscar Salvador
SUSE L3

Re: [PATCH v3 1/3] mm: replace memmap_context by memplug_context

[Michal Hocko]

On Tue 15-09-20 11:41:41, Laurent Dufour wrote:
> The memmap_context is used to detect whether a memory operation is due to a
> hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it at memplug_context.

s@memplug_context@meminit_context@g

> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>  arch/ia64/mm/init.c    |  6 +++---
>  include/linux/mm.h     |  2 +-
>  include/linux/mmzone.h | 11 ++++++++---
>  mm/memory_hotplug.c    |  2 +-
>  mm/page_alloc.c        | 10 +++++-----
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 0b3fb4c7af29..8e7b8c6c576e 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
>  	if (map_start < map_end)
>  		memmap_init_zone((unsigned long)(map_end - map_start),
>  				 args->nid, args->zone, page_to_pfn(map_start),
> -				 MEMMAP_EARLY, NULL);
> +				 MEMINIT_EARLY, NULL);
>  	return 0;
>  }
>  
> @@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
>  	     unsigned long start_pfn)
>  {
>  	if (!vmem_map) {
> -		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
> -				NULL);
> +		memmap_init_zone(size, nid, zone, start_pfn,
> +				 MEMINIT_EARLY, NULL);
>  	} else {
>  		struct page *start;
>  		struct memmap_init_callback_data args;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1983e08f5906..e942f91ed155 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
>  
>  extern void set_dma_reserve(unsigned long new_dma_reserve);
>  extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
> -		enum memmap_context, struct vmem_altmap *);
> +		enum meminit_context, struct vmem_altmap *);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
>  extern void mem_init(void);
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 8379432f4f2f..0f7a4ff4b059 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
>  		unsigned int alloc_flags);
>  bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
>  		unsigned long mark, int highest_zoneidx);
> -enum memmap_context {
> -	MEMMAP_EARLY,
> -	MEMMAP_HOTPLUG,
> +/*
> + * Memory initialization context, use to differentiate memory added by
> + * the platform statically or via memory hotplug interface.
> + */
> +enum meminit_context {
> +	MEMINIT_EARLY,
> +	MEMINIT_HOTPLUG,
>  };
> +
>  extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
>  				     unsigned long size);
>  
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e9d5ab5d3ca0..fc25886ad719 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  	 * are reserved so nobody should be touching them so we should be safe
>  	 */
>  	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
> -			MEMMAP_HOTPLUG, altmap);
> +			 MEMINIT_HOTPLUG, altmap);
>  
>  	set_zone_contiguous(zone);
>  }
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fab5e97dc9ca..5661fa164f13 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
>   * done. Non-atomic initialization, single-pass.
>   */
>  void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> -		unsigned long start_pfn, enum memmap_context context,
> +		unsigned long start_pfn, enum meminit_context context,
>  		struct vmem_altmap *altmap)
>  {
>  	unsigned long pfn, end_pfn = start_pfn + size;
> @@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  		 * There can be holes in boot-time mem_map[]s handed to this
>  		 * function.  They do not exist on hotplugged memory.
>  		 */
> -		if (context == MEMMAP_EARLY) {
> +		if (context == MEMINIT_EARLY) {
>  			if (overlap_memmap_init(zone, &pfn))
>  				continue;
>  			if (defer_init(nid, pfn, end_pfn))
> @@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  
>  		page = pfn_to_page(pfn);
>  		__init_single_page(page, pfn, zone, nid);
> -		if (context == MEMMAP_HOTPLUG)
> +		if (context == MEMINIT_HOTPLUG)
>  			__SetPageReserved(page);
>  
>  		/*
> @@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  		 * check here not to call set_pageblock_migratetype() against
>  		 * pfn out of zone.
>  		 *
> -		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
> +		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
>  		 * because this is done early in section_activate()
>  		 */
>  		if (!(pfn & (pageblock_nr_pages - 1))) {
> @@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
>  		if (end_pfn > start_pfn) {
>  			size = end_pfn - start_pfn;
>  			memmap_init_zone(size, nid, zone, start_pfn,
> -					 MEMMAP_EARLY, NULL);
> +					 MEMINIT_EARLY, NULL);
>  		}
>  	}
>  }
> -- 
> 2.28.0
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v3 1/3] mm: replace memmap_context by memplug_context

[Michal Hocko]

On Tue 15-09-20 11:41:41, Laurent Dufour wrote:
> The memmap_context is used to detect whether a memory operation is due to a
> hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it at memplug_context.

s@memplug_context@meminit_context@g

> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>  arch/ia64/mm/init.c    |  6 +++---
>  include/linux/mm.h     |  2 +-
>  include/linux/mmzone.h | 11 ++++++++---
>  mm/memory_hotplug.c    |  2 +-
>  mm/page_alloc.c        | 10 +++++-----
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 0b3fb4c7af29..8e7b8c6c576e 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
>  	if (map_start < map_end)
>  		memmap_init_zone((unsigned long)(map_end - map_start),
>  				 args->nid, args->zone, page_to_pfn(map_start),
> -				 MEMMAP_EARLY, NULL);
> +				 MEMINIT_EARLY, NULL);
>  	return 0;
>  }
>  
> @@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
>  	     unsigned long start_pfn)
>  {
>  	if (!vmem_map) {
> -		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
> -				NULL);
> +		memmap_init_zone(size, nid, zone, start_pfn,
> +				 MEMINIT_EARLY, NULL);
>  	} else {
>  		struct page *start;
>  		struct memmap_init_callback_data args;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1983e08f5906..e942f91ed155 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
>  
>  extern void set_dma_reserve(unsigned long new_dma_reserve);
>  extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
> -		enum memmap_context, struct vmem_altmap *);
> +		enum meminit_context, struct vmem_altmap *);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
>  extern void mem_init(void);
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 8379432f4f2f..0f7a4ff4b059 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
>  		unsigned int alloc_flags);
>  bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
>  		unsigned long mark, int highest_zoneidx);
> -enum memmap_context {
> -	MEMMAP_EARLY,
> -	MEMMAP_HOTPLUG,
> +/*
> + * Memory initialization context, use to differentiate memory added by
> + * the platform statically or via memory hotplug interface.
> + */
> +enum meminit_context {
> +	MEMINIT_EARLY,
> +	MEMINIT_HOTPLUG,
>  };
> +
>  extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
>  				     unsigned long size);
>  
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e9d5ab5d3ca0..fc25886ad719 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  	 * are reserved so nobody should be touching them so we should be safe
>  	 */
>  	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
> -			MEMMAP_HOTPLUG, altmap);
> +			 MEMINIT_HOTPLUG, altmap);
>  
>  	set_zone_contiguous(zone);
>  }
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fab5e97dc9ca..5661fa164f13 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
>   * done. Non-atomic initialization, single-pass.
>   */
>  void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> -		unsigned long start_pfn, enum memmap_context context,
> +		unsigned long start_pfn, enum meminit_context context,
>  		struct vmem_altmap *altmap)
>  {
>  	unsigned long pfn, end_pfn = start_pfn + size;
> @@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  		 * There can be holes in boot-time mem_map[]s handed to this
>  		 * function.  They do not exist on hotplugged memory.
>  		 */
> -		if (context = MEMMAP_EARLY) {
> +		if (context = MEMINIT_EARLY) {
>  			if (overlap_memmap_init(zone, &pfn))
>  				continue;
>  			if (defer_init(nid, pfn, end_pfn))
> @@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  
>  		page = pfn_to_page(pfn);
>  		__init_single_page(page, pfn, zone, nid);
> -		if (context = MEMMAP_HOTPLUG)
> +		if (context = MEMINIT_HOTPLUG)
>  			__SetPageReserved(page);
>  
>  		/*
> @@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  		 * check here not to call set_pageblock_migratetype() against
>  		 * pfn out of zone.
>  		 *
> -		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
> +		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
>  		 * because this is done early in section_activate()
>  		 */
>  		if (!(pfn & (pageblock_nr_pages - 1))) {
> @@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
>  		if (end_pfn > start_pfn) {
>  			size = end_pfn - start_pfn;
>  			memmap_init_zone(size, nid, zone, start_pfn,
> -					 MEMMAP_EARLY, NULL);
> +					 MEMINIT_EARLY, NULL);
>  		}
>  	}
>  }
> -- 
> 2.28.0
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v3 2/3] mm: don't rely on system state to detect hot-plug operations

[Michal Hocko]

On Tue 15-09-20 11:41:42, Laurent Dufour wrote:
> In register_mem_sect_under_node() the system_state’s value is checked to
> detect whether the call is made during boot time or during an hot-plug
> operation. Unfortunately, that check against SYSTEM_BOOTING is wrong
> because regular memory is registered at SYSTEM_SCHEDULING state. In
> addition, memory hot-plug operation can be triggered at this system state
> by the ACPI [1]. So checking against the system state is not enough.
> 
> The consequence is that on system with interleaved node's ranges like this:
>  Early memory node ranges
>    node   1: [mem 0x0000000000000000-0x000000011fffffff]
>    node   2: [mem 0x0000000120000000-0x000000014fffffff]
>    node   1: [mem 0x0000000150000000-0x00000001ffffffff]
>    node   0: [mem 0x0000000200000000-0x000000048fffffff]
>    node   2: [mem 0x0000000490000000-0x00000007ffffffff]
> 
> This can be seen on PowerPC LPAR after multiple memory hot-plug and
> hot-unplug operations are done. At the next reboot the node's memory ranges
> can be interleaved and since the call to link_mem_sections() is made in
> topology_init() while the system is in the SYSTEM_SCHEDULING state, the
> node's id is not checked, and the sections registered to multiple nodes:
> 
> $ ls -l /sys/devices/system/memory/memory21/node*
> total 0
> lrwxrwxrwx 1 root root     0 Aug 24 05:27 node1 -> ../../node/node1
> lrwxrwxrwx 1 root root     0 Aug 24 05:27 node2 -> ../../node/node2
> 
> In that case, the system is able to boot but if later one of theses memory
> blocks is hot-unplugged and then hot-plugged, the sysfs inconsistency is
> detected and this is triggering a BUG_ON():
> 
> ------------[ cut here ]------------
> kernel BUG at /Users/laurent/src/linux-ppc/mm/memory_hotplug.c:1084!
> Oops: Exception in kernel mode, sig: 5 [#1]
> LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
> Modules linked in: rpadlpar_io rpaphp pseries_rng rng_core vmx_crypto gf128mul binfmt_misc ip_tables x_tables xfs libcrc32c crc32c_vpmsum autofs4
> CPU: 8 PID: 10256 Comm: drmgr Not tainted 5.9.0-rc1+ #25
> NIP:  c000000000403f34 LR: c000000000403f2c CTR: 0000000000000000
> REGS: c0000004876e3660 TRAP: 0700   Not tainted  (5.9.0-rc1+)
> MSR:  800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24000448  XER: 20040000
> CFAR: c000000000846d20 IRQMASK: 0
> GPR00: c000000000403f2c c0000004876e38f0 c0000000012f6f00 ffffffffffffffef
> GPR04: 0000000000000227 c0000004805ae680 0000000000000000 00000004886f0000
> GPR08: 0000000000000226 0000000000000003 0000000000000002 fffffffffffffffd
> GPR12: 0000000088000484 c00000001ec96280 0000000000000000 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000004 0000000000000003
> GPR20: c00000047814ffe0 c0000007ffff7c08 0000000000000010 c0000000013332c8
> GPR24: 0000000000000000 c0000000011f6cc0 0000000000000000 0000000000000000
> GPR28: ffffffffffffffef 0000000000000001 0000000150000000 0000000010000000
> NIP [c000000000403f34] add_memory_resource+0x244/0x340
> LR [c000000000403f2c] add_memory_resource+0x23c/0x340
> Call Trace:
> [c0000004876e38f0] [c000000000403f2c] add_memory_resource+0x23c/0x340 (unreliable)
> [c0000004876e39c0] [c00000000040408c] __add_memory+0x5c/0xf0
> [c0000004876e39f0] [c0000000000e2b94] dlpar_add_lmb+0x1b4/0x500
> [c0000004876e3ad0] [c0000000000e3888] dlpar_memory+0x1f8/0xb80
> [c0000004876e3b60] [c0000000000dc0d0] handle_dlpar_errorlog+0xc0/0x190
> [c0000004876e3bd0] [c0000000000dc398] dlpar_store+0x198/0x4a0
> [c0000004876e3c90] [c00000000072e630] kobj_attr_store+0x30/0x50
> [c0000004876e3cb0] [c00000000051f954] sysfs_kf_write+0x64/0x90
> [c0000004876e3cd0] [c00000000051ee40] kernfs_fop_write+0x1b0/0x290
> [c0000004876e3d20] [c000000000438dd8] vfs_write+0xe8/0x290
> [c0000004876e3d70] [c0000000004391ac] ksys_write+0xdc/0x130
> [c0000004876e3dc0] [c000000000034e40] system_call_exception+0x160/0x270
> [c0000004876e3e20] [c00000000000d740] system_call_common+0xf0/0x27c
> Instruction dump:
> 48442e35 60000000 0b030000 3cbe0001 7fa3eb78 7bc48402 38a5fffe 7ca5fa14
> 78a58402 48442db1 60000000 7c7c1b78 <0b030000> 7f23cb78 4bda371d 60000000
> ---[ end trace 562fd6c109cd0fb2 ]---
> 
> This patch addresses the root cause by not relying on the system_state
> value to detect whether the call is due to a hot-plug operation. An extra
> parameter is added to link_mem_sections() detailing whether the operation
> is due to a hot-plug operation.
> 
> [1] According to Oscar Salvador, using this qemu command line, ACPI memory
> hotplug operations are raised at SYSTEM_SCHEDULING state:
> 
> $QEMU -enable-kvm -machine pc -smp 4,sockets=4,cores=1,threads=1 -cpu host -monitor pty \
>         -m size=$MEM,slots=255,maxmem=4294967296k  \
>         -numa node,nodeid=0,cpus=0-3,mem=512 -numa node,nodeid=1,mem=512 \
>         -object memory-backend-ram,id=memdimm0,size=134217728 -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \
>         -object memory-backend-ram,id=memdimm1,size=134217728 -device pc-dimm,node=0,memdev=memdimm1,id=dimm1,slot=1 \
>         -object memory-backend-ram,id=memdimm2,size=134217728 -device pc-dimm,node=0,memdev=memdimm2,id=dimm2,slot=2 \
>         -object memory-backend-ram,id=memdimm3,size=134217728 -device pc-dimm,node=0,memdev=memdimm3,id=dimm3,slot=3 \
>         -object memory-backend-ram,id=memdimm4,size=134217728 -device pc-dimm,node=1,memdev=memdimm4,id=dimm4,slot=4 \
>         -object memory-backend-ram,id=memdimm5,size=134217728 -device pc-dimm,node=1,memdev=memdimm5,id=dimm5,slot=5 \
>         -object memory-backend-ram,id=memdimm6,size=134217728 -device pc-dimm,node=1,memdev=memdimm6,id=dimm6,slot=6 \
> 
> Fixes: 4fbce633910e ("mm/memory_hotplug.c: make register_mem_sect_under_node() a callback of walk_memory_range()")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Cc: stable@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oscar Salvador <osalvador@suse.de>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  drivers/base/node.c  | 85 ++++++++++++++++++++++++++++----------------
>  include/linux/node.h | 11 +++---
>  mm/memory_hotplug.c  |  3 +-
>  3 files changed, 64 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 508b80f6329b..50af16e68d98 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -761,14 +761,36 @@ static int __ref get_nid_for_pfn(unsigned long pfn)
>  	return pfn_to_nid(pfn);
>  }
>  
> +static int do_register_memory_block_under_node(int nid,
> +					       struct memory_block *mem_blk)
> +{
> +	int ret;
> +
> +	/*
> +	 * If this memory block spans multiple nodes, we only indicate
> +	 * the last processed node.
> +	 */
> +	mem_blk->nid = nid;
> +
> +	ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
> +				       &mem_blk->dev.kobj,
> +				       kobject_name(&mem_blk->dev.kobj));
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
> +				&node_devices[nid]->dev.kobj,
> +				kobject_name(&node_devices[nid]->dev.kobj));
> +}
> +
>  /* register memory section under specified node if it spans that node */
> -static int register_mem_sect_under_node(struct memory_block *mem_blk,
> -					 void *arg)
> +static int register_mem_block_under_node_early(struct memory_block *mem_blk,
> +					       void *arg)
>  {
>  	unsigned long memory_block_pfns = memory_block_size_bytes() / PAGE_SIZE;
>  	unsigned long start_pfn = section_nr_to_pfn(mem_blk->start_section_nr);
>  	unsigned long end_pfn = start_pfn + memory_block_pfns - 1;
> -	int ret, nid = *(int *)arg;
> +	int nid = *(int *)arg;
>  	unsigned long pfn;
>  
>  	for (pfn = start_pfn; pfn <= end_pfn; pfn++) {
> @@ -785,38 +807,33 @@ static int register_mem_sect_under_node(struct memory_block *mem_blk,
>  		}
>  
>  		/*
> -		 * We need to check if page belongs to nid only for the boot
> -		 * case, during hotplug we know that all pages in the memory
> -		 * block belong to the same node.
> -		 */
> -		if (system_state == SYSTEM_BOOTING) {
> -			page_nid = get_nid_for_pfn(pfn);
> -			if (page_nid < 0)
> -				continue;
> -			if (page_nid != nid)
> -				continue;
> -		}
> -
> -		/*
> -		 * If this memory block spans multiple nodes, we only indicate
> -		 * the last processed node.
> +		 * We need to check if page belongs to nid only at the boot
> +		 * case because node's ranges can be interleaved.
>  		 */
> -		mem_blk->nid = nid;
> -
> -		ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
> -					&mem_blk->dev.kobj,
> -					kobject_name(&mem_blk->dev.kobj));
> -		if (ret)
> -			return ret;
> +		page_nid = get_nid_for_pfn(pfn);
> +		if (page_nid < 0)
> +			continue;
> +		if (page_nid != nid)
> +			continue;
>  
> -		return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
> -				&node_devices[nid]->dev.kobj,
> -				kobject_name(&node_devices[nid]->dev.kobj));
> +		return do_register_memory_block_under_node(nid, mem_blk);
>  	}
>  	/* mem section does not span the specified node */
>  	return 0;
>  }
>  
> +/*
> + * During hotplug we know that all pages in the memory block belong to the same
> + * node.
> + */
> +static int register_mem_block_under_node_hotplug(struct memory_block *mem_blk,
> +						 void *arg)
> +{
> +	int nid = *(int *)arg;
> +
> +	return do_register_memory_block_under_node(nid, mem_blk);
> +}
> +
>  /*
>   * Unregister a memory block device under the node it spans. Memory blocks
>   * with multiple nodes cannot be offlined and therefore also never be removed.
> @@ -832,11 +849,19 @@ void unregister_memory_block_under_nodes(struct memory_block *mem_blk)
>  			  kobject_name(&node_devices[mem_blk->nid]->dev.kobj));
>  }
>  
> -int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn)
> +int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
> +		      enum meminit_context context)
>  {
> +	walk_memory_blocks_func_t func;
> +
> +	if (context == MEMINIT_HOTPLUG)
> +		func = register_mem_block_under_node_hotplug;
> +	else
> +		func = register_mem_block_under_node_early;
> +
>  	return walk_memory_blocks(PFN_PHYS(start_pfn),
>  				  PFN_PHYS(end_pfn - start_pfn), (void *)&nid,
> -				  register_mem_sect_under_node);
> +				  func);
>  }
>  
>  #ifdef CONFIG_HUGETLBFS
> diff --git a/include/linux/node.h b/include/linux/node.h
> index 4866f32a02d8..014ba3ab2efd 100644
> --- a/include/linux/node.h
> +++ b/include/linux/node.h
> @@ -99,11 +99,13 @@ extern struct node *node_devices[];
>  typedef  void (*node_registration_func_t)(struct node *);
>  
>  #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
> -extern int link_mem_sections(int nid, unsigned long start_pfn,
> -			     unsigned long end_pfn);
> +int link_mem_sections(int nid, unsigned long start_pfn,
> +		      unsigned long end_pfn,
> +		      enum meminit_context context);
>  #else
>  static inline int link_mem_sections(int nid, unsigned long start_pfn,
> -				    unsigned long end_pfn)
> +				    unsigned long end_pfn,
> +				    enum meminit_context context)
>  {
>  	return 0;
>  }
> @@ -128,7 +130,8 @@ static inline int register_one_node(int nid)
>  		if (error)
>  			return error;
>  		/* link memory sections under this node */
> -		error = link_mem_sections(nid, start_pfn, end_pfn);
> +		error = link_mem_sections(nid, start_pfn, end_pfn,
> +					  MEMINIT_EARLY);
>  	}
>  
>  	return error;
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index fc25886ad719..03df20078827 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1080,7 +1080,8 @@ int __ref add_memory_resource(int nid, struct resource *res)
>  	}
>  
>  	/* link memory sections under this node.*/
> -	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1));
> +	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1),
> +				MEMINIT_HOTPLUG);
>  	BUG_ON(ret);
>  
>  	/* create new memmap entry */
> -- 
> 2.28.0
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v3 2/3] mm: don't rely on system state to detect hot-plug operations

[Michal Hocko]

On Tue 15-09-20 11:41:42, Laurent Dufour wrote:
> In register_mem_sect_under_node() the system_state’s value is checked to
> detect whether the call is made during boot time or during an hot-plug
> operation. Unfortunately, that check against SYSTEM_BOOTING is wrong
> because regular memory is registered at SYSTEM_SCHEDULING state. In
> addition, memory hot-plug operation can be triggered at this system state
> by the ACPI [1]. So checking against the system state is not enough.
> 
> The consequence is that on system with interleaved node's ranges like this:
>  Early memory node ranges
>    node   1: [mem 0x0000000000000000-0x000000011fffffff]
>    node   2: [mem 0x0000000120000000-0x000000014fffffff]
>    node   1: [mem 0x0000000150000000-0x00000001ffffffff]
>    node   0: [mem 0x0000000200000000-0x000000048fffffff]
>    node   2: [mem 0x0000000490000000-0x00000007ffffffff]
> 
> This can be seen on PowerPC LPAR after multiple memory hot-plug and
> hot-unplug operations are done. At the next reboot the node's memory ranges
> can be interleaved and since the call to link_mem_sections() is made in
> topology_init() while the system is in the SYSTEM_SCHEDULING state, the
> node's id is not checked, and the sections registered to multiple nodes:
> 
> $ ls -l /sys/devices/system/memory/memory21/node*
> total 0
> lrwxrwxrwx 1 root root     0 Aug 24 05:27 node1 -> ../../node/node1
> lrwxrwxrwx 1 root root     0 Aug 24 05:27 node2 -> ../../node/node2
> 
> In that case, the system is able to boot but if later one of theses memory
> blocks is hot-unplugged and then hot-plugged, the sysfs inconsistency is
> detected and this is triggering a BUG_ON():
> 
> ------------[ cut here ]------------
> kernel BUG at /Users/laurent/src/linux-ppc/mm/memory_hotplug.c:1084!
> Oops: Exception in kernel mode, sig: 5 [#1]
> LE PAGE_SIZEdK MMU=Hash SMP NR_CPUS 48 NUMA pSeries
> Modules linked in: rpadlpar_io rpaphp pseries_rng rng_core vmx_crypto gf128mul binfmt_misc ip_tables x_tables xfs libcrc32c crc32c_vpmsum autofs4
> CPU: 8 PID: 10256 Comm: drmgr Not tainted 5.9.0-rc1+ #25
> NIP:  c000000000403f34 LR: c000000000403f2c CTR: 0000000000000000
> REGS: c0000004876e3660 TRAP: 0700   Not tainted  (5.9.0-rc1+)
> MSR:  800000000282b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE>  CR: 24000448  XER: 20040000
> CFAR: c000000000846d20 IRQMASK: 0
> GPR00: c000000000403f2c c0000004876e38f0 c0000000012f6f00 ffffffffffffffef
> GPR04: 0000000000000227 c0000004805ae680 0000000000000000 00000004886f0000
> GPR08: 0000000000000226 0000000000000003 0000000000000002 fffffffffffffffd
> GPR12: 0000000088000484 c00000001ec96280 0000000000000000 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000004 0000000000000003
> GPR20: c00000047814ffe0 c0000007ffff7c08 0000000000000010 c0000000013332c8
> GPR24: 0000000000000000 c0000000011f6cc0 0000000000000000 0000000000000000
> GPR28: ffffffffffffffef 0000000000000001 0000000150000000 0000000010000000
> NIP [c000000000403f34] add_memory_resource+0x244/0x340
> LR [c000000000403f2c] add_memory_resource+0x23c/0x340
> Call Trace:
> [c0000004876e38f0] [c000000000403f2c] add_memory_resource+0x23c/0x340 (unreliable)
> [c0000004876e39c0] [c00000000040408c] __add_memory+0x5c/0xf0
> [c0000004876e39f0] [c0000000000e2b94] dlpar_add_lmb+0x1b4/0x500
> [c0000004876e3ad0] [c0000000000e3888] dlpar_memory+0x1f8/0xb80
> [c0000004876e3b60] [c0000000000dc0d0] handle_dlpar_errorlog+0xc0/0x190
> [c0000004876e3bd0] [c0000000000dc398] dlpar_store+0x198/0x4a0
> [c0000004876e3c90] [c00000000072e630] kobj_attr_store+0x30/0x50
> [c0000004876e3cb0] [c00000000051f954] sysfs_kf_write+0x64/0x90
> [c0000004876e3cd0] [c00000000051ee40] kernfs_fop_write+0x1b0/0x290
> [c0000004876e3d20] [c000000000438dd8] vfs_write+0xe8/0x290
> [c0000004876e3d70] [c0000000004391ac] ksys_write+0xdc/0x130
> [c0000004876e3dc0] [c000000000034e40] system_call_exception+0x160/0x270
> [c0000004876e3e20] [c00000000000d740] system_call_common+0xf0/0x27c
> Instruction dump:
> 48442e35 60000000 0b030000 3cbe0001 7fa3eb78 7bc48402 38a5fffe 7ca5fa14
> 78a58402 48442db1 60000000 7c7c1b78 <0b030000> 7f23cb78 4bda371d 60000000
> ---[ end trace 562fd6c109cd0fb2 ]---
> 
> This patch addresses the root cause by not relying on the system_state
> value to detect whether the call is due to a hot-plug operation. An extra
> parameter is added to link_mem_sections() detailing whether the operation
> is due to a hot-plug operation.
> 
> [1] According to Oscar Salvador, using this qemu command line, ACPI memory
> hotplug operations are raised at SYSTEM_SCHEDULING state:
> 
> $QEMU -enable-kvm -machine pc -smp 4,sockets=4,cores=1,threads=1 -cpu host -monitor pty \
>         -m size=$MEM,slots%5,maxmemB94967296k  \
>         -numa node,nodeid=0,cpus=0-3,memQ2 -numa node,nodeid=1,memQ2 \
>         -object memory-backend-ram,id=memdimm0,size\x134217728 -device pc-dimm,node=0,memdev=memdimm0,id=dimm0,slot=0 \
>         -object memory-backend-ram,id=memdimm1,size\x134217728 -device pc-dimm,node=0,memdev=memdimm1,id=dimm1,slot=1 \
>         -object memory-backend-ram,id=memdimm2,size\x134217728 -device pc-dimm,node=0,memdev=memdimm2,id=dimm2,slot=2 \
>         -object memory-backend-ram,id=memdimm3,size\x134217728 -device pc-dimm,node=0,memdev=memdimm3,id=dimm3,slot=3 \
>         -object memory-backend-ram,id=memdimm4,size\x134217728 -device pc-dimm,node=1,memdev=memdimm4,id=dimm4,slot=4 \
>         -object memory-backend-ram,id=memdimm5,size\x134217728 -device pc-dimm,node=1,memdev=memdimm5,id=dimm5,slot=5 \
>         -object memory-backend-ram,id=memdimm6,size\x134217728 -device pc-dimm,node=1,memdev=memdimm6,id=dimm6,slot=6 \
> 
> Fixes: 4fbce633910e ("mm/memory_hotplug.c: make register_mem_sect_under_node() a callback of walk_memory_range()")
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Cc: stable@vger.kernel.org
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: "Rafael J. Wysocki" <rafael@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oscar Salvador <osalvador@suse.de>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  drivers/base/node.c  | 85 ++++++++++++++++++++++++++++----------------
>  include/linux/node.h | 11 +++---
>  mm/memory_hotplug.c  |  3 +-
>  3 files changed, 64 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 508b80f6329b..50af16e68d98 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -761,14 +761,36 @@ static int __ref get_nid_for_pfn(unsigned long pfn)
>  	return pfn_to_nid(pfn);
>  }
>  
> +static int do_register_memory_block_under_node(int nid,
> +					       struct memory_block *mem_blk)
> +{
> +	int ret;
> +
> +	/*
> +	 * If this memory block spans multiple nodes, we only indicate
> +	 * the last processed node.
> +	 */
> +	mem_blk->nid = nid;
> +
> +	ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
> +				       &mem_blk->dev.kobj,
> +				       kobject_name(&mem_blk->dev.kobj));
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
> +				&node_devices[nid]->dev.kobj,
> +				kobject_name(&node_devices[nid]->dev.kobj));
> +}
> +
>  /* register memory section under specified node if it spans that node */
> -static int register_mem_sect_under_node(struct memory_block *mem_blk,
> -					 void *arg)
> +static int register_mem_block_under_node_early(struct memory_block *mem_blk,
> +					       void *arg)
>  {
>  	unsigned long memory_block_pfns = memory_block_size_bytes() / PAGE_SIZE;
>  	unsigned long start_pfn = section_nr_to_pfn(mem_blk->start_section_nr);
>  	unsigned long end_pfn = start_pfn + memory_block_pfns - 1;
> -	int ret, nid = *(int *)arg;
> +	int nid = *(int *)arg;
>  	unsigned long pfn;
>  
>  	for (pfn = start_pfn; pfn <= end_pfn; pfn++) {
> @@ -785,38 +807,33 @@ static int register_mem_sect_under_node(struct memory_block *mem_blk,
>  		}
>  
>  		/*
> -		 * We need to check if page belongs to nid only for the boot
> -		 * case, during hotplug we know that all pages in the memory
> -		 * block belong to the same node.
> -		 */
> -		if (system_state = SYSTEM_BOOTING) {
> -			page_nid = get_nid_for_pfn(pfn);
> -			if (page_nid < 0)
> -				continue;
> -			if (page_nid != nid)
> -				continue;
> -		}
> -
> -		/*
> -		 * If this memory block spans multiple nodes, we only indicate
> -		 * the last processed node.
> +		 * We need to check if page belongs to nid only at the boot
> +		 * case because node's ranges can be interleaved.
>  		 */
> -		mem_blk->nid = nid;
> -
> -		ret = sysfs_create_link_nowarn(&node_devices[nid]->dev.kobj,
> -					&mem_blk->dev.kobj,
> -					kobject_name(&mem_blk->dev.kobj));
> -		if (ret)
> -			return ret;
> +		page_nid = get_nid_for_pfn(pfn);
> +		if (page_nid < 0)
> +			continue;
> +		if (page_nid != nid)
> +			continue;
>  
> -		return sysfs_create_link_nowarn(&mem_blk->dev.kobj,
> -				&node_devices[nid]->dev.kobj,
> -				kobject_name(&node_devices[nid]->dev.kobj));
> +		return do_register_memory_block_under_node(nid, mem_blk);
>  	}
>  	/* mem section does not span the specified node */
>  	return 0;
>  }
>  
> +/*
> + * During hotplug we know that all pages in the memory block belong to the same
> + * node.
> + */
> +static int register_mem_block_under_node_hotplug(struct memory_block *mem_blk,
> +						 void *arg)
> +{
> +	int nid = *(int *)arg;
> +
> +	return do_register_memory_block_under_node(nid, mem_blk);
> +}
> +
>  /*
>   * Unregister a memory block device under the node it spans. Memory blocks
>   * with multiple nodes cannot be offlined and therefore also never be removed.
> @@ -832,11 +849,19 @@ void unregister_memory_block_under_nodes(struct memory_block *mem_blk)
>  			  kobject_name(&node_devices[mem_blk->nid]->dev.kobj));
>  }
>  
> -int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn)
> +int link_mem_sections(int nid, unsigned long start_pfn, unsigned long end_pfn,
> +		      enum meminit_context context)
>  {
> +	walk_memory_blocks_func_t func;
> +
> +	if (context = MEMINIT_HOTPLUG)
> +		func = register_mem_block_under_node_hotplug;
> +	else
> +		func = register_mem_block_under_node_early;
> +
>  	return walk_memory_blocks(PFN_PHYS(start_pfn),
>  				  PFN_PHYS(end_pfn - start_pfn), (void *)&nid,
> -				  register_mem_sect_under_node);
> +				  func);
>  }
>  
>  #ifdef CONFIG_HUGETLBFS
> diff --git a/include/linux/node.h b/include/linux/node.h
> index 4866f32a02d8..014ba3ab2efd 100644
> --- a/include/linux/node.h
> +++ b/include/linux/node.h
> @@ -99,11 +99,13 @@ extern struct node *node_devices[];
>  typedef  void (*node_registration_func_t)(struct node *);
>  
>  #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_NUMA)
> -extern int link_mem_sections(int nid, unsigned long start_pfn,
> -			     unsigned long end_pfn);
> +int link_mem_sections(int nid, unsigned long start_pfn,
> +		      unsigned long end_pfn,
> +		      enum meminit_context context);
>  #else
>  static inline int link_mem_sections(int nid, unsigned long start_pfn,
> -				    unsigned long end_pfn)
> +				    unsigned long end_pfn,
> +				    enum meminit_context context)
>  {
>  	return 0;
>  }
> @@ -128,7 +130,8 @@ static inline int register_one_node(int nid)
>  		if (error)
>  			return error;
>  		/* link memory sections under this node */
> -		error = link_mem_sections(nid, start_pfn, end_pfn);
> +		error = link_mem_sections(nid, start_pfn, end_pfn,
> +					  MEMINIT_EARLY);
>  	}
>  
>  	return error;
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index fc25886ad719..03df20078827 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1080,7 +1080,8 @@ int __ref add_memory_resource(int nid, struct resource *res)
>  	}
>  
>  	/* link memory sections under this node.*/
> -	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1));
> +	ret = link_mem_sections(nid, PFN_DOWN(start), PFN_UP(start + size - 1),
> +				MEMINIT_HOTPLUG);
>  	BUG_ON(ret);
>  
>  	/* create new memmap entry */
> -- 
> 2.28.0
> 

-- 
Michal Hocko
SUSE Labs

[PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

The memmap_context enum is used to detect whether a memory operation is due
to a hot-add operation or happening at boot time.

Make it general to the hotplug operation and rename it as meminit_context.

There is no functional change introduced by this patch

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
 arch/ia64/mm/init.c    |  6 +++---
 include/linux/mm.h     |  2 +-
 include/linux/mmzone.h | 11 ++++++++---
 mm/memory_hotplug.c    |  2 +-
 mm/page_alloc.c        | 10 +++++-----
 5 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 0b3fb4c7af29..8e7b8c6c576e 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
 	if (map_start < map_end)
 		memmap_init_zone((unsigned long)(map_end - map_start),
 				 args->nid, args->zone, page_to_pfn(map_start),
-				 MEMMAP_EARLY, NULL);
+				 MEMINIT_EARLY, NULL);
 	return 0;
 }
 
@@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
 	     unsigned long start_pfn)
 {
 	if (!vmem_map) {
-		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
-				NULL);
+		memmap_init_zone(size, nid, zone, start_pfn,
+				 MEMINIT_EARLY, NULL);
 	} else {
 		struct page *start;
 		struct memmap_init_callback_data args;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1983e08f5906..e942f91ed155 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
 
 extern void set_dma_reserve(unsigned long new_dma_reserve);
 extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
-		enum memmap_context, struct vmem_altmap *);
+		enum meminit_context, struct vmem_altmap *);
 extern void setup_per_zone_wmarks(void);
 extern int __meminit init_per_zone_wmark_min(void);
 extern void mem_init(void);
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 8379432f4f2f..0f7a4ff4b059 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
 		unsigned int alloc_flags);
 bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
 		unsigned long mark, int highest_zoneidx);
-enum memmap_context {
-	MEMMAP_EARLY,
-	MEMMAP_HOTPLUG,
+/*
+ * Memory initialization context, use to differentiate memory added by
+ * the platform statically or via memory hotplug interface.
+ */
+enum meminit_context {
+	MEMINIT_EARLY,
+	MEMINIT_HOTPLUG,
 };
+
 extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
 				     unsigned long size);
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e9d5ab5d3ca0..fc25886ad719 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 	 * are reserved so nobody should be touching them so we should be safe
 	 */
 	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
-			MEMMAP_HOTPLUG, altmap);
+			 MEMINIT_HOTPLUG, altmap);
 
 	set_zone_contiguous(zone);
 }
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fab5e97dc9ca..5661fa164f13 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
  * done. Non-atomic initialization, single-pass.
  */
 void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
-		unsigned long start_pfn, enum memmap_context context,
+		unsigned long start_pfn, enum meminit_context context,
 		struct vmem_altmap *altmap)
 {
 	unsigned long pfn, end_pfn = start_pfn + size;
@@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 		 * There can be holes in boot-time mem_map[]s handed to this
 		 * function.  They do not exist on hotplugged memory.
 		 */
-		if (context == MEMMAP_EARLY) {
+		if (context == MEMINIT_EARLY) {
 			if (overlap_memmap_init(zone, &pfn))
 				continue;
 			if (defer_init(nid, pfn, end_pfn))
@@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 
 		page = pfn_to_page(pfn);
 		__init_single_page(page, pfn, zone, nid);
-		if (context == MEMMAP_HOTPLUG)
+		if (context == MEMINIT_HOTPLUG)
 			__SetPageReserved(page);
 
 		/*
@@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
 		 * check here not to call set_pageblock_migratetype() against
 		 * pfn out of zone.
 		 *
-		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
+		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
 		 * because this is done early in section_activate()
 		 */
 		if (!(pfn & (pageblock_nr_pages - 1))) {
@@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
 		if (end_pfn > start_pfn) {
 			size = end_pfn - start_pfn;
 			memmap_init_zone(size, nid, zone, start_pfn,
-					 MEMMAP_EARLY, NULL);
+					 MEMINIT_EARLY, NULL);
 		}
 	}
 }
-- 
2.28.0

[PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

The memmap_context enum is used to detect whether a memory operation is due
to a hot-add operation or happening at boot time.

Make it general to the hotplug operation and rename it as meminit_context.

There is no functional change introduced by this patch

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
 arch/ia64/mm/init.c    |  6 +++---
 include/linux/mm.h     |  2 +-
 include/linux/mmzone.h | 11 ++++++++---
 mm/memory_hotplug.c    |  2 +-
 mm/page_alloc.c        | 10 +++++-----
 5 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
index 0b3fb4c7af29..8e7b8c6c576e 100644
--- a/arch/ia64/mm/init.c
+++ b/arch/ia64/mm/init.c
@@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
 	if (map_start < map_end)
 		memmap_init_zone((unsigned long)(map_end - map_start),
 				 args->nid, args->zone, page_to_pfn(map_start),
-				 MEMMAP_EARLY, NULL);
+				 MEMINIT_EARLY, NULL);
 	return 0;
 }
 
@@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
 	     unsigned long start_pfn)
 {
 	if (!vmem_map) {
-		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
-				NULL);
+		memmap_init_zone(size, nid, zone, start_pfn,
+				 MEMINIT_EARLY, NULL);
 	} else {
 		struct page *start;
 		struct memmap_init_callback_data args;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1983e08f5906..e942f91ed155 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
 
 extern void set_dma_reserve(unsigned long new_dma_reserve);
 extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
-		enum memmap_context, struct vmem_altmap *);
+		enum meminit_context, struct vmem_altmap *);
 extern void setup_per_zone_wmarks(void);
 extern int __meminit init_per_zone_wmark_min(void);
 extern void mem_init(void);
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 8379432f4f2f..0f7a4ff4b059 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
 		unsigned int alloc_flags);
 bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
 		unsigned long mark, int highest_zoneidx);
-enum memmap_context {
-	MEMMAP_EARLY,
-	MEMMAP_HOTPLUG,
+/*
+ * Memory initialization context, use to differentiate memory added by
+ * the platform statically or via memory hotplug interface.
+ */
+enum meminit_context {
+	MEMINIT_EARLY,
+	MEMINIT_HOTPLUG,
 };
+
 extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
 				     unsigned long size);
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e9d5ab5d3ca0..fc25886ad719 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 	 * are reserved so nobody should be touching them so we should be safe
 	 */
 	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
-			MEMMAP_HOTPLUG, altmap);
+			 MEMINIT_HOTPLUG, altmap);
 
 	set_zone_contiguous(zone);
 }
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fab5e97dc9ca..5661fa164f13 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
  * done. Non-atomic initialization, single-pass.
  */
 void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
-		unsigned long start_pfn, enum memmap_context context,
+		unsigned long start_pfn, enum meminit_context context,
 		struct vmem_altmap *altmap)
 {
 	unsigned long pfn, end_pfn = start_pfn + size;
@@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 		 * There can be holes in boot-time mem_map[]s handed to this
 		 * function.  They do not exist on hotplugged memory.
 		 */
-		if (context = MEMMAP_EARLY) {
+		if (context = MEMINIT_EARLY) {
 			if (overlap_memmap_init(zone, &pfn))
 				continue;
 			if (defer_init(nid, pfn, end_pfn))
@@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 
 		page = pfn_to_page(pfn);
 		__init_single_page(page, pfn, zone, nid);
-		if (context = MEMMAP_HOTPLUG)
+		if (context = MEMINIT_HOTPLUG)
 			__SetPageReserved(page);
 
 		/*
@@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
 		 * check here not to call set_pageblock_migratetype() against
 		 * pfn out of zone.
 		 *
-		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
+		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
 		 * because this is done early in section_activate()
 		 */
 		if (!(pfn & (pageblock_nr_pages - 1))) {
@@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
 		if (end_pfn > start_pfn) {
 			size = end_pfn - start_pfn;
 			memmap_init_zone(size, nid, zone, start_pfn,
-					 MEMMAP_EARLY, NULL);
+					 MEMINIT_EARLY, NULL);
 		}
 	}
 }
-- 
2.28.0

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Michal Hocko]

On Tue 15-09-20 15:26:24, Laurent Dufour wrote:
> The memmap_context enum is used to detect whether a memory operation is due
> to a hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it as meminit_context.
> 
> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  arch/ia64/mm/init.c    |  6 +++---
>  include/linux/mm.h     |  2 +-
>  include/linux/mmzone.h | 11 ++++++++---
>  mm/memory_hotplug.c    |  2 +-
>  mm/page_alloc.c        | 10 +++++-----
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 0b3fb4c7af29..8e7b8c6c576e 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
>  	if (map_start < map_end)
>  		memmap_init_zone((unsigned long)(map_end - map_start),
>  				 args->nid, args->zone, page_to_pfn(map_start),
> -				 MEMMAP_EARLY, NULL);
> +				 MEMINIT_EARLY, NULL);
>  	return 0;
>  }
>  
> @@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
>  	     unsigned long start_pfn)
>  {
>  	if (!vmem_map) {
> -		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
> -				NULL);
> +		memmap_init_zone(size, nid, zone, start_pfn,
> +				 MEMINIT_EARLY, NULL);
>  	} else {
>  		struct page *start;
>  		struct memmap_init_callback_data args;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1983e08f5906..e942f91ed155 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
>  
>  extern void set_dma_reserve(unsigned long new_dma_reserve);
>  extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
> -		enum memmap_context, struct vmem_altmap *);
> +		enum meminit_context, struct vmem_altmap *);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
>  extern void mem_init(void);
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 8379432f4f2f..0f7a4ff4b059 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
>  		unsigned int alloc_flags);
>  bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
>  		unsigned long mark, int highest_zoneidx);
> -enum memmap_context {
> -	MEMMAP_EARLY,
> -	MEMMAP_HOTPLUG,
> +/*
> + * Memory initialization context, use to differentiate memory added by
> + * the platform statically or via memory hotplug interface.
> + */
> +enum meminit_context {
> +	MEMINIT_EARLY,
> +	MEMINIT_HOTPLUG,
>  };
> +
>  extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
>  				     unsigned long size);
>  
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e9d5ab5d3ca0..fc25886ad719 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  	 * are reserved so nobody should be touching them so we should be safe
>  	 */
>  	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
> -			MEMMAP_HOTPLUG, altmap);
> +			 MEMINIT_HOTPLUG, altmap);
>  
>  	set_zone_contiguous(zone);
>  }
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fab5e97dc9ca..5661fa164f13 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
>   * done. Non-atomic initialization, single-pass.
>   */
>  void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> -		unsigned long start_pfn, enum memmap_context context,
> +		unsigned long start_pfn, enum meminit_context context,
>  		struct vmem_altmap *altmap)
>  {
>  	unsigned long pfn, end_pfn = start_pfn + size;
> @@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  		 * There can be holes in boot-time mem_map[]s handed to this
>  		 * function.  They do not exist on hotplugged memory.
>  		 */
> -		if (context == MEMMAP_EARLY) {
> +		if (context == MEMINIT_EARLY) {
>  			if (overlap_memmap_init(zone, &pfn))
>  				continue;
>  			if (defer_init(nid, pfn, end_pfn))
> @@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  
>  		page = pfn_to_page(pfn);
>  		__init_single_page(page, pfn, zone, nid);
> -		if (context == MEMMAP_HOTPLUG)
> +		if (context == MEMINIT_HOTPLUG)
>  			__SetPageReserved(page);
>  
>  		/*
> @@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  		 * check here not to call set_pageblock_migratetype() against
>  		 * pfn out of zone.
>  		 *
> -		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
> +		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
>  		 * because this is done early in section_activate()
>  		 */
>  		if (!(pfn & (pageblock_nr_pages - 1))) {
> @@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
>  		if (end_pfn > start_pfn) {
>  			size = end_pfn - start_pfn;
>  			memmap_init_zone(size, nid, zone, start_pfn,
> -					 MEMMAP_EARLY, NULL);
> +					 MEMINIT_EARLY, NULL);
>  		}
>  	}
>  }
> -- 
> 2.28.0

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Michal Hocko]

On Tue 15-09-20 15:26:24, Laurent Dufour wrote:
> The memmap_context enum is used to detect whether a memory operation is due
> to a hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it as meminit_context.
> 
> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  arch/ia64/mm/init.c    |  6 +++---
>  include/linux/mm.h     |  2 +-
>  include/linux/mmzone.h | 11 ++++++++---
>  mm/memory_hotplug.c    |  2 +-
>  mm/page_alloc.c        | 10 +++++-----
>  5 files changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index 0b3fb4c7af29..8e7b8c6c576e 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -538,7 +538,7 @@ virtual_memmap_init(u64 start, u64 end, void *arg)
>  	if (map_start < map_end)
>  		memmap_init_zone((unsigned long)(map_end - map_start),
>  				 args->nid, args->zone, page_to_pfn(map_start),
> -				 MEMMAP_EARLY, NULL);
> +				 MEMINIT_EARLY, NULL);
>  	return 0;
>  }
>  
> @@ -547,8 +547,8 @@ memmap_init (unsigned long size, int nid, unsigned long zone,
>  	     unsigned long start_pfn)
>  {
>  	if (!vmem_map) {
> -		memmap_init_zone(size, nid, zone, start_pfn, MEMMAP_EARLY,
> -				NULL);
> +		memmap_init_zone(size, nid, zone, start_pfn,
> +				 MEMINIT_EARLY, NULL);
>  	} else {
>  		struct page *start;
>  		struct memmap_init_callback_data args;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 1983e08f5906..e942f91ed155 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2409,7 +2409,7 @@ extern int __meminit __early_pfn_to_nid(unsigned long pfn,
>  
>  extern void set_dma_reserve(unsigned long new_dma_reserve);
>  extern void memmap_init_zone(unsigned long, int, unsigned long, unsigned long,
> -		enum memmap_context, struct vmem_altmap *);
> +		enum meminit_context, struct vmem_altmap *);
>  extern void setup_per_zone_wmarks(void);
>  extern int __meminit init_per_zone_wmark_min(void);
>  extern void mem_init(void);
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 8379432f4f2f..0f7a4ff4b059 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -824,10 +824,15 @@ bool zone_watermark_ok(struct zone *z, unsigned int order,
>  		unsigned int alloc_flags);
>  bool zone_watermark_ok_safe(struct zone *z, unsigned int order,
>  		unsigned long mark, int highest_zoneidx);
> -enum memmap_context {
> -	MEMMAP_EARLY,
> -	MEMMAP_HOTPLUG,
> +/*
> + * Memory initialization context, use to differentiate memory added by
> + * the platform statically or via memory hotplug interface.
> + */
> +enum meminit_context {
> +	MEMINIT_EARLY,
> +	MEMINIT_HOTPLUG,
>  };
> +
>  extern void init_currently_empty_zone(struct zone *zone, unsigned long start_pfn,
>  				     unsigned long size);
>  
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index e9d5ab5d3ca0..fc25886ad719 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -729,7 +729,7 @@ void __ref move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
>  	 * are reserved so nobody should be touching them so we should be safe
>  	 */
>  	memmap_init_zone(nr_pages, nid, zone_idx(zone), start_pfn,
> -			MEMMAP_HOTPLUG, altmap);
> +			 MEMINIT_HOTPLUG, altmap);
>  
>  	set_zone_contiguous(zone);
>  }
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index fab5e97dc9ca..5661fa164f13 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -5975,7 +5975,7 @@ overlap_memmap_init(unsigned long zone, unsigned long *pfn)
>   * done. Non-atomic initialization, single-pass.
>   */
>  void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
> -		unsigned long start_pfn, enum memmap_context context,
> +		unsigned long start_pfn, enum meminit_context context,
>  		struct vmem_altmap *altmap)
>  {
>  	unsigned long pfn, end_pfn = start_pfn + size;
> @@ -6007,7 +6007,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  		 * There can be holes in boot-time mem_map[]s handed to this
>  		 * function.  They do not exist on hotplugged memory.
>  		 */
> -		if (context = MEMMAP_EARLY) {
> +		if (context = MEMINIT_EARLY) {
>  			if (overlap_memmap_init(zone, &pfn))
>  				continue;
>  			if (defer_init(nid, pfn, end_pfn))
> @@ -6016,7 +6016,7 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
>  
>  		page = pfn_to_page(pfn);
>  		__init_single_page(page, pfn, zone, nid);
> -		if (context = MEMMAP_HOTPLUG)
> +		if (context = MEMINIT_HOTPLUG)
>  			__SetPageReserved(page);
>  
>  		/*
> @@ -6099,7 +6099,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
>  		 * check here not to call set_pageblock_migratetype() against
>  		 * pfn out of zone.
>  		 *
> -		 * Please note that MEMMAP_HOTPLUG path doesn't clear memmap
> +		 * Please note that MEMINIT_HOTPLUG path doesn't clear memmap
>  		 * because this is done early in section_activate()
>  		 */
>  		if (!(pfn & (pageblock_nr_pages - 1))) {
> @@ -6137,7 +6137,7 @@ void __meminit __weak memmap_init(unsigned long size, int nid,
>  		if (end_pfn > start_pfn) {
>  			size = end_pfn - start_pfn;
>  			memmap_init_zone(size, nid, zone, start_pfn,
> -					 MEMMAP_EARLY, NULL);
> +					 MEMINIT_EARLY, NULL);
>  		}
>  	}
>  }
> -- 
> 2.28.0

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Greg Kroah-Hartman]

On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
> The memmap_context enum is used to detect whether a memory operation is due
> to a hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it as meminit_context.
> 
> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>  arch/ia64/mm/init.c    |  6 +++---
>  include/linux/mm.h     |  2 +-
>  include/linux/mmzone.h | 11 ++++++++---
>  mm/memory_hotplug.c    |  2 +-
>  mm/page_alloc.c        | 10 +++++-----
>  5 files changed, 18 insertions(+), 13 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Greg Kroah-Hartman]

On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
> The memmap_context enum is used to detect whether a memory operation is due
> to a hot-add operation or happening at boot time.
> 
> Make it general to the hotplug operation and rename it as meminit_context.
> 
> There is no functional change introduced by this patch
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
>  arch/ia64/mm/init.c    |  6 +++---
>  include/linux/mm.h     |  2 +-
>  include/linux/mmzone.h | 11 ++++++++---
>  mm/memory_hotplug.c    |  2 +-
>  mm/page_alloc.c        | 10 +++++-----
>  5 files changed, 18 insertions(+), 13 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
> On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
>> The memmap_context enum is used to detect whether a memory operation is due
>> to a hot-add operation or happening at boot time.
>>
>> Make it general to the hotplug operation and rename it as meminit_context.
>>
>> There is no functional change introduced by this patch
>>
>> Suggested-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>> ---
>>   arch/ia64/mm/init.c    |  6 +++---
>>   include/linux/mm.h     |  2 +-
>>   include/linux/mmzone.h | 11 ++++++++---
>>   mm/memory_hotplug.c    |  2 +-
>>   mm/page_alloc.c        | 10 +++++-----
>>   5 files changed, 18 insertions(+), 13 deletions(-)
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

Hi Greg,

I'm sorry, I read that document few days ago before sending the series and again 
this morning, but I can't figure out what I missed (following option 1).

Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series even 
if the whole series has been sent to stable ?

Should the whole series sent again (v4) instead of sending a fix as a reply to ?

Thanks,
Laurent.

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
> On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
>> The memmap_context enum is used to detect whether a memory operation is due
>> to a hot-add operation or happening at boot time.
>>
>> Make it general to the hotplug operation and rename it as meminit_context.
>>
>> There is no functional change introduced by this patch
>>
>> Suggested-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>> ---
>>   arch/ia64/mm/init.c    |  6 +++---
>>   include/linux/mm.h     |  2 +-
>>   include/linux/mmzone.h | 11 ++++++++---
>>   mm/memory_hotplug.c    |  2 +-
>>   mm/page_alloc.c        | 10 +++++-----
>>   5 files changed, 18 insertions(+), 13 deletions(-)
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

Hi Greg,

I'm sorry, I read that document few days ago before sending the series and again 
this morning, but I can't figure out what I missed (following option 1).

Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series even 
if the whole series has been sent to stable ?

Should the whole series sent again (v4) instead of sending a fix as a reply to ?

Thanks,
Laurent.

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Greg Kroah-Hartman]

On Wed, Sep 16, 2020 at 09:29:22AM +0200, Laurent Dufour wrote:
> Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
> > On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
> > > The memmap_context enum is used to detect whether a memory operation is due
> > > to a hot-add operation or happening at boot time.
> > > 
> > > Make it general to the hotplug operation and rename it as meminit_context.
> > > 
> > > There is no functional change introduced by this patch
> > > 
> > > Suggested-by: David Hildenbrand <david@redhat.com>
> > > Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> > > ---
> > >   arch/ia64/mm/init.c    |  6 +++---
> > >   include/linux/mm.h     |  2 +-
> > >   include/linux/mmzone.h | 11 ++++++++---
> > >   mm/memory_hotplug.c    |  2 +-
> > >   mm/page_alloc.c        | 10 +++++-----
> > >   5 files changed, 18 insertions(+), 13 deletions(-)
> > 
> > <formletter>
> > 
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read:
> >      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to do this properly.
> > 
> > </formletter>
> 
> Hi Greg,
> 
> I'm sorry, I read that document few days ago before sending the series and
> again this morning, but I can't figure out what I missed (following option
> 1).
> 
> Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series
> even if the whole series has been sent to stable ?

That should be on any patch you expect to show up in a stable kernel
release.

> Should the whole series sent again (v4) instead of sending a fix as a reply to ?

It's up to the maintainer what they want, but as it is, this patch is
not going to end up in stable kernel release (which it looks like is the
right thing to do...)

thanks,

greg k-h

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Greg Kroah-Hartman]

On Wed, Sep 16, 2020 at 09:29:22AM +0200, Laurent Dufour wrote:
> Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
> > On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
> > > The memmap_context enum is used to detect whether a memory operation is due
> > > to a hot-add operation or happening at boot time.
> > > 
> > > Make it general to the hotplug operation and rename it as meminit_context.
> > > 
> > > There is no functional change introduced by this patch
> > > 
> > > Suggested-by: David Hildenbrand <david@redhat.com>
> > > Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> > > ---
> > >   arch/ia64/mm/init.c    |  6 +++---
> > >   include/linux/mm.h     |  2 +-
> > >   include/linux/mmzone.h | 11 ++++++++---
> > >   mm/memory_hotplug.c    |  2 +-
> > >   mm/page_alloc.c        | 10 +++++-----
> > >   5 files changed, 18 insertions(+), 13 deletions(-)
> > 
> > <formletter>
> > 
> > This is not the correct way to submit patches for inclusion in the
> > stable kernel tree.  Please read:
> >      https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> > for how to do this properly.
> > 
> > </formletter>
> 
> Hi Greg,
> 
> I'm sorry, I read that document few days ago before sending the series and
> again this morning, but I can't figure out what I missed (following option
> 1).
> 
> Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series
> even if the whole series has been sent to stable ?

That should be on any patch you expect to show up in a stable kernel
release.

> Should the whole series sent again (v4) instead of sending a fix as a reply to ?

It's up to the maintainer what they want, but as it is, this patch is
not going to end up in stable kernel release (which it looks like is the
right thing to do...)

thanks,

greg k-h

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

Le 16/09/2020 à 09:40, Greg Kroah-Hartman a écrit :
> On Wed, Sep 16, 2020 at 09:29:22AM +0200, Laurent Dufour wrote:
>> Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
>>> On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
>>>> The memmap_context enum is used to detect whether a memory operation is due
>>>> to a hot-add operation or happening at boot time.
>>>>
>>>> Make it general to the hotplug operation and rename it as meminit_context.
>>>>
>>>> There is no functional change introduced by this patch
>>>>
>>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>>>> ---
>>>>    arch/ia64/mm/init.c    |  6 +++---
>>>>    include/linux/mm.h     |  2 +-
>>>>    include/linux/mmzone.h | 11 ++++++++---
>>>>    mm/memory_hotplug.c    |  2 +-
>>>>    mm/page_alloc.c        | 10 +++++-----
>>>>    5 files changed, 18 insertions(+), 13 deletions(-)
>>>
>>> <formletter>
>>>
>>> This is not the correct way to submit patches for inclusion in the
>>> stable kernel tree.  Please read:
>>>       https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>> for how to do this properly.
>>>
>>> </formletter>
>>
>> Hi Greg,
>>
>> I'm sorry, I read that document few days ago before sending the series and
>> again this morning, but I can't figure out what I missed (following option
>> 1).
>>
>> Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series
>> even if the whole series has been sent to stable ?
> 
> That should be on any patch you expect to show up in a stable kernel
> release.
> 
>> Should the whole series sent again (v4) instead of sending a fix as a reply to ?
> 
> It's up to the maintainer what they want, but as it is, this patch is
> not going to end up in stable kernel release (which it looks like is the
> right thing to do...)

Thanks a lot Greg.

I'll send that single patch again with the Cc: stable tag.

I don't think the patch 3 need to be backported, it doesn't fix any issue and 
with the patch 1 and 2 applied, the BUG_ON should no more be triggered easily.

Laurent.

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

Le 16/09/2020 à 09:40, Greg Kroah-Hartman a écrit :
> On Wed, Sep 16, 2020 at 09:29:22AM +0200, Laurent Dufour wrote:
>> Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
>>> On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
>>>> The memmap_context enum is used to detect whether a memory operation is due
>>>> to a hot-add operation or happening at boot time.
>>>>
>>>> Make it general to the hotplug operation and rename it as meminit_context.
>>>>
>>>> There is no functional change introduced by this patch
>>>>
>>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>>>> ---
>>>>    arch/ia64/mm/init.c    |  6 +++---
>>>>    include/linux/mm.h     |  2 +-
>>>>    include/linux/mmzone.h | 11 ++++++++---
>>>>    mm/memory_hotplug.c    |  2 +-
>>>>    mm/page_alloc.c        | 10 +++++-----
>>>>    5 files changed, 18 insertions(+), 13 deletions(-)
>>>
>>> <formletter>
>>>
>>> This is not the correct way to submit patches for inclusion in the
>>> stable kernel tree.  Please read:
>>>       https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>> for how to do this properly.
>>>
>>> </formletter>
>>
>> Hi Greg,
>>
>> I'm sorry, I read that document few days ago before sending the series and
>> again this morning, but I can't figure out what I missed (following option
>> 1).
>>
>> Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series
>> even if the whole series has been sent to stable ?
> 
> That should be on any patch you expect to show up in a stable kernel
> release.
> 
>> Should the whole series sent again (v4) instead of sending a fix as a reply to ?
> 
> It's up to the maintainer what they want, but as it is, this patch is
> not going to end up in stable kernel release (which it looks like is the
> right thing to do...)

Thanks a lot Greg.

I'll send that single patch again with the Cc: stable tag.

I don't think the patch 3 need to be backported, it doesn't fix any issue and 
with the patch 1 and 2 applied, the BUG_ON should no more be triggered easily.

Laurent.

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[David Hildenbrand]

On 16.09.20 09:47, Laurent Dufour wrote:
> Le 16/09/2020 à 09:40, Greg Kroah-Hartman a écrit :
>> On Wed, Sep 16, 2020 at 09:29:22AM +0200, Laurent Dufour wrote:
>>> Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
>>>> On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
>>>>> The memmap_context enum is used to detect whether a memory operation is due
>>>>> to a hot-add operation or happening at boot time.
>>>>>
>>>>> Make it general to the hotplug operation and rename it as meminit_context.
>>>>>
>>>>> There is no functional change introduced by this patch
>>>>>
>>>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>>>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>>>>> ---
>>>>>    arch/ia64/mm/init.c    |  6 +++---
>>>>>    include/linux/mm.h     |  2 +-
>>>>>    include/linux/mmzone.h | 11 ++++++++---
>>>>>    mm/memory_hotplug.c    |  2 +-
>>>>>    mm/page_alloc.c        | 10 +++++-----
>>>>>    5 files changed, 18 insertions(+), 13 deletions(-)
>>>>
>>>> <formletter>
>>>>
>>>> This is not the correct way to submit patches for inclusion in the
>>>> stable kernel tree.  Please read:
>>>>       https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>>> for how to do this properly.
>>>>
>>>> </formletter>
>>>
>>> Hi Greg,
>>>
>>> I'm sorry, I read that document few days ago before sending the series and
>>> again this morning, but I can't figure out what I missed (following option
>>> 1).
>>>
>>> Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series
>>> even if the whole series has been sent to stable ?
>>
>> That should be on any patch you expect to show up in a stable kernel
>> release.
>>
>>> Should the whole series sent again (v4) instead of sending a fix as a reply to ?
>>
>> It's up to the maintainer what they want, but as it is, this patch is
>> not going to end up in stable kernel release (which it looks like is the
>> right thing to do...)
> 
> Thanks a lot Greg.
> 
> I'll send that single patch again with the Cc: stable tag.

I think Andrew can add that when sending upstream.

While a single patch to fix + backport would be nicer, I don't see an
easy (!ugly) way to achieve the same without this cleanup.

1. We could rework patch #2 to pass a simple boolean flag, and a
follow-on patch to pass the context. Not sure if that's any better.

2. We could rework patch #2 to pass memmap_context first, and modify
patch #1 to also rename this instance.

Maybe 2. might be reasonable (not sure if worth the trouble). @Greg any
preference?

> 
> I don't think the patch 3 need to be backported, it doesn't fix any issue and 
> with the patch 1 and 2 applied, the BUG_ON should no more be triggered easily.

Agreed.


-- 
Thanks,

David / dhildenb

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[David Hildenbrand]

On 16.09.20 09:47, Laurent Dufour wrote:
> Le 16/09/2020 à 09:40, Greg Kroah-Hartman a écrit :
>> On Wed, Sep 16, 2020 at 09:29:22AM +0200, Laurent Dufour wrote:
>>> Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
>>>> On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
>>>>> The memmap_context enum is used to detect whether a memory operation is due
>>>>> to a hot-add operation or happening at boot time.
>>>>>
>>>>> Make it general to the hotplug operation and rename it as meminit_context.
>>>>>
>>>>> There is no functional change introduced by this patch
>>>>>
>>>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>>>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>>>>> ---
>>>>>    arch/ia64/mm/init.c    |  6 +++---
>>>>>    include/linux/mm.h     |  2 +-
>>>>>    include/linux/mmzone.h | 11 ++++++++---
>>>>>    mm/memory_hotplug.c    |  2 +-
>>>>>    mm/page_alloc.c        | 10 +++++-----
>>>>>    5 files changed, 18 insertions(+), 13 deletions(-)
>>>>
>>>> <formletter>
>>>>
>>>> This is not the correct way to submit patches for inclusion in the
>>>> stable kernel tree.  Please read:
>>>>       https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>>> for how to do this properly.
>>>>
>>>> </formletter>
>>>
>>> Hi Greg,
>>>
>>> I'm sorry, I read that document few days ago before sending the series and
>>> again this morning, but I can't figure out what I missed (following option
>>> 1).
>>>
>>> Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series
>>> even if the whole series has been sent to stable ?
>>
>> That should be on any patch you expect to show up in a stable kernel
>> release.
>>
>>> Should the whole series sent again (v4) instead of sending a fix as a reply to ?
>>
>> It's up to the maintainer what they want, but as it is, this patch is
>> not going to end up in stable kernel release (which it looks like is the
>> right thing to do...)
> 
> Thanks a lot Greg.
> 
> I'll send that single patch again with the Cc: stable tag.

I think Andrew can add that when sending upstream.

While a single patch to fix + backport would be nicer, I don't see an
easy (!ugly) way to achieve the same without this cleanup.

1. We could rework patch #2 to pass a simple boolean flag, and a
follow-on patch to pass the context. Not sure if that's any better.

2. We could rework patch #2 to pass memmap_context first, and modify
patch #1 to also rename this instance.

Maybe 2. might be reasonable (not sure if worth the trouble). @Greg any
preference?

> 
> I don't think the patch 3 need to be backported, it doesn't fix any issue and 
> with the patch 1 and 2 applied, the BUG_ON should no more be triggered easily.

Agreed.


-- 
Thanks,

David / dhildenb

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

Le 16/09/2020 à 09:52, David Hildenbrand a écrit :
> On 16.09.20 09:47, Laurent Dufour wrote:
>> Le 16/09/2020 à 09:40, Greg Kroah-Hartman a écrit :
>>> On Wed, Sep 16, 2020 at 09:29:22AM +0200, Laurent Dufour wrote:
>>>> Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
>>>>> On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
>>>>>> The memmap_context enum is used to detect whether a memory operation is due
>>>>>> to a hot-add operation or happening at boot time.
>>>>>>
>>>>>> Make it general to the hotplug operation and rename it as meminit_context.
>>>>>>
>>>>>> There is no functional change introduced by this patch
>>>>>>
>>>>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>>>>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>>>>>> ---
>>>>>>     arch/ia64/mm/init.c    |  6 +++---
>>>>>>     include/linux/mm.h     |  2 +-
>>>>>>     include/linux/mmzone.h | 11 ++++++++---
>>>>>>     mm/memory_hotplug.c    |  2 +-
>>>>>>     mm/page_alloc.c        | 10 +++++-----
>>>>>>     5 files changed, 18 insertions(+), 13 deletions(-)
>>>>>
>>>>> <formletter>
>>>>>
>>>>> This is not the correct way to submit patches for inclusion in the
>>>>> stable kernel tree.  Please read:
>>>>>        https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>>>> for how to do this properly.
>>>>>
>>>>> </formletter>
>>>>
>>>> Hi Greg,
>>>>
>>>> I'm sorry, I read that document few days ago before sending the series and
>>>> again this morning, but I can't figure out what I missed (following option
>>>> 1).
>>>>
>>>> Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series
>>>> even if the whole series has been sent to stable ?
>>>
>>> That should be on any patch you expect to show up in a stable kernel
>>> release.
>>>
>>>> Should the whole series sent again (v4) instead of sending a fix as a reply to ?
>>>
>>> It's up to the maintainer what they want, but as it is, this patch is
>>> not going to end up in stable kernel release (which it looks like is the
>>> right thing to do...)
>>
>> Thanks a lot Greg.
>>
>> I'll send that single patch again with the Cc: stable tag.
> 
> I think Andrew can add that when sending upstream.

Andrew, can you do that?

> While a single patch to fix + backport would be nicer, I don't see an
> easy (!ugly) way to achieve the same without this cleanup.
> 
> 1. We could rework patch #2 to pass a simple boolean flag, and a
> follow-on patch to pass the context. Not sure if that's any better.
> 
> 2. We could rework patch #2 to pass memmap_context first, and modify
> patch #1 to also rename this instance.
> 
> Maybe 2. might be reasonable (not sure if worth the trouble). @Greg any
> preference?
> 
>>
>> I don't think the patch 3 need to be backported, it doesn't fix any issue and
>> with the patch 1 and 2 applied, the BUG_ON should no more be triggered easily.
> 
> Agreed.
> 
> 

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

Le 16/09/2020 à 09:52, David Hildenbrand a écrit :
> On 16.09.20 09:47, Laurent Dufour wrote:
>> Le 16/09/2020 à 09:40, Greg Kroah-Hartman a écrit :
>>> On Wed, Sep 16, 2020 at 09:29:22AM +0200, Laurent Dufour wrote:
>>>> Le 16/09/2020 à 08:33, Greg Kroah-Hartman a écrit :
>>>>> On Tue, Sep 15, 2020 at 03:26:24PM +0200, Laurent Dufour wrote:
>>>>>> The memmap_context enum is used to detect whether a memory operation is due
>>>>>> to a hot-add operation or happening at boot time.
>>>>>>
>>>>>> Make it general to the hotplug operation and rename it as meminit_context.
>>>>>>
>>>>>> There is no functional change introduced by this patch
>>>>>>
>>>>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>>>>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>>>>>> ---
>>>>>>     arch/ia64/mm/init.c    |  6 +++---
>>>>>>     include/linux/mm.h     |  2 +-
>>>>>>     include/linux/mmzone.h | 11 ++++++++---
>>>>>>     mm/memory_hotplug.c    |  2 +-
>>>>>>     mm/page_alloc.c        | 10 +++++-----
>>>>>>     5 files changed, 18 insertions(+), 13 deletions(-)
>>>>>
>>>>> <formletter>
>>>>>
>>>>> This is not the correct way to submit patches for inclusion in the
>>>>> stable kernel tree.  Please read:
>>>>>        https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>>>>> for how to do this properly.
>>>>>
>>>>> </formletter>
>>>>
>>>> Hi Greg,
>>>>
>>>> I'm sorry, I read that document few days ago before sending the series and
>>>> again this morning, but I can't figure out what I missed (following option
>>>> 1).
>>>>
>>>> Should the "Cc: stable@vger.kernel.org" tag be on each patch of the series
>>>> even if the whole series has been sent to stable ?
>>>
>>> That should be on any patch you expect to show up in a stable kernel
>>> release.
>>>
>>>> Should the whole series sent again (v4) instead of sending a fix as a reply to ?
>>>
>>> It's up to the maintainer what they want, but as it is, this patch is
>>> not going to end up in stable kernel release (which it looks like is the
>>> right thing to do...)
>>
>> Thanks a lot Greg.
>>
>> I'll send that single patch again with the Cc: stable tag.
> 
> I think Andrew can add that when sending upstream.

Andrew, can you do that?

> While a single patch to fix + backport would be nicer, I don't see an
> easy (!ugly) way to achieve the same without this cleanup.
> 
> 1. We could rework patch #2 to pass a simple boolean flag, and a
> follow-on patch to pass the context. Not sure if that's any better.
> 
> 2. We could rework patch #2 to pass memmap_context first, and modify
> patch #1 to also rename this instance.
> 
> Maybe 2. might be reasonable (not sure if worth the trouble). @Greg any
> preference?
> 
>>
>> I don't think the patch 3 need to be backported, it doesn't fix any issue and
>> with the patch 1 and 2 applied, the BUG_ON should no more be triggered easily.
> 
> Agreed.
> 
> 

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Andrew Morton]

On Wed, 16 Sep 2020 18:09:55 +0200 Laurent Dufour <ldufour@linux.ibm.com> wrote:

> >>> It's up to the maintainer what they want, but as it is, this patch is
> >>> not going to end up in stable kernel release (which it looks like is the
> >>> right thing to do...)
> >>
> >> Thanks a lot Greg.
> >>
> >> I'll send that single patch again with the Cc: stable tag.
> > 
> > I think Andrew can add that when sending upstream.
> 
> Andrew, can you do that?
> 

I did.

Patches 1 & 2 are cc:stable, patch 3 is not.

I'll queue up 1 & 2 for a 5.9-rcX merge.

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Andrew Morton]

On Wed, 16 Sep 2020 18:09:55 +0200 Laurent Dufour <ldufour@linux.ibm.com> wrote:

> >>> It's up to the maintainer what they want, but as it is, this patch is
> >>> not going to end up in stable kernel release (which it looks like is the
> >>> right thing to do...)
> >>
> >> Thanks a lot Greg.
> >>
> >> I'll send that single patch again with the Cc: stable tag.
> > 
> > I think Andrew can add that when sending upstream.
> 
> Andrew, can you do that?
> 

I did.

Patches 1 & 2 are cc:stable, patch 3 is not.

I'll queue up 1 & 2 for a 5.9-rcX merge.

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

Le 17/09/2020 à 01:37, Andrew Morton a écrit :
> On Wed, 16 Sep 2020 18:09:55 +0200 Laurent Dufour <ldufour@linux.ibm.com> wrote:
> 
>>>>> It's up to the maintainer what they want, but as it is, this patch is
>>>>> not going to end up in stable kernel release (which it looks like is the
>>>>> right thing to do...)
>>>>
>>>> Thanks a lot Greg.
>>>>
>>>> I'll send that single patch again with the Cc: stable tag.
>>>
>>> I think Andrew can add that when sending upstream.
>>
>> Andrew, can you do that?
>>
> 
> I did.
> 
> Patches 1 & 2 are cc:stable, patch 3 is not.
> 
> I'll queue up 1 & 2 for a 5.9-rcX merge.

Thanks a lot Andrew.

Re: [PATCH v3 1/3] mm: replace memmap_context by meminit_context

[Laurent Dufour]

Le 17/09/2020 à 01:37, Andrew Morton a écrit :
> On Wed, 16 Sep 2020 18:09:55 +0200 Laurent Dufour <ldufour@linux.ibm.com> wrote:
> 
>>>>> It's up to the maintainer what they want, but as it is, this patch is
>>>>> not going to end up in stable kernel release (which it looks like is the
>>>>> right thing to do...)
>>>>
>>>> Thanks a lot Greg.
>>>>
>>>> I'll send that single patch again with the Cc: stable tag.
>>>
>>> I think Andrew can add that when sending upstream.
>>
>> Andrew, can you do that?
>>
> 
> I did.
> 
> Patches 1 & 2 are cc:stable, patch 3 is not.
> 
> I'll queue up 1 & 2 for a 5.9-rcX merge.

Thanks a lot Andrew.