[PATCH] drm/pagemap: centralize migrate src base-page counting

[PATCH] drm/pagemap: centralize migrate src base-page counting

[Junhua Shen]

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.

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 <Junhua.Shen@amd.com>
---
 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

Re: [PATCH] drm/pagemap: centralize migrate src base-page counting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect base-page count calculation for large folios desynchronizes migration loops and skips valid entries.
- [High] Permanent device memory leak on migration error path.
--

commit f9a15fc79a089e7970a0abb86569d6cf4a696964
Author: Junhua Shen <Junhua.Shen@amd.com>

drm/pagemap: centralize migrate src base-page counting

This commit factors out the base-page count calculation for migrate.src
entries into a new drm_pagemap_src_pfn_nr_pages function. It applies this
new helper to the accounting loops to consistently handle compound pages.

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 892b325fa99b8..a31cc4f0af68c 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,
[ ... ]
> +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)));

[Severity: High]
This is a pre-existing issue, but does computing the stride using the backing
folio's size skip independent PTE entries? If a VMA maps a transparent huge
page (THP) via PTEs, migrate_vma_setup produces individual base-page entries.
However, using folio_order(page_folio(page)) returns the full compound order
(e.g., 512). The loops incrementing by this value would then skip the next
511 valid entries in the migrate.src array.

[Severity: High]
This is also a pre-existing issue related to the above, but does this create
a desync between the migration steps? 

In the central loop in drivers/gpu/drm/drm_pagemap.c:drm_pagemap_migrate_to_devmem():

	for (i = 0; i < npages;) {
		unsigned int order = 0;
		...
		if (migrate.src[i] & MIGRATE_PFN_COMPOUND) {
			order = HPAGE_PMD_ORDER;
			...
		}
		...
		i += NR_PAGES(order);
	}

Because order is 0 for PTE-mapped THPs here (MIGRATE_PFN_COMPOUND is false),
this central loop correctly strides by 1. However, the accounting loops that
use the new drm_pagemap_src_pfn_nr_pages() helper will stride by the full
folio size. Will this result in device pages being allocated and processed
here, but skipped in the accounting phases?

[Severity: High]
There is another pre-existing issue on the migration error path. Does
truncating npages cause device pages to be permanently leaked? 

drivers/gpu/drm/drm_pagemap.c:drm_pagemap_migrate_to_devmem() {
	...
	err = ops->populate_devmem_pfn(devmem_allocation, npages, migrate.dst);
	...
	for (i = 0; i < npages;) {
		...
		err = drm_pagemap_migrate_range(...);
		if (err) {
			npages = i + 1;
			goto err_finalize;
		}
		...
	}
	...
err_finalize:
	if (err) {
		drm_pagemap_migration_unlock_put_pages(npages, migrate.dst);
		for (i = npages; i < npages_in_range(start, end); ++i)
			migrate.dst[i] = 0;
	}
	...
}

When drm_pagemap_migrate_range() fails, npages is truncated. The cleanup
code in err_finalize then only handles pages up to this truncated index.
The loop clearing migrate.dst[i] = 0 wipes the only references to the
remaining device pages originally allocated by ops->populate_devmem_pfn(),
ensuring they can never be freed. Could we avoid this by unlocking only
the processed pages while still freeing the remaining unprocessed device
pages?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828083652.40855-1-Junhua.Shen@amd.com?part=1

Re: [PATCH] drm/pagemap: centralize migrate src base-page counting

[Francois Dugast]

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 <Junhua.Shen@amd.com>

Consequently, we need a "Fixes" tag here.

With that, the change itself LGTM:

    Reviewed-by: Francois Dugast <francois.dugast@intel.com>

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
> 

Re: [PATCH] drm/pagemap: centralize migrate src base-page counting

[Junhua Shen]

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 <Junhua.Shen@amd.com>
> 
> 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 <francois.dugast@intel.com>
> 
> 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
> > 

[PATCH v2] drm/pagemap: fix page undercount for compound src entries

[Junhua Shen]

A migrate.src[] entry with MIGRATE_PFN_VALID=0 && MIGRATE_PFN_COMPOUND=1
represents a compound (THP) source page, but migrate_pfn_to_page() returns
NULL for it. Both accounting loops in drm_pagemap_migrate_to_devmem()
derive the count from that NULL page and fall back to nr_pages = 1 instead
of HPAGE_PMD_NR, undercounting migrated_pages and risking a spurious
-EBUSY in the final migration race check. drm_pagemap_cpages() already
handles this case, so the open-coded sites disagree.

Factor the decoding 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 a COMPOUND-only entry is consistently
counted as HPAGE_PMD_NR base pages.

Fixes: 192cb1f5cb16 ("drm/pagemap: Enable THP support for GPU memory migration")
Signed-off-by: Junhua Shen <Junhua.Shen@amd.com>
Reviewed-by: Francois Dugast <francois.dugast@intel.com>
---
v2:
- Reframe as a bug fix: a MIGRATE_PFN_VALID=0 && MIGRATE_PFN_COMPOUND=1
  entry was undercounted as 1 base page instead of HPAGE_PMD_NR, risking a
  spurious -EBUSY. Rewrite the commit message accordingly and add a Fixes
  tag.
- Add Reviewed-by: Francois Dugast.
- No functional change to the diff since v1.

 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

Re: [PATCH v2] drm/pagemap: fix page undercount for compound src entries

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] drm_pagemap_src_pfn_nr_pages relies on folio_order for valid pages, causing severe overcounting and skipped entries for PTE-mapped THPs, which masks migration failures.
--

commit bb0a5adb569e2e78382923dc4aeb9bb445b55021
Author: Junhua Shen <Junhua.Shen@amd.com>

drm/pagemap: fix page undercount for compound src entries

This commit factors out the page count decoding into a new helper function, 
drm_pagemap_src_pfn_nr_pages(), to consistently handle compound source entries. 
It ensures that entries with only MIGRATE_PFN_COMPOUND are correctly counted 
as HPAGE_PMD_NR base pages in both accounting loops of 
drm_pagemap_migrate_to_devmem(). This prevents a potential spurious -EBUSY 
in the final migration race check due to undercounting.

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 892b325fa99b8..a31cc4f0af68c 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,
> +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)));

[Severity: High]
This is a pre-existing issue, but by factoring out this logic, the helper
perpetuates a flaw for PTE-mapped large folios (THPs). For a valid page
that is part of a PTE-mapped THP, this derives the step size from the folio
order rather than the migration entry flags.

Since migrate_vma tracks PTE-mapped THPs as independent order-0 entries
(each with MIGRATE_PFN_VALID but not MIGRATE_PFN_COMPOUND), will returning
the full large folio size here cause severe overcounting for these entries?

> +	} else if (src_pfn & MIGRATE_PFN_COMPOUND) {
> +		nr_pages = HPAGE_PMD_NR;
> +	}
> +
> +	if (src_page)
> +		*src_page = page;
> +
> +	return nr_pages;
> +}

[ ... ]

> @@ -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;

[Severity: High]
This isn't a bug introduced by this patch, but because
drm_pagemap_src_pfn_nr_pages() returns the full folio order for PTE-mapped
THPs, won't this final verification loop in drm_pagemap_migrate_to_devmem()
step by 512 instead of 1?

If it steps by 512 for a PTE-mapped THP, it skips checking the
MIGRATE_PFN_MIGRATE flag for the remaining independent tail pages. If any
tail page failed to migrate, does this silently mask the migration failure
and falsely report complete success, risking data corruption and permanently
desynchronized CPU/GPU page tables?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831060743.70024-1-Junhua.Shen@amd.com?part=1