On 9/18/25 22:27, Chris Mason wrote: > > [ apologies for resending this, debugging sendmail ] > > Hi Balbir, > > On Mon, 8 Sep 2025 10:04:34 +1000 Balbir Singh <balb...@nvidia.com> wrote: >> Add routines to support allocation of large order zone device folios >> and helper functions for zone device folios, to check if a folio is >> device private and helpers for setting zone device data. >> >> When large folios are used, the existing page_free() callback in >> pgmap is called when the folio is freed, this is true for both >> PAGE_SIZE and higher order pages. >> >> Zone device private large folios do not support deferred split and >> scan like normal THP folios. > > [ ... ] > >> diff --git a/mm/memremap.c b/mm/memremap.c >> index 46cb1b0b6f72..66f9186b5500 100644 >> --- a/mm/memremap.c >> +++ b/mm/memremap.c >> @@ -453,11 +452,15 @@ void free_zone_device_folio(struct folio *folio) >> >> switch (pgmap->type) { >> case MEMORY_DEVICE_PRIVATE: >> + percpu_ref_put_many(&folio->pgmap->ref, nr); > > Here we're dropping nr refs > >> + pgmap->ops->page_free(&folio->page); >> + folio->page.mapping = NULL; >> + break; >> case MEMORY_DEVICE_COHERENT: >> if (WARN_ON_ONCE(!pgmap->ops || !pgmap->ops->page_free)) >> break; >> - pgmap->ops->page_free(folio_page(folio, 0)); >> - put_dev_pagemap(pgmap); >> + pgmap->ops->page_free(&folio->page); >> + percpu_ref_put(&folio->pgmap->ref); > > Here we're dropping one ref? > >> break; >> >> case MEMORY_DEVICE_GENERIC: >> @@ -480,14 +483,23 @@ void free_zone_device_folio(struct folio *folio) >> } >> } >> >> -void zone_device_page_init(struct page *page) >> +void zone_device_folio_init(struct folio *folio, unsigned int order) >> { >> + struct page *page = folio_page(folio, 0); >> + >> + VM_WARN_ON_ONCE(order > MAX_ORDER_NR_PAGES); >> + >> /* >> * Drivers shouldn't be allocating pages after calling >> * memunmap_pages(). >> */ >> - WARN_ON_ONCE(!percpu_ref_tryget_live(&page_pgmap(page)->ref)); >> - set_page_count(page, 1); >> + WARN_ON_ONCE(!percpu_ref_tryget_many(&page_pgmap(page)->ref, 1 << >> order)); > > Here we always bump by 1 << order > > I hesitate to send this one because I don't know the code at all, but the > AI review prompts keep flagging this apparent refcount mismatch, and it looks > real to me. > > Are the differences in refcount handling inside free_zone_device_folio() > intentional? >
Thanks for the review! I've posted a v6, but in general 1. Large folios for supported for device-private pages, hence the nr (from folio_order) 2. The user of the folios specifies the order (for mTHP), a folio gets get created with that order 3. What you are seeing with the percpu_ref_put for two different cases MEMORY_DEVICE_COHERENT and MEMORY_DEVICE_PRIVATE The reason I point you to v6, is that the diff is a little bit different there Balbir