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

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Jan 31, 2025 at 09:47:52AM +0200, Gwan-gyeong Mun wrote:
> 
> 
> 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.)
> 

The *evict sections in [1] to test device memory pressure.

Matt

[1] https://patchwork.freedesktop.org/series/137545/#rev3

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