On Fri, Aug 28, 2026 at 11:46:05AM +0200, Francois Dugast wrote: > Hi, > > On Fri, Aug 28, 2026 at 04:36:52PM +0800, Junhua Shen wrote: > > > > The base-page count for a migrate.src[] entry was open-coded in several > > places along the migration path, and these variants handled > > MIGRATE_PFN_COMPOUND entries inconsistently. > > Nit: not just inconsistently, I would mention the existing undercount issue > this patch fixes when VALID=0 && COMPOUND=1. > > > > > Factor it out into drm_pagemap_src_pfn_nr_pages() and use it in > > drm_pagemap_cpages() and both accounting loops of > > drm_pagemap_migrate_to_devmem(), so every site agrees on the base-page > > count and iteration stride. > > > > Signed-off-by: Junhua Shen <[email protected]> > > Consequently, we need a "Fixes" tag here. > Agreed. In v2 I'll lead with the bug (VALID=0 && COMPOUND=1 undercounted as 1 instead of HPAGE_PMD_NR, risking a spurious -EBUSY) and add a Fixes tag.
Thanks! Junhua > With that, the change itself LGTM: > > Reviewed-by: Francois Dugast <[email protected]> > > Francois > > > --- > > drivers/gpu/drm/drm_pagemap.c | 60 +++++++++++++++++++++++++---------- > > 1 file changed, 43 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > > index 892b325fa99b..a31cc4f0af68 100644 > > --- a/drivers/gpu/drm/drm_pagemap.c > > +++ b/drivers/gpu/drm/drm_pagemap.c > > @@ -554,6 +554,37 @@ static int drm_pagemap_migrate_range(struct > > drm_pagemap_devmem *devmem, > > return ret; > > } > > > > +/** > > + * drm_pagemap_src_pfn_nr_pages() - Decode src entry and return base-page > > count > > + * @src_pfn: Source migrate entry > > + * @src_page: Optional decoded source page when MIGRATE_PFN_VALID is set > > + * > > + * Decode @src_pfn to compute how many base pages it represents: use folio > > + * page count for valid entries, HPAGE_PMD_NR for COMPOUND-only entries, > > + * otherwise 1. > > + * > > + * Return: Number of base pages represented by @src_pfn. > > + */ > > +static unsigned long drm_pagemap_src_pfn_nr_pages(unsigned long src_pfn, > > + struct page **src_page) > > +{ > > + struct page *page = NULL; > > + unsigned long nr_pages = 1; > > + > > + if (src_pfn & MIGRATE_PFN_VALID) { > > + page = migrate_pfn_to_page(src_pfn); > > + if (page) > > + nr_pages = NR_PAGES(folio_order(page_folio(page))); > > + } else if (src_pfn & MIGRATE_PFN_COMPOUND) { > > + nr_pages = HPAGE_PMD_NR; > > + } > > + > > + if (src_page) > > + *src_page = page; > > + > > + return nr_pages; > > +} > > + > > /** > > * drm_pagemap_cpages() - Count collected pages > > * @migrate_pfn: Array of migrate_pfn entries to account > > @@ -570,20 +601,14 @@ static int drm_pagemap_cpages(unsigned long > > *migrate_pfn, unsigned long npages) > > unsigned long i, cpages = 0; > > > > for (i = 0; i < npages;) { > > - struct page *page = migrate_pfn_to_page(migrate_pfn[i]); > > - struct folio *folio; > > - unsigned int order = 0; > > + unsigned long src_pfn = migrate_pfn[i]; > > + struct page *page; > > + unsigned long nr_pages = drm_pagemap_src_pfn_nr_pages(src_pfn, > > &page); > > > > - if (page) { > > - folio = page_folio(page); > > - order = folio_order(folio); > > - cpages += NR_PAGES(order); > > - } else if (migrate_pfn[i] & MIGRATE_PFN_COMPOUND) { > > - order = HPAGE_PMD_ORDER; > > - cpages += NR_PAGES(order); > > - } > > + if (page || (src_pfn & MIGRATE_PFN_COMPOUND)) > > + cpages += nr_pages; > > > > - i += NR_PAGES(order); > > + i += nr_pages; > > } > > > > return cpages; > > @@ -703,8 +728,9 @@ int drm_pagemap_migrate_to_devmem(struct > > drm_pagemap_devmem *devmem_allocation, > > > > /* Count device-private pages to migrate */ > > for (i = 0; i < npages;) { > > - struct page *src_page = migrate_pfn_to_page(migrate.src[i]); > > - unsigned long nr_pages = src_page ? > > NR_PAGES(folio_order(page_folio(src_page))) : 1; > > + unsigned long src_pfn = migrate.src[i]; > > + struct page *src_page; > > + unsigned long nr_pages = drm_pagemap_src_pfn_nr_pages(src_pfn, > > &src_page); > > > > if (src_page && is_zone_device_page(src_page)) { > > if (page_pgmap(src_page) == pagemap) > > @@ -818,10 +844,10 @@ int drm_pagemap_migrate_to_devmem(struct > > drm_pagemap_devmem *devmem_allocation, > > migrate_vma_pages(&migrate); > > > > for (i = 0; !err && i < npages;) { > > - struct page *page = migrate_pfn_to_page(migrate.src[i]); > > - unsigned long nr_pages = page ? > > NR_PAGES(folio_order(page_folio(page))) : 1; > > + unsigned long src_pfn = migrate.src[i]; > > + unsigned long nr_pages = drm_pagemap_src_pfn_nr_pages(src_pfn, > > NULL); > > > > - if (migrate.src[i] & MIGRATE_PFN_MIGRATE) > > + if (src_pfn & MIGRATE_PFN_MIGRATE) > > migrated_pages += nr_pages; > > > > i += nr_pages; > > -- > > 2.34.1 > >
