diff mbox series

[v3,1/1] mm/migrate: Add migrate_device_pfns

Message ID 20241023233944.1798835-2-matthew.brost@intel.com (mailing list archive)
State New
Headers show
Series mm/migrate: Add migrate_device_pfns | expand

Commit Message

Matthew Brost Oct. 23, 2024, 11:39 p.m. UTC
Implement migrate_device_pfns to prepare an array of PFNs for migration.
Handles non-contiguous ranges of device pages that require migration.

v2:
 - s/migrate_device_vma_range/migrate_device_prepopulated_range
 - Drop extra mmu invalidation (Vetter)
v3:
 - s/migrate_device_prepopulated_range/migrate_device_pfns (Alistair)
 - Use helper to lock device pages (Alistair)

Cc: Cc: Alistair Popple <apopple@nvidia.com>
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(-)

Comments

Andrew Morton Oct. 24, 2024, 1:22 a.m. UTC | #1
On Wed, 23 Oct 2024 16:39:43 -0700 Matthew Brost <matthew.brost@intel.com> wrote:

> Part of series [1]. Sending as individual patch ahead of that series as
> this is a prerequisite for merging.

That's news to me - singleton patches are perfectly OK?

On Wed, 23 Oct 2024 16:39:44 -0700 Matthew Brost <matthew.brost@intel.com> wrote:

> Implement migrate_device_pfns to prepare an array of PFNs for migration.
> Handles non-contiguous ranges of device pages that require migration.

OK, that's "what".  We're more interested in "why".

> +EXPORT_SYMBOL(migrate_device_pfns);

And it's exported to modules, which adds to the significance.

Please fully describe the reasons for proposing this change.
Matthew Brost Oct. 24, 2024, 1:50 a.m. UTC | #2
On Wed, Oct 23, 2024 at 06:22:17PM -0700, Andrew Morton wrote:
> On Wed, 23 Oct 2024 16:39:43 -0700 Matthew Brost <matthew.brost@intel.com> wrote:
> 
> > Part of series [1]. Sending as individual patch ahead of that series as
> > this is a prerequisite for merging.
> 
> That's news to me - singleton patches are perfectly OK?
> 

I've merged a couple of other patches outside of the DRM subsystem for
pending series which we then have picked up in a following kernel
release. If I have this flow wrong, my mistake.

> On Wed, 23 Oct 2024 16:39:44 -0700 Matthew Brost <matthew.brost@intel.com> wrote:
> 
> > Implement migrate_device_pfns to prepare an array of PFNs for migration.
> > Handles non-contiguous ranges of device pages that require migration.
> 
> OK, that's "what".  We're more interested in "why".
> 

Sure can add. The 'why' is:

A non-contiguous allocation of device pages can occur if a device is
under memory pressure within a single driver allocation of device
memory. Additionally, a driver allocation of memory can also be evicted
under memory pressure. Therefore, an interface for migrating a set of
non-contiguous device pages is required.

Matt

> > +EXPORT_SYMBOL(migrate_device_pfns);
> 
> And it's exported to modules, which adds to the significance.
> 
> Please fully describe the reasons for proposing this change.
Andrew Morton Oct. 24, 2024, 2:22 a.m. UTC | #3
On Thu, 24 Oct 2024 01:50:34 +0000 Matthew Brost <matthew.brost@intel.com> wrote:

> > On Wed, 23 Oct 2024 16:39:44 -0700 Matthew Brost <matthew.brost@intel.com> wrote:
> > 
> > > Implement migrate_device_pfns to prepare an array of PFNs for migration.
> > > Handles non-contiguous ranges of device pages that require migration.
> > 
> > OK, that's "what".  We're more interested in "why".
> > 
> 
> Sure can add. The 'why' is:
> 
> A non-contiguous allocation of device pages can occur if a device is
> under memory pressure within a single driver allocation of device
> memory. Additionally, a driver allocation of memory can also be evicted
> under memory pressure. Therefore, an interface for migrating a set of
> non-contiguous device pages is required.

OK, thanks.  But when merging a new interface such as this we like to
see the code which will actually use the interface.  Along with reasons
to believe that the calling code will actually be merged, so we don't
end up with a new interface which has no callers.

Apart from that, I suspect that it makes more sense to merge this via
the DRM tree, alongside the code which uses it.
diff mbox series

Patch

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..950229706485 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 pfns 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