[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
>