Re: [PATCH v5 4/8] mm: add a template-based fast path for zone-device page init

["Li Zhe" <lizhe.67@bytedance.com>]

On Tue, 7 Jul 2026 13:02:52 +1000, apopple@nvidia.com wrote:
> > +static inline bool zone_device_page_init_optimization_enabled(void)
> > +{
> > +	/*
> > +	 * The template fast path copies a preinitialized struct page image.
> > +	 * Skip it when the page_ref_set tracepoint is enabled.
> > +	 */
> > +	return !page_ref_tracepoint_active(page_ref_set);
> > +}
> > +
> > +static inline void zone_device_template_page_init(struct page *template,
> > +						  struct page *src)
> > +{
> > +	memcpy(template, src, sizeof(*template));
> 
> I'd drop this function and just open-code the memcpy as I think that ends up
> being clearer and removes a naming confusion - I kept mixing up the purpose of
> `zone_device_template_page_init` and `zone_device_page_init_from_template`

Agreed. I will drop zone_device_template_page_init() and open-code
that memcpy() in v6.

> > -	for (pfn = start_pfn; pfn < end_pfn; pfn += pfns_per_compound) {
> > -		struct page *page = pfn_to_page(pfn);
> > +
> > +	if (!nr_pages)
> > +		return;
> > +
> > +	pfn = start_pfn;
> > +	/*
> > +	 * Seed the reusable head-page template from the first real struct
> > +	 * page, because the existing page-init and pageblock helpers expect
> > +	 * a real memmap entry rather than a stack object.
> > +	 */
> > +	if (use_template) {
> > +		struct page *page = pfn_to_page(start_pfn);
> >
> >  		zone_device_page_init_slow(page, pfn, zone_idx, nid, pgmap);
> > +		zone_device_template_page_init(&template, page);
> > +		if (pfns_per_compound != 1)
> > +			memmap_init_compound(page, pfn, zone_idx, nid, pgmap,
> > +				compound_nr_pages(start_pfn, altmap, pgmap));
> > +		pfn += pfns_per_compound;
> 
> I think it would be clearer and less error prone to not unroll the loop here.
> Instead just initialise the template page and leave the for loop starting at
> pfn = start_pfn and let it handle the rest of the steps. I can't imagine the
> extra call to zone_deivce_page_init_from_template() would cause a noticable
> perf impact.
> 
> It also avoids the obvious danger of someone updating the loop but missing the
> unrolled version above.

Thanks. I will keep the initial slow-path template seeding as a
separate step, still using the first real memmap entry for that, but
let the main loop start at start_pfn again so the first PFN goes
through the same loop body as the rest. That should make the code
clearer and avoid the duplicated first-iteration path drifting from
the main loop in later changes.

Thanks,
Zhe