On Wed, Oct 16, 2024 at 03:04:06PM +1100, Alistair Popple wrote:
>
> Matthew Brost <matthew.brost@xxxxxxxxx> writes:
>
> > Add migrate_device_prepoluated_range which prepares an array of
> > pre-populated device pages for migration.
>
> It would be nice if the commit message could also include an explanation
> of why the existing migrate_device_range() is inadequate for your needs.
>
Yea, my bad. It should explain this is required for non-contiguous
device pages. I suppose I could always iterate for contiguous regions
with migrate_device_range too if you think that is better.
> > v2:
> > - s/migrate_device_vma_range/migrate_device_prepopulated_range
> > - Drop extra mmu invalidation (Vetter)
> >
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> > ---
> > include/linux/migrate.h | 1 +
> > mm/migrate_device.c | 35 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 36 insertions(+)
> >
> > diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> > index 002e49b2ebd9..9146ed39a2a3 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_prepopulated_range(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..f163c2131022 100644
> > --- a/mm/migrate_device.c
> > +++ b/mm/migrate_device.c
> > @@ -924,6 +924,41 @@ int migrate_device_range(unsigned long *src_pfns, unsigned long start,
> > }
> > EXPORT_SYMBOL(migrate_device_range);
> >
> > +/**
> > + * migrate_device_prepopulated_range() - 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_prepopulated_range(unsigned long *src_pfns, unsigned long npages)
>
> I don't love the name, I think because it is quite long and makes me
> think of something complicated like prefaulting. Perhaps
> migrate_device_pfns()?
>
Sure.
> > +{
> > + unsigned long i;
> > +
> > + for (i = 0; i < npages; i++) {
> > + struct page *page = pfn_to_page(src_pfns[i]);
> > +
> > + if (!get_page_unless_zero(page)) {
> > + src_pfns[i] = 0;
> > + continue;
> > + }
> > +
> > + if (!trylock_page(page)) {
> > + src_pfns[i] = 0;
> > + put_page(page);
> > + continue;
> > + }
> > +
> > + src_pfns[i] = migrate_pfn(src_pfns[i]) | MIGRATE_PFN_MIGRATE;
>
> This needs to be converted to use a folio like
> migrate_device_range(). But more importantly this should be split out as
> a function that both migrate_device_range() and this function can call
> given this bit is identical.
>
Missed the folio conversion and agree a helper shared between this
function and migrate_device_range would be a good idea. Let add that.
Matt
> > + }
> > +
> > + migrate_device_unmap(src_pfns, npages, NULL);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(migrate_device_prepopulated_range);
> > +
> > /*
> > * Migrate a device coherent folio back to normal memory. The caller should have
> > * a reference on folio which will be copied to the new folio if migration is
>
