2020-07-21 21:33:22

by Ralph Campbell

[permalink] [raw]
Subject: [PATCH v3 0/5] mm/migrate: avoid device private invalidations

The goal for this series is to avoid device private memory TLB
invalidations when migrating a range of addresses from system
memory to device private memory and some of those pages have already
been migrated. The approach taken is to introduce a new mmu notifier
invalidation event type and use that in the device driver to skip
invalidation callbacks from migrate_vma_setup(). The device driver is
also then expected to handle device MMU invalidations as part of the
migrate_vma_setup(), migrate_vma_pages(), migrate_vma_finalize() process.
Note that this is opt-in. A device driver can simply invalidate its MMU
in the mmu notifier callback and not handle MMU invalidations in the
migration sequence.

This series is based on Jason Gunthorpe's HMM tree (linux-5.8.0-rc4).

Also, this replaces the need for the following two patches I sent:
("mm: fix migrate_vma_setup() src_owner and normal pages")
https://lore.kernel.org/linux-mm/[email protected]
("nouveau: fix mixed normal and device private page migration")
https://lore.kernel.org/lkml/[email protected]

Bharata Rao, let me know if I can add your reviewed-by back since
I made a fair number of changes to this version of the series.

Changes in v3:
Changed the direction field "dir" to a "flags" field and renamed
src_owner to pgmap_owner.
Fixed a locking issue in nouveau for the migration invalidation.
Added a HMM selftest test case to exercise the HMM test driver
invalidation changes.
Removed reviewed-by Bharata B Rao since this version is moderately
changed.

Changes in v2:
Rebase to Jason Gunthorpe's HMM tree.
Added reviewed-by from Bharata B Rao.
Rename the mmu_notifier_range::data field to migrate_pgmap_owner as
suggested by Jason Gunthorpe.

Ralph Campbell (5):
nouveau: fix storing invalid ptes
mm/migrate: add a flags parameter to migrate_vma
mm/notifier: add migration invalidation type
nouveau/svm: use the new migration invalidation
mm/hmm/test: use the new migration invalidation

arch/powerpc/kvm/book3s_hv_uvmem.c | 4 ++-
drivers/gpu/drm/nouveau/nouveau_dmem.c | 19 ++++++++---
drivers/gpu/drm/nouveau/nouveau_svm.c | 21 +++++-------
drivers/gpu/drm/nouveau/nouveau_svm.h | 13 ++++++-
.../drm/nouveau/nvkm/subdev/mmu/vmmgp100.c | 13 ++++---
include/linux/migrate.h | 16 ++++++---
include/linux/mmu_notifier.h | 7 ++++
lib/test_hmm.c | 34 +++++++++++--------
mm/migrate.c | 14 ++++++--
tools/testing/selftests/vm/hmm-tests.c | 18 +++++++---
10 files changed, 112 insertions(+), 47 deletions(-)

--
2.20.1


2020-07-21 21:35:17

by Ralph Campbell

[permalink] [raw]
Subject: [PATCH v3 2/5] mm/migrate: add a flags parameter to migrate_vma

The src_owner field in struct migrate_vma is being used for two purposes,
it acts as a selection filter for which types of pages are to be migrated
and it identifies device private pages owned by the caller. Split this
into separate parameters so the src_owner field can be used just to
identify device private pages owned by the caller of migrate_vma_setup().
Rename the src_owner field to pgmap_owner to reflect it is now used only
to identify which device private pages to migrate.

Signed-off-by: Ralph Campbell <[email protected]>
---
arch/powerpc/kvm/book3s_hv_uvmem.c | 4 +++-
drivers/gpu/drm/nouveau/nouveau_dmem.c | 4 +++-
include/linux/migrate.h | 13 +++++++++----
lib/test_hmm.c | 6 ++++--
mm/migrate.c | 6 ++++--
5 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
index 09d8119024db..6850bd04bcb9 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -400,6 +400,7 @@ kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start,
mig.end = end;
mig.src = &src_pfn;
mig.dst = &dst_pfn;
+ mig.flags = MIGRATE_VMA_SELECT_SYSTEM;

/*
* We come here with mmap_lock write lock held just for
@@ -577,7 +578,8 @@ kvmppc_svm_page_out(struct vm_area_struct *vma, unsigned long start,
mig.end = end;
mig.src = &src_pfn;
mig.dst = &dst_pfn;
- mig.src_owner = &kvmppc_uvmem_pgmap;
+ mig.pgmap_owner = &kvmppc_uvmem_pgmap;
+ mig.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;

mutex_lock(&kvm->arch.uvmem_lock);
/* The requested page is already paged-out, nothing to do */
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index e5c230d9ae24..78b9e3c2a5b3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -182,7 +182,8 @@ static vm_fault_t nouveau_dmem_migrate_to_ram(struct vm_fault *vmf)
.end = vmf->address + PAGE_SIZE,
.src = &src,
.dst = &dst,
- .src_owner = drm->dev,
+ .pgmap_owner = drm->dev,
+ .flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE,
};

/*
@@ -615,6 +616,7 @@ nouveau_dmem_migrate_vma(struct nouveau_drm *drm,
struct migrate_vma args = {
.vma = vma,
.start = start,
+ .flags = MIGRATE_VMA_SELECT_SYSTEM,
};
unsigned long i;
u64 *pfns;
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 3e546cbf03dd..aafec0ca7b41 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -180,6 +180,11 @@ static inline unsigned long migrate_pfn(unsigned long pfn)
return (pfn << MIGRATE_PFN_SHIFT) | MIGRATE_PFN_VALID;
}

+enum migrate_vma_direction {
+ MIGRATE_VMA_SELECT_SYSTEM = (1UL << 0),
+ MIGRATE_VMA_SELECT_DEVICE_PRIVATE = (1UL << 1),
+};
+
struct migrate_vma {
struct vm_area_struct *vma;
/*
@@ -199,11 +204,11 @@ struct migrate_vma {

/*
* Set to the owner value also stored in page->pgmap->owner for
- * migrating out of device private memory. If set only device
- * private pages with this owner are migrated. If not set
- * device private pages are not migrated at all.
+ * migrating out of device private memory. The flags also need to
+ * be set to MIGRATE_VMA_SELECT_DEVICE_PRIVATE.
*/
- void *src_owner;
+ void *pgmap_owner;
+ unsigned long flags;
};

int migrate_vma_setup(struct migrate_vma *args);
diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index 9aa577afc269..74c6ee66ef15 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -702,7 +702,8 @@ static int dmirror_migrate(struct dmirror *dmirror,
args.dst = dst_pfns;
args.start = addr;
args.end = next;
- args.src_owner = NULL;
+ args.pgmap_owner = NULL;
+ args.flags = MIGRATE_VMA_SELECT_SYSTEM;
ret = migrate_vma_setup(&args);
if (ret)
goto out;
@@ -1053,7 +1054,8 @@ static vm_fault_t dmirror_devmem_fault(struct vm_fault *vmf)
args.end = args.start + PAGE_SIZE;
args.src = &src_pfns;
args.dst = &dst_pfns;
- args.src_owner = dmirror->mdevice;
+ args.pgmap_owner = dmirror->mdevice;
+ args.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;

if (migrate_vma_setup(&args))
return VM_FAULT_SIGBUS;
diff --git a/mm/migrate.c b/mm/migrate.c
index f37729673558..e3ea68e3a08b 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2287,7 +2287,9 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
goto next;

page = device_private_entry_to_page(entry);
- if (page->pgmap->owner != migrate->src_owner)
+ if (!(migrate->flags &
+ MIGRATE_VMA_SELECT_DEVICE_PRIVATE) ||
+ page->pgmap->owner != migrate->pgmap_owner)
goto next;

mpfn = migrate_pfn(page_to_pfn(page)) |
@@ -2295,7 +2297,7 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp,
if (is_write_device_private_entry(entry))
mpfn |= MIGRATE_PFN_WRITE;
} else {
- if (migrate->src_owner)
+ if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM))
goto next;
pfn = pte_pfn(pte);
if (is_zero_pfn(pfn)) {
--
2.20.1

2020-07-22 11:54:06

by Bharata B Rao

[permalink] [raw]
Subject: Re: [PATCH v3 2/5] mm/migrate: add a flags parameter to migrate_vma

On Tue, Jul 21, 2020 at 02:31:16PM -0700, Ralph Campbell wrote:
> The src_owner field in struct migrate_vma is being used for two purposes,
> it acts as a selection filter for which types of pages are to be migrated
> and it identifies device private pages owned by the caller. Split this
> into separate parameters so the src_owner field can be used just to
> identify device private pages owned by the caller of migrate_vma_setup().
> Rename the src_owner field to pgmap_owner to reflect it is now used only
> to identify which device private pages to migrate.
>
> Signed-off-by: Ralph Campbell <[email protected]>
> ---
> arch/powerpc/kvm/book3s_hv_uvmem.c | 4 +++-
> drivers/gpu/drm/nouveau/nouveau_dmem.c | 4 +++-
> include/linux/migrate.h | 13 +++++++++----
> lib/test_hmm.c | 6 ++++--
> mm/migrate.c | 6 ++++--
> 5 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
> index 09d8119024db..6850bd04bcb9 100644
> --- a/arch/powerpc/kvm/book3s_hv_uvmem.c
> +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
> @@ -400,6 +400,7 @@ kvmppc_svm_page_in(struct vm_area_struct *vma, unsigned long start,
> mig.end = end;
> mig.src = &src_pfn;
> mig.dst = &dst_pfn;
> + mig.flags = MIGRATE_VMA_SELECT_SYSTEM;
>
> /*
> * We come here with mmap_lock write lock held just for
> @@ -577,7 +578,8 @@ kvmppc_svm_page_out(struct vm_area_struct *vma, unsigned long start,
> mig.end = end;
> mig.src = &src_pfn;
> mig.dst = &dst_pfn;
> - mig.src_owner = &kvmppc_uvmem_pgmap;
> + mig.pgmap_owner = &kvmppc_uvmem_pgmap;
> + mig.flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
>
> mutex_lock(&kvm->arch.uvmem_lock);

For the kvmppc changes above,
Reviewed-by: Bharata B Rao <[email protected]>