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

[Muchun Song <muchun.song@linux.dev>]

On 2026/11/9 10:51, Li Zhe wrote:
> 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@bytedance.com>
>>> ---
>>>    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 used to think that too.

> 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.

After thinking it through, that should be the reason. To prevent
similar confusion for others, I suggest adding a comment here
explaining why this specific order is used.

Thanks.

>
> 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