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 <[email protected]> >> Reviewed-by: Mike Rapoport (Microsoft) <[email protected]> >> >> 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. 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
