Re: [PATCH v5 4/8] mm: add a template-based fast path for zone-device page init
"Li Zhe" <[email protected]>
| Newsgroups | org.kernel.vger.linux-arch,org.kernel.vger.linux-hardening,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Tue, 7 Jul 2026 13:02:52 +1000, [email protected] 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