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

[sashiko-bot@kernel.org]

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