Re: [PATCH v3] drm/pagemap: Prevent double migration of device pages

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect PFN array iteration skips unrelated folios when encountering tail pages.
--

--- Patch [1]: [PATCH v3] drm/pagemap: Prevent double migration of device pages ---
commit 8b99d73a2bacb652ddc2b438316d6f18c0ebfb5e
Author: Arvind Yadav <[email protected]>

drm/pagemap: Prevent double migration of device pages

A device-private folio migrated to system memory by a CPU fault can
remain reachable through the raw-PFN eviction path until migration
finalization drops the source reference.

> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
[ ... ]
> +static void drm_pagemap_retire_migrated_pages(unsigned long *src_pfns,
> +					      unsigned long npages)
> +{
> +	unsigned long i = 0;
> +
> +	while (i < npages) {
> +		struct page *page = migrate_pfn_to_page(src_pfns[i]);
> +		unsigned long nr = 1;
> +
> +		if (!page) {
> +			i++;
> +			continue;
> +		}
> +
> +		nr = folio_nr_pages(page_folio(page));
> +
> +		if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
> +		    is_device_private_page(page))
> +			drm_pagemap_page_set_flags(page,
> +						   DRM_PAGEMAP_ZDD_FLAG_MIGRATED);
> +
> +		i += nr;

[Severity: High]
Does this iteration correctly handle tail pages? If src_pfns[i] points to a
tail page of a large folio (for instance, when a migration batch starts in
the middle of a large device-private folio due to unaligned VMA boundaries
or partial unmaps), folio_nr_pages() returns the total number of pages in
the entire large folio.

Advancing the array index by i += nr will overshoot the remaining pages in
the array, skipping subsequent unrelated folios. This could prevent the
DRM_PAGEMAP_ZDD_FLAG_MIGRATED flag from being set on the skipped folios,
leaving them vulnerable to the exact double-migration soft lockups and RCU
stalls this patch aims to fix.

> +	}
> +}
> +
> +static void drm_pagemap_skip_retired_pages(unsigned long *src_pfns,
> +					   unsigned long npages)
> +{
> +	unsigned long i = 0;
> +
> +	while (i < npages) {
> +		struct page *page = migrate_pfn_to_page(src_pfns[i]);
> +		unsigned long nr = 1;
> +
> +		if (!page) {
> +			i++;
> +			continue;
> +		}
> +
> +		nr = folio_nr_pages(page_folio(page));
> +
> +		if ((src_pfns[i] & MIGRATE_PFN_MIGRATE) &&
> +		    is_device_private_page(page) &&
> +		    (drm_pagemap_page_get_flags(page) &
> +		     DRM_PAGEMAP_ZDD_FLAG_MIGRATED))
> +			src_pfns[i] &= ~MIGRATE_PFN_MIGRATE;
> +
> +		i += nr;

[Severity: High]
Can this loop also skip unrelated folios if the current page is a tail
page? Using folio_nr_pages() to advance the array index from a tail page
has the same overshoot issue as above, potentially failing to clear the
MIGRATE_PFN_MIGRATE bit for migrated folios and leading to a
double-migration attempt.

> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1