From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

deferred_init_memmap_chunk() calls deferred_init_maxorder() to initialize
struct pages in MAX_ORDER_NR_PAGES because according to commit 0e56acae4b4d
("mm: initialize MAX_ORDER_NR_PAGES at a time instead of doing larger
sections") this provides better cache locality than initializing the memory
map in larger sections.

The looping through free memory ranges is quite cumbersome in the current
implementation as it is divided between deferred_init_memmap_chunk() and
deferred_init_maxorder(). Besides, the latter has two loops, one that
initializes struct pages and another one that frees them.

There is no need in two loops because it is safe to free pages in groups
smaller than MAX_ORDER_NR_PAGES. Even if lookup for a buddy page will
access a struct page ahead of the pages being initialized, that page is
guaranteed to be initialized either by memmap_init_reserved_pages() or by
init_unavailable_range().

Simplify the code by moving initialization and freeing of the pages into
deferred_init_memmap_chunk() and dropping deferred_init_maxorder().

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 mm/mm_init.c | 122 ++++++++++++---------------------------------------
 1 file changed, 29 insertions(+), 93 deletions(-)

diff --git a/mm/mm_init.c b/mm/mm_init.c
index 1ecfba98ddbe..bca05891cb16 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -2046,111 +2046,47 @@ static unsigned long __init deferred_init_pages(struct zone *zone,
 }
 
 /*
- * This function is meant to pre-load the iterator for the zone init from
- * a given point.
- * Specifically it walks through the ranges starting with initial index
- * passed to it until we are caught up to the first_init_pfn value and
- * exits there. If we never encounter the value we return false indicating
- * there are no valid ranges left.
- */
-static bool __init
-deferred_init_mem_pfn_range_in_zone(u64 *i, struct zone *zone,
-				    unsigned long *spfn, unsigned long *epfn,
-				    unsigned long first_init_pfn)
-{
-	u64 j = *i;
-
-	if (j == 0)
-		__next_mem_pfn_range_in_zone(&j, zone, spfn, epfn);
-
-	/*
-	 * Start out by walking through the ranges in this zone that have
-	 * already been initialized. We don't need to do anything with them
-	 * so we just need to flush them out of the system.
-	 */
-	for_each_free_mem_pfn_range_in_zone_from(j, zone, spfn, epfn) {
-		if (*epfn <= first_init_pfn)
-			continue;
-		if (*spfn < first_init_pfn)
-			*spfn = first_init_pfn;
-		*i = j;
-		return true;
-	}
-
-	return false;
-}
-
-/*
- * Initialize and free pages. We do it in two loops: first we initialize
- * struct page, then free to buddy allocator, because while we are
- * freeing pages we can access pages that are ahead (computing buddy
- * page in __free_one_page()).
+ * Initialize and free pages.
+ *
+ * At this point reserved pages and struct pages that correspond to holes in
+ * memblock.memory are already intialized so every free range has a valid
+ * memory map around it.
+ * This ensures that access of pages that are ahead of the range being
+ * initialized (computing buddy page in __free_one_page()) always reads a valid
+ * struct page.
  *
- * In order to try and keep some memory in the cache we have the loop
- * broken along max page order boundaries. This way we will not cause
- * any issues with the buddy page computation.
+ * In order to try and improve CPU cache locality we have the loop broken along
+ * max page order boundaries.
  */
 static unsigned long __init
-deferred_init_maxorder(u64 *i, struct zone *zone, unsigned long *start_pfn,
-		       unsigned long *end_pfn)
+deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
+			   struct zone *zone)
 {
-	unsigned long mo_pfn = ALIGN(*start_pfn + 1, MAX_ORDER_NR_PAGES);
-	unsigned long spfn = *start_pfn, epfn = *end_pfn;
+	int nid = zone_to_nid(zone);
 	unsigned long nr_pages = 0;
-	u64 j = *i;
-
-	/* First we loop through and initialize the page values */
-	for_each_free_mem_pfn_range_in_zone_from(j, zone, start_pfn, end_pfn) {
-		unsigned long t;
-
-		if (mo_pfn <= *start_pfn)
-			break;
-
-		t = min(mo_pfn, *end_pfn);
-		nr_pages += deferred_init_pages(zone, *start_pfn, t);
-
-		if (mo_pfn < *end_pfn) {
-			*start_pfn = mo_pfn;
-			break;
-		}
-	}
-
-	/* Reset values and now loop through freeing pages as needed */
-	swap(j, *i);
-
-	for_each_free_mem_pfn_range_in_zone_from(j, zone, &spfn, &epfn) {
-		unsigned long t;
-
-		if (mo_pfn <= spfn)
-			break;
+	phys_addr_t start, end;
+	u64 i = 0;
 
-		t = min(mo_pfn, epfn);
-		deferred_free_pages(spfn, t - spfn);
+	for_each_free_mem_range(i, nid, 0, &start, &end, NULL) {
+		unsigned long spfn = PFN_UP(start);
+		unsigned long epfn = PFN_DOWN(end);
 
-		if (mo_pfn <= epfn)
+		if (spfn >= end_pfn)
 			break;
-	}
 
-	return nr_pages;
-}
+		spfn = max(spfn, start_pfn);
+		epfn = min(epfn, end_pfn);
 
-static unsigned long __init
-deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
-			   struct zone *zone)
-{
-	unsigned long nr_pages = 0;
-	unsigned long spfn, epfn;
-	u64 i = 0;
+		while (spfn < epfn) {
+			unsigned long mo_pfn = ALIGN(spfn + 1, MAX_ORDER_NR_PAGES);
+			unsigned long chunk_end = min(mo_pfn, epfn);
 
-	deferred_init_mem_pfn_range_in_zone(&i, zone, &spfn, &epfn, start_pfn);
+			nr_pages += deferred_init_pages(zone, spfn, chunk_end);
+			deferred_free_pages(spfn, chunk_end - spfn);
 
-	/*
-	 * Initialize and free pages in MAX_PAGE_ORDER sized increments so that
-	 * we can avoid introducing any issues with the buddy allocator.
-	 */
-	while (spfn < end_pfn) {
-		nr_pages += deferred_init_maxorder(&i, zone, &spfn, &epfn);
-		cond_resched();
+			spfn = chunk_end;
+			cond_resched();
+		}
 	}
 
 	return nr_pages;
-- 
2.50.1

>   
> -static unsigned long __init
> -deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
> -			   struct zone *zone)
> -{
> -	unsigned long nr_pages = 0;
> -	unsigned long spfn, epfn;
> -	u64 i = 0;
> +		while (spfn < epfn) {
> +			unsigned long mo_pfn = ALIGN(spfn + 1, MAX_ORDER_NR_PAGES);
> +			unsigned long chunk_end = min(mo_pfn, epfn);
>   
> -	deferred_init_mem_pfn_range_in_zone(&i, zone, &spfn, &epfn, start_pfn);
> +			nr_pages += deferred_init_pages(zone, spfn, chunk_end);
> +			deferred_free_pages(spfn, chunk_end - spfn);


I assume the expectation is that all PFNs in the start_pfn -> end_pfn 
range will go to this zone, correct?

-- 
Cheers

David / dhildenb

On Tue, Aug 19, 2025 at 09:54:22AM +0200, David Hildenbrand wrote:
>> -static unsigned long __init
>> -deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
>> -			   struct zone *zone)
>> -{
>> -	unsigned long nr_pages = 0;
>> -	unsigned long spfn, epfn;
>> -	u64 i = 0;
>> +		while (spfn < epfn) {
>> +			unsigned long mo_pfn = ALIGN(spfn + 1, MAX_ORDER_NR_PAGES);
>> +			unsigned long chunk_end = min(mo_pfn, epfn);
>> -	deferred_init_mem_pfn_range_in_zone(&i, zone, &spfn, &epfn, start_pfn);
>> +			nr_pages += deferred_init_pages(zone, spfn, chunk_end);
>> +			deferred_free_pages(spfn, chunk_end - spfn);
>
>
>I assume the expectation is that all PFNs in the start_pfn -> end_pfn range
>will go to this zone, correct?

I think so.

defer_init only apply to the highest zone in one node.

>
>-- 
>Cheers
>
>David / dhildenb
>

-- 
Wei Yang
Help you, Help me

On Tue, Aug 19, 2025 at 09:22:54AM +0000, Wei Yang wrote:
> On Tue, Aug 19, 2025 at 09:54:22AM +0200, David Hildenbrand wrote:
> >> -static unsigned long __init
> >> -deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
> >> -			   struct zone *zone)
> >> -{
> >> -	unsigned long nr_pages = 0;
> >> -	unsigned long spfn, epfn;
> >> -	u64 i = 0;
> >> +		while (spfn < epfn) {
> >> +			unsigned long mo_pfn = ALIGN(spfn + 1, MAX_ORDER_NR_PAGES);
> >> +			unsigned long chunk_end = min(mo_pfn, epfn);
> >> -	deferred_init_mem_pfn_range_in_zone(&i, zone, &spfn, &epfn, start_pfn);
> >> +			nr_pages += deferred_init_pages(zone, spfn, chunk_end);
> >> +			deferred_free_pages(spfn, chunk_end - spfn);
> >
> >
> >I assume the expectation is that all PFNs in the start_pfn -> end_pfn range
> >will go to this zone, correct?
> 
> I think so.
> 
> defer_init only apply to the highest zone in one node.

Right, we defer initialization of last zone in every node and there is a
thread per node that does the initialization.

-- 
Sincerely yours,
Mike.

On 19.08.25 12:39, Mike Rapoport wrote:
> On Tue, Aug 19, 2025 at 09:22:54AM +0000, Wei Yang wrote:
>> On Tue, Aug 19, 2025 at 09:54:22AM +0200, David Hildenbrand wrote:
>>>> -static unsigned long __init
>>>> -deferred_init_memmap_chunk(unsigned long start_pfn, unsigned long end_pfn,
>>>> -			   struct zone *zone)
>>>> -{
>>>> -	unsigned long nr_pages = 0;
>>>> -	unsigned long spfn, epfn;
>>>> -	u64 i = 0;
>>>> +		while (spfn < epfn) {
>>>> +			unsigned long mo_pfn = ALIGN(spfn + 1, MAX_ORDER_NR_PAGES);
>>>> +			unsigned long chunk_end = min(mo_pfn, epfn);
>>>> -	deferred_init_mem_pfn_range_in_zone(&i, zone, &spfn, &epfn, start_pfn);
>>>> +			nr_pages += deferred_init_pages(zone, spfn, chunk_end);
>>>> +			deferred_free_pages(spfn, chunk_end - spfn);
>>>
>>>
>>> I assume the expectation is that all PFNs in the start_pfn -> end_pfn range
>>> will go to this zone, correct?
>>
>> I think so.
>>
>> defer_init only apply to the highest zone in one node.
> 
> Right, we defer initialization of last zone in every node and there is a
> thread per node that does the initialization.

Thanks, my memory comes back :)

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

-- 
Cheers

David / dhildenb