On 8/17/26 7:15 PM, Mike Rapoport wrote: > 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 <[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. > But you still call zone_device_page_init_slow() from > zone_device_tail_page_init() so it's anyway there.
Thanks for pointing this out. I was mistaken about the current split. > > 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. Thanks for the suggestion. This is indeed clearer. I will fix this in v11. Thanks, Zhe > >> 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 >>
