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

[Li Zhe]

On 11/9/26 10:57 AM, Muchun Song wrote:
>
>
> 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@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 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.

Agreed, thanks. I will add a short comment in v2.

Thanks,
Zhe

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