Re: [PATCH v2 02/29] mm/migrate: Add migrate_device_prepopulated_range

[Alistair Popple <apopple@nvidia.com>]

Matthew Brost <matthew.brost@intel.com> 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.

> v2:
>  - s/migrate_device_vma_range/migrate_device_prepopulated_range
>  - Drop extra mmu invalidation (Vetter)
>
> 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     | 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()?

> +{
> +	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.

> +	}
> +
> +	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