Re: [PATCH v4 02/33] mm/migrate: Add migrate_device_pfns

[Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>]

On 1/29/25 9:51 PM, Matthew Brost wrote:
> Add migrate_device_pfns which prepares an array of pre-populated device
> pages for migration. This is needed for eviction of known set of
> non-contiguous devices pages to cpu pages which is a common case for SVM
> in DRM drivers using TTM.
> 
> v2:
>   - s/migrate_device_vma_range/migrate_device_prepopulated_range
>   - Drop extra mmu invalidation (Vetter)
> v3:
>   - s/migrate_device_prepopulated_range/migrate_device_pfns (Alistar)
>   - Use helper to lock device pages (Alistar)
>   - Update commit message with why this is required (Alistar)
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> ---
>   include/linux/migrate.h |  1 +
>   mm/migrate_device.c     | 52 +++++++++++++++++++++++++++++------------
>   2 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index 002e49b2ebd9..6254746648cc 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -229,6 +229,7 @@ void migrate_vma_pages(struct migrate_vma *migrate);
>   void migrate_vma_finalize(struct migrate_vma *migrate);
>   int migrate_device_range(unsigned long *src_pfns, unsigned long start,
>   			unsigned long npages);
> +int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages);
>   void migrate_device_pages(unsigned long *src_pfns, unsigned long *dst_pfns,
>   			unsigned long npages);
>   void migrate_device_finalize(unsigned long *src_pfns,
> diff --git a/mm/migrate_device.c b/mm/migrate_device.c
> index 9cf26592ac93..19960743f927 100644
> --- a/mm/migrate_device.c
> +++ b/mm/migrate_device.c
> @@ -876,6 +876,22 @@ void migrate_vma_finalize(struct migrate_vma *migrate)
>   }
>   EXPORT_SYMBOL(migrate_vma_finalize);
>   
> +static unsigned long migrate_device_pfn_lock(unsigned long pfn)
> +{
> +	struct folio *folio;
> +
> +	folio = folio_get_nontail_page(pfn_to_page(pfn));
> +	if (!folio)
> +		return 0;
> +
> +	if (!folio_trylock(folio)) {
> +		folio_put(folio);
> +		return 0;
> +	}
> +
> +	return migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
> +}
> +
>   /**
>    * migrate_device_range() - migrate device private pfns to normal memory.
>    * @src_pfns: array large enough to hold migrating source device private pfns.
> @@ -900,29 +916,35 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
>   {
>   	unsigned long i, pfn;
>   
> -	for (pfn = start, i = 0; i < npages; pfn++, i++) {
> -		struct folio *folio;
> +	for (pfn = start, i = 0; i < npages; pfn++, i++)
> +		src_pfns[i] = migrate_device_pfn_lock(pfn);
>   
> -		folio = folio_get_nontail_page(pfn_to_page(pfn));
> -		if (!folio) {
> -			src_pfns[i] = 0;
> -			continue;
> -		}
> +	migrate_device_unmap(src_pfns, npages, NULL);
>   
> -		if (!folio_trylock(folio)) {
> -			src_pfns[i] = 0;
> -			folio_put(folio);
> -			continue;
> -		}
> +	return 0;
> +}
> +EXPORT_SYMBOL(migrate_device_range);
>   
> -		src_pfns[i] = migrate_pfn(pfn) | MIGRATE_PFN_MIGRATE;
> -	}
> +/**
> + * migrate_device_pfns() - migrate device private pfns to normal memory.
> + * @src_pfns: pre-popluated array of source device private pfns to migrate.
> + * @npages: number of pages to migrate.
> + *
> + * Similar to migrate_device_range() but supports non-contiguous pre-popluated
> + * array of device pages to migrate.
> + */
> +int migrate_device_pfns(unsigned long *src_pfns, unsigned long npages)
> +{
> +	unsigned long i;
> +
> +	for (i = 0; i < npages; i++)
> +		src_pfns[i] = migrate_device_pfn_lock(src_pfns[i]);
>   
>   	migrate_device_unmap(src_pfns, npages, NULL);
>   
>   	return 0;
>   }
> -EXPORT_SYMBOL(migrate_device_range);
> +EXPORT_SYMBOL(migrate_device_pfns);
>   
>   /*
>    * Migrate a device coherent folio back to normal memory. The caller should have
Looks good to me and I have confirmed that a code flow has been added 
that calls this function from the actual driver (xe),
which did not exist in the previous v3 patch series.
(This code flow called when ttm performs eviction when ttm needs)

There seems to be no test scenario for testing device memory pressure in 
the test cases mentioned in the cover letter.
Do you plan to add this scenario as well? ( Please correct me if I 
misunderstood.)

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>