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

[Francois Dugast <francois.dugast@intel.com>]

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
>