Skip to contentSkip to conversation

mm/mm_init: don't overlap zones with kernelcore=mirror

PATCH Superseded Mike Rapoport 3 participants 10 messages Jun 25 – Jun 29

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Mike Rapoport

author maintainer
#0 lore raw

cover mm/mm_init: don't overlap zones with kernelcore=mirror

From: "Mike Rapoport (Microsoft)" <[email protected]>

Hi,

These patches make the behaviour of kernelcore= parameter uniform and treat mirror just as another way to size the zones and cleanup a weird part of the memory map initialization.

Mike Rapoport (Microsoft) (2):
mm/mm_init: don't overlap NORMAL and MOVABLE zones with kernelcore=mirror mm/mm_init: drop overlap_memmap_init()

 mm/mm_init.c | 60 +++-------------------------------------------------
 1 file changed, 3 insertions(+), 57 deletions(-)

base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48

Signature · 1 line
2.53.0
author maintainer
#1 lore raw

PATCH 1/2 mm/mm_init: don't overlap NORMAL and MOVABLE zones with kernelcore=mirror

From: "Mike Rapoport (Microsoft)" <[email protected]>

When kernelcore or movablecore kernel parameters define size of the NORMAL and MOVABLE zones as percents of the total memory or by absolute value, ZONE_NORMAL is clamped at the beginning of ZONE_MOVABLE.

However, when kernelcore=mirror the ZONE_NORMAL span is not changed but rather pages from ZONE_MOVABLE counted as absent in ZONE_NORMAL.

Make the behaviour of kernelcore= parameter uniform and treat mirror just as another way to size the zones.

---
 mm/mm_init.c | 36 +++---------------------------------
 1 file changed, 3 insertions(+), 33 deletions(-)
mm/mm_init.c +3 -33
index f9f8e1af921c..dce9dc9f2302 100644
@@ -1159,9 +1159,8 @@ static void __init adjust_zone_range_for_zone_movable(int nid,
 				arch_zone_highest_possible_pfn[movable_zone]);
 
 		/* Adjust for ZONE_MOVABLE starting within this range */
-		} else if (!mirrored_kernelcore &&
-			*zone_start_pfn < zone_movable_pfn[nid] &&
-			*zone_end_pfn > zone_movable_pfn[nid]) {
+		} else if (*zone_start_pfn < zone_movable_pfn[nid] &&
+			   *zone_end_pfn > zone_movable_pfn[nid]) {
 			*zone_end_pfn = zone_movable_pfn[nid];
 
 		/* Check if this whole range is within ZONE_MOVABLE */
@@ -1209,40 +1208,11 @@ static unsigned long __init zone_absent_pages_in_node(int nid,
 					unsigned long zone_start_pfn,
 					unsigned long zone_end_pfn)
 {
-	unsigned long nr_absent;
-
 	/* zone is empty, we don't have any absent pages */
 	if (zone_start_pfn == zone_end_pfn)
 		return 0;
 
-	nr_absent = __absent_pages_in_range(nid, zone_start_pfn, zone_end_pfn);
-
-	/*
-	 * ZONE_MOVABLE handling.
-	 * Treat pages to be ZONE_MOVABLE in ZONE_NORMAL as absent pages
-	 * and vice versa.
-	 */
-	if (mirrored_kernelcore && zone_movable_pfn[nid]) {
-		unsigned long start_pfn, end_pfn;
-		struct memblock_region *r;
-
-		for_each_mem_region(r) {
-			start_pfn = clamp(memblock_region_memory_base_pfn(r),
-					  zone_start_pfn, zone_end_pfn);
-			end_pfn = clamp(memblock_region_memory_end_pfn(r),
-					zone_start_pfn, zone_end_pfn);
-
-			if (zone_type == ZONE_MOVABLE &&
-			    memblock_is_mirror(r))
-				nr_absent += end_pfn - start_pfn;
-
-			if (zone_type == ZONE_NORMAL &&
-			    !memblock_is_mirror(r))
-				nr_absent += end_pfn - start_pfn;
-		}
-	}
-
-	return nr_absent;
+	return __absent_pages_in_range(nid, zone_start_pfn, zone_end_pfn);
 }
 
 /*
Signature · 1 line
2.53.0
Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] mm/mm_init: don't overlap NORMAL and MOVABLE zones with kernelcore=mirror

Quoted 11 lines · Mike Rapoport

From: "Mike Rapoport (Microsoft)" <[email protected]>

When kernelcore or movablecore kernel parameters define size of the NORMAL and MOVABLE zones as percents of the total memory or by absolute value, ZONE_NORMAL is clamped at the beginning of ZONE_MOVABLE.

However, when kernelcore=mirror the ZONE_NORMAL span is not changed but rather pages from ZONE_MOVABLE counted as absent in ZONE_NORMAL.

Make the behaviour of kernelcore= parameter uniform and treat mirror just as another way to size the zones.

That sounds just about right to me.

Hard to imagine that any code would have relied on the old behavior.

Signature · 3 lines
Cheers,

David
author maintainer
#3 lore raw

PATCH 2/2 mm/mm_init: drop overlap_memmap_init()

In reply to Mike Rapoport

From: "Mike Rapoport (Microsoft)" <[email protected]>

When ZONE_NORMAL and ZONE_MOVABLE could overlap because kernelcore=mirror didn't reduce the span of ZONE_NORMAL, initialization of the memory map had to skip overlapping pages during initialization of ZONE_MOVABLE to avoid double initialization of the same struct pages.

Since kernelcore=mirror works now the same way as other variants of kernelcore=/movablecore=, and adjusts the span of ZONE_NORMAL, there can't be an overlap between ZONE_NORMAL and ZONE_MOVABLE.

Remove overlap_memmap_init().

---
 mm/mm_init.c | 24 ------------------------
 1 file changed, 24 deletions(-)
mm/mm_init.c +0 -24
index dce9dc9f2302..6f0a71ccca30 100644
@@ -783,28 +783,6 @@ void __meminit init_deferred_page(unsigned long pfn, int nid)
 	__init_deferred_page(pfn, nid);
 }
 
-/* If zone is ZONE_MOVABLE but memory is mirrored, it is an overlapped init */
-static bool __meminit
-overlap_memmap_init(unsigned long zone, unsigned long *pfn)
-{
-	static struct memblock_region *r __meminitdata;
-
-	if (mirrored_kernelcore && zone == ZONE_MOVABLE) {
-		if (!r || *pfn >= memblock_region_memory_end_pfn(r)) {
-			for_each_mem_region(r) {
-				if (*pfn < memblock_region_memory_end_pfn(r))
-					break;
-			}
-		}
-		if (*pfn >= memblock_region_memory_base_pfn(r) &&
-		    memblock_is_mirror(r)) {
-			*pfn = memblock_region_memory_end_pfn(r);
-			return true;
-		}
-	}
-	return false;
-}
-
 /*
  * Only struct pages that correspond to ranges defined by memblock.memory
  * are zeroed and initialized by going through __init_single_page() during
@@ -891,8 +869,6 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
 		 * function.  They do not exist on hotplugged memory.
 		 */
 		if (context == MEMINIT_EARLY) {
-			if (overlap_memmap_init(zone, &pfn))
-				continue;
 			if (defer_init(nid, pfn, zone_end_pfn)) {
 				deferred_struct_pages = true;
 				break;
Signature · 1 line
2.53.0
#4 lore raw

Re: [PATCH 2/2] mm/mm_init: drop overlap_memmap_init()

Quoted 7 lines · Mike Rapoport

From: "Mike Rapoport (Microsoft)" <[email protected]>

When ZONE_NORMAL and ZONE_MOVABLE could overlap because kernelcore=mirror didn't reduce the span of ZONE_NORMAL, initialization of the memory map had to skip overlapping pages during initialization of ZONE_MOVABLE to avoid double initialization of the same struct pages.

Quoted 6 lines

Since kernelcore=mirror works now the same way as other variants of kernelcore=/movablecore=, and adjusts the span of ZONE_NORMAL, there can't be an overlap between ZONE_NORMAL and ZONE_MOVABLE.

Remove overlap_memmap_init().

Hi, Mike

Do you think this is valuable to mention the double init for the overlapped range?

Quoted 51 lines

---
mm/mm_init.c | 24 ------------------------

1 file changed, 24 deletions(-)
mm/mm_init.c +0 -24
index dce9dc9f2302..6f0a71ccca30 100644
@@ -783,28 +783,6 @@ void __meminit init_deferred_page(unsigned long pfn, int nid)
 	__init_deferred_page(pfn, nid);
 }
 
-/* If zone is ZONE_MOVABLE but memory is mirrored, it is an overlapped init */
-static bool __meminit
-overlap_memmap_init(unsigned long zone, unsigned long *pfn)
-{
-	static struct memblock_region *r __meminitdata;
-
-	if (mirrored_kernelcore && zone == ZONE_MOVABLE) {
-		if (!r || *pfn >= memblock_region_memory_end_pfn(r)) {
-			for_each_mem_region(r) {
-				if (*pfn < memblock_region_memory_end_pfn(r))
-					break;
-			}
-		}
-		if (*pfn >= memblock_region_memory_base_pfn(r) &&
-		    memblock_is_mirror(r)) {
-			*pfn = memblock_region_memory_end_pfn(r);
-			return true;
-		}
-	}
-	return false;
-}
-
 /*
 * Only struct pages that correspond to ranges defined by memblock.memory
 * are zeroed and initialized by going through __init_single_page() during
@@ -891,8 +869,6 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
 		 * function.  They do not exist on hotplugged memory.
 		 */
 		if (context == MEMINIT_EARLY) {
-			if (overlap_memmap_init(zone, &pfn))
-				continue;
 			if (defer_init(nid, pfn, zone_end_pfn)) {
 				deferred_struct_pages = true;
 				break;
Signature · 1 line
2.53.0
Signature · 2 lines
Wei Yang
Help you, Help me
author maintainer
#5 lore raw

Re: [PATCH 2/2] mm/mm_init: drop overlap_memmap_init()

Hi Wei,

Quoted 12 lines · Wei Yang
Quoted 11 lines · Mike Rapoport

From: "Mike Rapoport (Microsoft)" <[email protected]>

When ZONE_NORMAL and ZONE_MOVABLE could overlap because kernelcore=mirror didn't reduce the span of ZONE_NORMAL, initialization of the memory map had to skip overlapping pages during initialization of ZONE_MOVABLE to avoid double initialization of the same struct pages.

Since kernelcore=mirror works now the same way as other variants of kernelcore=/movablecore=, and adjusts the span of ZONE_NORMAL, there can't be an overlap between ZONE_NORMAL and ZONE_MOVABLE.

Quoted 5 lines
Quoted 2 lines

Remove overlap_memmap_init().

Do you think this is valuable to mention the double init for the overlapped range?

Not sure I follow.
We had this weird overlap_memmap_init() exactly to avoid double init.

Quoted 55 lines
Quoted 51 lines

---
mm/mm_init.c | 24 ------------------------

1 file changed, 24 deletions(-)
mm/mm_init.c +0 -24
index dce9dc9f2302..6f0a71ccca30 100644
@@ -783,28 +783,6 @@ void __meminit init_deferred_page(unsigned long pfn, int nid)
 	__init_deferred_page(pfn, nid);
 }
 
-/* If zone is ZONE_MOVABLE but memory is mirrored, it is an overlapped init */
-static bool __meminit
-overlap_memmap_init(unsigned long zone, unsigned long *pfn)
-{
-	static struct memblock_region *r __meminitdata;
-
-	if (mirrored_kernelcore && zone == ZONE_MOVABLE) {
-		if (!r || *pfn >= memblock_region_memory_end_pfn(r)) {
-			for_each_mem_region(r) {
-				if (*pfn < memblock_region_memory_end_pfn(r))
-					break;
-			}
-		}
-		if (*pfn >= memblock_region_memory_base_pfn(r) &&
-		    memblock_is_mirror(r)) {
-			*pfn = memblock_region_memory_end_pfn(r);
-			return true;
-		}
-	}
-	return false;
-}
-
 /*
 * Only struct pages that correspond to ranges defined by memblock.memory
 * are zeroed and initialized by going through __init_single_page() during
@@ -891,8 +869,6 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
 		 * function.  They do not exist on hotplugged memory.
 		 */
 		if (context == MEMINIT_EARLY) {
-			if (overlap_memmap_init(zone, &pfn))
-				continue;
 			if (defer_init(nid, pfn, zone_end_pfn)) {
 				deferred_struct_pages = true;
 				break;
Signature · 1 line
2.53.0
Signature · 2 lines
Wei Yang
Help you, Help me
Signature · 2 lines
Sincerely yours,
Mike.
#6 lore raw

Re: [PATCH 2/2] mm/mm_init: drop overlap_memmap_init()

Quoted 15 lines · Mike Rapoport

Hi Wei,

Quoted 12 lines · Wei Yang
Quoted 11 lines · Mike Rapoport

From: "Mike Rapoport (Microsoft)" <[email protected]>

When ZONE_NORMAL and ZONE_MOVABLE could overlap because kernelcore=mirror didn't reduce the span of ZONE_NORMAL, initialization of the memory map had to skip overlapping pages during initialization of ZONE_MOVABLE to avoid double initialization of the same struct pages.

Since kernelcore=mirror works now the same way as other variants of kernelcore=/movablecore=, and adjusts the span of ZONE_NORMAL, there can't be an overlap between ZONE_NORMAL and ZONE_MOVABLE.

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

Remove overlap_memmap_init().

Do you think this is valuable to mention the double init for the overlapped range?

Not sure I follow.
We had this weird overlap_memmap_init() exactly to avoid double init.

Yes, this is really weird.

As I described the detail in [1], before this change kernel would have overlapped zone range like:

Normal [100000, 1c0000]
Movable [140000, 1c0000]

Which means [140000, 1c0000] belongs to both Normal and Movable. And memmap_init_range() would be called on this range twice for Normal and Movable respectively. And we want overlap_memmap_init() could skip init this to Normal. Because actually, the memory in this range belongs to Movable Zone.

But overlap_memmap_init() only would skip for Zone Movable. This means when init to Zone Normal, it doesn't skip as we want. And when init to Zone Movable, we don't expect it to skip.

[1]: https://lore.kernel.org/linux-mm/[email protected]/

Signature · 2 lines
Wei Yang
Help you, Help me
author maintainer
#7 lore raw

Re: [PATCH 2/2] mm/mm_init: drop overlap_memmap_init()

Quoted 34 lines · Wei Yang
Quoted 15 lines · Mike Rapoport

Hi Wei,

Quoted 12 lines · Wei Yang
Quoted 11 lines · Mike Rapoport

From: "Mike Rapoport (Microsoft)" <[email protected]>

When ZONE_NORMAL and ZONE_MOVABLE could overlap because kernelcore=mirror didn't reduce the span of ZONE_NORMAL, initialization of the memory map had to skip overlapping pages during initialization of ZONE_MOVABLE to avoid double initialization of the same struct pages.

Since kernelcore=mirror works now the same way as other variants of kernelcore=/movablecore=, and adjusts the span of ZONE_NORMAL, there can't be an overlap between ZONE_NORMAL and ZONE_MOVABLE.

Quoted 9 lines
Quoted 5 lines
Quoted 2 lines

Remove overlap_memmap_init().

Do you think this is valuable to mention the double init for the overlapped range?

Not sure I follow.
We had this weird overlap_memmap_init() exactly to avoid double init.

Yes, this is really weird.

As I described the detail in [1], before this change kernel would have overlapped zone range like:

Normal [100000, 1c0000]
Movable [140000, 1c0000]

Quoted 8 lines

Which means [140000, 1c0000] belongs to both Normal and Movable. And memmap_init_range() would be called on this range twice for Normal and Movable respectively. And we want overlap_memmap_init() could skip init this to Normal. Because actually, the memory in this range belongs to Movable Zone.

But overlap_memmap_init() only would skip for Zone Movable. This means when init to Zone Normal, it doesn't skip as we want. And when init to Zone Movable, we don't expect it to skip.

I'll add something along these lines into the cover letter and will merge it as a merge commit.

Will send v2 soon.

Quoted 5 lines

[1]: https://lore.kernel.org/linux-mm/[email protected]/

Signature · 2 lines
Wei Yang
Help you, Help me
Signature · 2 lines
Sincerely yours,
Mike.
Reviewed-by on v1 2/2
#8 lore raw

Re: [PATCH 2/2] mm/mm_init: drop overlap_memmap_init()

In reply to Mike Rapoport
Quoted 11 lines · Mike Rapoport

From: "Mike Rapoport (Microsoft)" <[email protected]>

When ZONE_NORMAL and ZONE_MOVABLE could overlap because kernelcore=mirror didn't reduce the span of ZONE_NORMAL, initialization of the memory map had to skip overlapping pages during initialization of ZONE_MOVABLE to avoid double initialization of the same struct pages.

Since kernelcore=mirror works now the same way as other variants of kernelcore=/movablecore=, and adjusts the span of ZONE_NORMAL, there can't be an overlap between ZONE_NORMAL and ZONE_MOVABLE.

Quoted 6 lines

Remove overlap_memmap_init().

---

Signature · 3 lines
Cheers,

David
author maintainer
#9 lore raw
In reply to Mike Rapoport
Quoted 7 lines · Mike Rapoport

From: "Mike Rapoport (Microsoft)" <[email protected]>

Hi,

These patches make the behaviour of kernelcore= parameter uniform and treat mirror just as another way to size the zones and cleanup a weird part of the memory map initialization.

I'm queueing it for now in the memblock tree, will properly push after merge window is closed.

Quoted 12 lines

Mike Rapoport (Microsoft) (2):
mm/mm_init: don't overlap NORMAL and MOVABLE zones with kernelcore=mirror mm/mm_init: drop overlap_memmap_init()

 mm/mm_init.c | 60 +++-------------------------------------------------
 1 file changed, 3 insertions(+), 57 deletions(-)

base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48

Signature · 1 line
2.53.0
Signature · 2 lines
Sincerely yours,
Mike.