Re: [PATCH v10 2/8] mm: factor zone-device page init helpers out of __init_zone_device_page

[Mike Rapoport <rppt@kernel.org>]

On Mon, Aug 17, 2026 at 03:11:56PM +0800, Li Zhe wrote:
> On 8/16/26 8:04 PM, Mike Rapoport wrote:
> > Hi,
> >
> >> memmap_init_zone_device() currently mixes refcount policy and core
> >> ZONE_DEVICE page setup in a single helper.
> >>
> >> Factor the refcount-reset predicate into pagemap_requires_refcount_reset(),
> >> move the common page initialization into __zone_device_page_init(), and
> >> wrap the existing slow path in zone_device_page_init_slow().
> >>
> >> This keeps the slow-path behaviour unchanged and gives later patches
> >> reusable helper boundaries.
> >>
> >> No functional change intended.
> >>
> >> Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
> >> Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> >>
> >> diff --git a/mm/mm_init.c b/mm/mm_init.c
> >> index 95808ab5cfdb..a70acb7431a6 100644
> >> --- a/mm/mm_init.c
> >> +++ b/mm/mm_init.c
> >> @@ -1005,11 +1005,37 @@ static void __init memmap_init(void)
> >>   }
> >>   
> >>   #ifdef CONFIG_ZONE_DEVICE
> >> -static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
> >> +/*
> >> + * Return true when memmap_init_zone_device() must initialize the page
> >> + * refcount to 0. MEMORY_DEVICE_GENERIC pages regain a refcount of 1 in
> >> + * the free path, while the remaining ZONE_DEVICE types start from 0 here
> >> + * and raise the count again when the allocator or driver hands the page
> >> + * out.
> >> + */
> >> +static inline bool pagemap_requires_refcount_reset(const struct dev_pagemap *pgmap)
> >> +{
> >> +	/*
> >> +	 * MEMORY_DEVICE_GENERIC pages regain a refcount of 1 in the free
> >> +	 * path. The remaining ZONE_DEVICE types start from 0 here and raise
> >> +	 * the count again when the allocator or driver hands the page out.
> >> +	 */
> >> +	switch (pgmap->type) {
> >> +	case MEMORY_DEVICE_FS_DAX:
> >> +	case MEMORY_DEVICE_PRIVATE:
> >> +	case MEMORY_DEVICE_COHERENT:
> >> +	case MEMORY_DEVICE_PCI_P2PDMA:
> >> +		return true;
> >> +	case MEMORY_DEVICE_GENERIC:
> >> +		return false;
> >> +	}
> >> +
> >> +	return false;
> >> +}
> >> +
> >> +static void __ref __zone_device_page_init(struct page *page, unsigned long pfn,
> >>   					  unsigned long zone_idx, int nid,
> >>   					  struct dev_pagemap *pgmap)
> >>   {
> >> -
> >>   	__init_single_page(page, pfn, zone_idx, nid);
> >>   
> >>   	/*
> >> @@ -1028,23 +1054,15 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
> >>   	 */
> >>   	page_folio(page)->pgmap = pgmap;
> >>   	page->zone_device_data = NULL;
> >> +}
> >>   
> >> -	/*
> >> -	 * MEMORY_DEVICE_GENERIC pages regain a refcount of 1 in the free
> >> -	 * path. The remaining ZONE_DEVICE types start from 0 here and raise
> >> -	 * the count again when the allocator or driver hands the page out.
> >> -	 */
> >> -	switch (pgmap->type) {
> >> -	case MEMORY_DEVICE_FS_DAX:
> >> -	case MEMORY_DEVICE_PRIVATE:
> >> -	case MEMORY_DEVICE_COHERENT:
> >> -	case MEMORY_DEVICE_PCI_P2PDMA:
> >> +static void __ref zone_device_page_init_slow(struct page *page,
> >> +		unsigned long pfn, unsigned long zone_idx, int nid,
> >> +		struct dev_pagemap *pgmap)
> >> +{
> >> +	__zone_device_page_init(page, pfn, zone_idx, nid, pgmap);
> >> +	if (pagemap_requires_refcount_reset(pgmap))
> >>   		set_page_count(page, 0);
> >> -		break;
> >> -
> >> -	case MEMORY_DEVICE_GENERIC:
> >> -		break;
> >> -	}
> >>   }
> >>   
> >>   /*
> >> @@ -1090,7 +1108,7 @@ static void __ref memmap_init_compound(struct page *head,
> >>   	for (pfn = head_pfn + 1; pfn < end_pfn; pfn++) {
> >>   		struct page *page = pfn_to_page(pfn);
> >>   
> >> -		__init_zone_device_page(page, pfn, zone_idx, nid, pgmap);
> >> +		zone_device_page_init_slow(page, pfn, zone_idx, nid, pgmap);
> >>   		prep_compound_tail(page, head, order);
> >>   		set_page_count(page, 0);
> >>   	}
> >> @@ -1126,7 +1144,7 @@ void __ref memmap_init_zone_device(struct zone *zone,
> >>   	for (pfn = start_pfn; pfn < end_pfn; pfn += pfns_per_compound) {
> >>   		struct page *page = pfn_to_page(pfn);
> >>   
> >> -		__init_zone_device_page(page, pfn, zone_idx, nid, pgmap);
> >> +		zone_device_page_init_slow(page, pfn, zone_idx, nid, pgmap);
> > It looks like all callers of __init_zone_device_page() are now calling
> > zone_device_page_init_slow() that with series evolution became a thin
> > wrapper for __zone_device_page_init().
> >
> > Maybe we can just rename __init_zone_device_page() to
> > __zone_device_page_init() and replace the open coded setting of the page
> > count with the call to pagemap_requires_refcount_reset() an be done
> > here?
> 
> 
> Yes, that would make patch 2 simpler.
> 
> The reason I kept the refcount policy outside __zone_device_page_init()
> is the compound-tail initialization added later in patch 5. Tail pages
> still need the same basic ZONE_DEVICE struct page initialization as head
> pages, including __init_single_page(), PageReserved, pgmap and
> zone_device_data setup. After that, the tail-page path calls
> prep_compound_tail() and sets the tail refcount to 0 unconditionally.
> 
> Folding pagemap_requires_refcount_reset() into __zone_device_page_init()
> would make that helper include the head-page refcount policy even for
> callers that will immediately apply the tail-page refcount rule
> afterwards.

But you still call zone_device_page_init_slow() from
zone_device_tail_page_init() so it's anyway there.

I'd suggest combining this and the next patch into one, keeping
__init_zone_device_page() name without renaming it at all and open-coding
the initialization of the first head and the first tail page to properly
set refcount and call other prep methods.

I'd also pull the initialization of the first page in both cases out of the
loop and make the loop only use the _template version.
 
> That preserves the final state, but I was trying to keep the common
> ZONE_DEVICE page setup separate from the head-page refcount policy for
> this reason.
> 
> Would you still prefer the simpler renamed helper here, or should I keep
> the common ZONE_DEVICE page setup separate from the refcount policy and
> make the naming clearer in v11?
> 
> Thanks,
> Zhe
> 

-- 
Sincerely yours,
Mike.