Re: [PATCH 1/2] mm/mm_init: factor out memmap_init_page_from_template()

[Li Zhe]

On 11/6/26 6:34 PM, Balbir Singh wrote:
> On Thu, Nov 05, 2026 at 08:06:14PM +0800, Li Zhe wrote:
>> ZONE_DEVICE memmap initialization already uses a reusable struct page
>> template when initializing compound tail pages.  The template mostly
>> contains invariant state, but a few fields still depend on the target
>> PFN and must be refreshed before copying the template into each
>> destination struct page.
>>
>> Factor that operation into memmap_init_page_from_template() so other
>> boot-time memmap initialization paths can reuse the same helper.
>>
>> No functional change intended.
>>
>> Signed-off-by: Li Zhe <lizhe.67@xxxxxxxxxxxxx>
>> ---
>>   mm/mm_init.c | 29 +++++++++++++++--------------
>>   mm/mm_init.h |  2 ++
>>   2 files changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/mm/mm_init.c b/mm/mm_init.c
>> index 2a0e782a151d..495e20fbdec0 100644
>> --- a/mm/mm_init.c
>> +++ b/mm/mm_init.c
>> @@ -625,6 +625,17 @@ void __meminit __init_single_page(struct page *page, unsigned long pfn,
>>   	vmemmap_optimization_verify_zone(page);
>>   }
>>   
>> +void __meminit memmap_init_page_from_template(struct page *page,
>> +		unsigned long pfn, unsigned long zone, struct page *template)
>> +{
>> +	set_page_section_from_pfn(template, pfn);
>> +#ifdef WANT_PAGE_VIRTUAL
>> +	if (!is_highmem_idx(zone))
>> +		set_page_address(template, __va(pfn << PAGE_SHIFT));
>> +#endif
>> +	memcpy_nontemporal(page, template, sizeof(*page));
>> +}
>> +
> Can we make the template a const? Why can't we do
>
> void __meminit memmap_init_page_from_template(struct page *page,
> 		unsigned long pfn, unsigned long zone,
> 		const struct page *template)
> {
> 	memcpy_nontemporal(page, template, sizeof(*page));
>
> 	set_page_section_from_pfn(page, pfn);
> #ifdef WANT_PAGE_VIRTUAL
> 	if (!is_highmem_idx(zone))
> 		set_page_address(page, __va(pfn << PAGE_SHIFT));
> #endif
>
> That way we don't update template once it is set, if template is r/o,
> then we don't need both first page and template in the caller to
> be different.
I kept it this way to preserve the memcpy_nontemporal() write pattern:
update the PFN-dependent fields in the template first, then copy the
final state to the destination page once.

If we copy first and then update the destination page, the hot loop
would do a non-temporal copy followed by normal stores to the same
struct page. That is the pattern this helper avoids.

Thanks,
Zhe
>
>>   #ifdef CONFIG_NUMA
>>   /*
>>    * During memory init memblocks map pfns to nids. The search is expensive and
>> @@ -1073,17 +1084,6 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
>>   	}
>>   }
>>   
>> -static void zone_device_page_init_from_template(struct page *page,
>> -		unsigned long pfn, struct page *template)
>> -{
>> -	set_page_section_from_pfn(template, pfn);
>> -#ifdef WANT_PAGE_VIRTUAL
>> -	if (!is_highmem_idx(ZONE_DEVICE))
>> -		set_page_address(template, __va(pfn << PAGE_SHIFT));
>> -#endif
>> -	memcpy_nontemporal(page, template, sizeof(*page));
>> -}
>> -
>>   /*
>>    * With compound page geometry and when struct pages are stored in ram most
>>    * tail pages are reused. Consequently, the amount of unique struct pages to
>> @@ -1136,8 +1136,8 @@ static void __ref memmap_init_compound(struct page *head,
>>   
>>   	/* Initialize the remaining tail pages from template. */
>>   	for (pfn = head_pfn + 2; pfn < end_pfn; pfn++)
>> -		zone_device_page_init_from_template(pfn_to_page(pfn), pfn,
>> -						    &template);
>> +		memmap_init_page_from_template(pfn_to_page(pfn), pfn,
>> +					       ZONE_DEVICE, &template);
>>   	prep_compound_head(head, order);
>>   }
>>   
>> @@ -1192,7 +1192,8 @@ void __ref memmap_init_zone_device(struct zone *zone,
>>   	for (; pfn < end_pfn; pfn += pfns_per_compound) {
>>   		page = pfn_to_page(pfn);
>>   
>> -		zone_device_page_init_from_template(page, pfn, &template);
>> +		memmap_init_page_from_template(page, pfn, ZONE_DEVICE,
>> +					       &template);
>>   
>>   		if (IS_ALIGNED(pfn, PAGES_PER_SECTION))
>>   			cond_resched();
>> diff --git a/mm/mm_init.h b/mm/mm_init.h
>> index 0034e37dbabd..22c465122b47 100644
>> --- a/mm/mm_init.h
>> +++ b/mm/mm_init.h
>> @@ -111,5 +111,7 @@ void memblock_free_all(void);
>>   
>>   void __meminit __init_single_page(struct page *page, unsigned long pfn,
>>   				unsigned long zone, int nid);
>> +void __meminit memmap_init_page_from_template(struct page *page,
>> +		unsigned long pfn, unsigned long zone, struct page *template);
>>   
>>   #endif /* __MM_MM_INIT_H */
>> -- 
>
> Balbir