From: Jason Gunthorpe jgg@mellanox.com
The API is a bit complicated for the uses we actually have, and disucssions for simplifying have come up a number of times.
This small series removes the customizable pfn format and simplifies the return code of hmm_range_fault()
All the drivers are adjusted to process in the simplified format. I would appreciated tested-by's for the two drivers, thanks!
This passes the hmm tester with the following diff:
diff --git a/lib/test_hmm.c b/lib/test_hmm.c index d75e18f2ffd245..a2442efa038c41 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -47,23 +47,8 @@ struct dmirror_bounce { unsigned long cpages; };
-#define DPT_SHIFT PAGE_SHIFT -#define DPT_VALID (1UL << 0) -#define DPT_WRITE (1UL << 1) - #define DPT_XA_TAG_WRITE 3UL
-static const uint64_t dmirror_hmm_flags[HMM_PFN_FLAG_MAX] = { - [HMM_PFN_VALID] = DPT_VALID, - [HMM_PFN_WRITE] = DPT_WRITE, -}; - -static const uint64_t dmirror_hmm_values[HMM_PFN_VALUE_MAX] = { - [HMM_PFN_NONE] = 0, - [HMM_PFN_ERROR] = 0x10, - [HMM_PFN_SPECIAL] = 0x10, -}; - /* * Data structure to track address ranges and register for mmu interval * notifier updates. @@ -175,7 +160,7 @@ static inline struct dmirror_device *dmirror_page_to_device(struct page *page)
static int dmirror_do_fault(struct dmirror *dmirror, struct hmm_range *range) { - uint64_t *pfns = range->pfns; + unsigned long *pfns = range->hmm_pfns; unsigned long pfn;
for (pfn = (range->start >> PAGE_SHIFT); @@ -188,15 +173,16 @@ static int dmirror_do_fault(struct dmirror *dmirror, struct hmm_range *range) * Since we asked for hmm_range_fault() to populate pages, * it shouldn't return an error entry on success. */ - WARN_ON(*pfns == range->values[HMM_PFN_ERROR]); + WARN_ON(*pfns & HMM_PFN_ERROR); + WARN_ON(!(*pfns & HMM_PFN_VALID));
- page = hmm_device_entry_to_page(range, *pfns); + page = hmm_pfn_to_page(*pfns); WARN_ON(!page);
entry = page; - if (*pfns & range->flags[HMM_PFN_WRITE]) + if (*pfns & HMM_PFN_WRITE) entry = xa_tag_pointer(entry, DPT_XA_TAG_WRITE); - else if (range->default_flags & range->flags[HMM_PFN_WRITE]) + else if (WARN_ON(range->default_flags & HMM_PFN_WRITE)) return -EFAULT; entry = xa_store(&dmirror->pt, pfn, entry, GFP_ATOMIC); if (xa_is_err(entry)) @@ -260,8 +246,6 @@ static int dmirror_range_fault(struct dmirror *dmirror, int ret;
while (true) { - long count; - if (time_after(jiffies, timeout)) { ret = -EBUSY; goto out; @@ -269,12 +253,11 @@ static int dmirror_range_fault(struct dmirror *dmirror,
range->notifier_seq = mmu_interval_read_begin(range->notifier); down_read(&mm->mmap_sem); - count = hmm_range_fault(range); + ret = hmm_range_fault(range); up_read(&mm->mmap_sem); - if (count <= 0) { - if (count == 0 || count == -EBUSY) + if (ret) { + if (ret == -EBUSY) continue; - ret = count; goto out; }
@@ -299,16 +282,13 @@ static int dmirror_fault(struct dmirror *dmirror, unsigned long start, { struct mm_struct *mm = dmirror->notifier.mm; unsigned long addr; - uint64_t pfns[64]; + unsigned long pfns[64]; struct hmm_range range = { .notifier = &dmirror->notifier, - .pfns = pfns, - .flags = dmirror_hmm_flags, - .values = dmirror_hmm_values, - .pfn_shift = DPT_SHIFT, + .hmm_pfns = pfns, .pfn_flags_mask = 0, - .default_flags = dmirror_hmm_flags[HMM_PFN_VALID] | - (write ? dmirror_hmm_flags[HMM_PFN_WRITE] : 0), + .default_flags = + HMM_PFN_REQ_FAULT | (write ? HMM_PFN_REQ_WRITE : 0), .dev_private_owner = dmirror->mdevice, }; int ret = 0; @@ -754,19 +734,20 @@ static int dmirror_migrate(struct dmirror *dmirror, }
static void dmirror_mkentry(struct dmirror *dmirror, struct hmm_range *range, - unsigned char *perm, uint64_t entry) + unsigned char *perm, unsigned long entry) { struct page *page;
- if (entry == range->values[HMM_PFN_ERROR]) { + if (entry & HMM_PFN_ERROR) { *perm = HMM_DMIRROR_PROT_ERROR; return; } - page = hmm_device_entry_to_page(range, entry); - if (!page) { + if (!(entry & HMM_PFN_VALID)) { *perm = HMM_DMIRROR_PROT_NONE; return; } + + page = hmm_pfn_to_page(entry); if (is_device_private_page(page)) { /* Is the page migrated to this device or some other? */ if (dmirror->mdevice == dmirror_page_to_device(page)) @@ -777,7 +758,7 @@ static void dmirror_mkentry(struct dmirror *dmirror, struct hmm_range *range, *perm = HMM_DMIRROR_PROT_ZERO; else *perm = HMM_DMIRROR_PROT_NONE; - if (entry & range->flags[HMM_PFN_WRITE]) + if (entry & HMM_PFN_WRITE) *perm |= HMM_DMIRROR_PROT_WRITE; else *perm |= HMM_DMIRROR_PROT_READ; @@ -832,8 +813,6 @@ static int dmirror_range_snapshot(struct dmirror *dmirror, return ret;
while (true) { - long count; - if (time_after(jiffies, timeout)) { ret = -EBUSY; goto out; @@ -842,12 +821,11 @@ static int dmirror_range_snapshot(struct dmirror *dmirror, range->notifier_seq = mmu_interval_read_begin(range->notifier);
down_read(&mm->mmap_sem); - count = hmm_range_fault(range); + ret = hmm_range_fault(range); up_read(&mm->mmap_sem); - if (count <= 0) { - if (count == 0 || count == -EBUSY) + if (ret) { + if (ret == -EBUSY) continue; - ret = count; goto out; }
@@ -862,7 +840,7 @@ static int dmirror_range_snapshot(struct dmirror *dmirror,
n = (range->end - range->start) >> PAGE_SHIFT; for (i = 0; i < n; i++) - dmirror_mkentry(dmirror, range, perm + i, range->pfns[i]); + dmirror_mkentry(dmirror, range, perm + i, range->hmm_pfns[i]);
mutex_unlock(&dmirror->mutex); out: @@ -878,15 +856,11 @@ static int dmirror_snapshot(struct dmirror *dmirror, unsigned long size = cmd->npages << PAGE_SHIFT; unsigned long addr; unsigned long next; - uint64_t pfns[64]; + unsigned long pfns[64]; unsigned char perm[64]; char __user *uptr; struct hmm_range range = { - .pfns = pfns, - .flags = dmirror_hmm_flags, - .values = dmirror_hmm_values, - .pfn_shift = DPT_SHIFT, - .pfn_flags_mask = 0, + .hmm_pfns = pfns, .dev_private_owner = dmirror->mdevice, }; int ret = 0; @@ -1097,6 +1071,7 @@ static int dmirror_device_init(struct dmirror_device *mdevice, int id) spin_lock_init(&mdevice->lock);
cdev_init(&mdevice->cdevice, &dmirror_fops); + mdevice->cdevice.owner = THIS_MODULE; ret = cdev_add(&mdevice->cdevice, dev, 1); if (ret) return ret; diff --git a/tools/testing/selftests/vm/hmm-tests.c b/tools/testing/selftests/vm/hmm-tests.c index 033a12c7ab5b6d..da15471a2bbf9a 100644 --- a/tools/testing/selftests/vm/hmm-tests.c +++ b/tools/testing/selftests/vm/hmm-tests.c @@ -1274,7 +1274,7 @@ TEST_F(hmm2, snapshot) /* Check what the device saw. */ m = buffer->mirror; ASSERT_EQ(m[0], HMM_DMIRROR_PROT_ERROR); - ASSERT_EQ(m[1], HMM_DMIRROR_PROT_NONE); + ASSERT_EQ(m[1], HMM_DMIRROR_PROT_ERROR); ASSERT_EQ(m[2], HMM_DMIRROR_PROT_ZERO | HMM_DMIRROR_PROT_READ); ASSERT_EQ(m[3], HMM_DMIRROR_PROT_READ); ASSERT_EQ(m[4], HMM_DMIRROR_PROT_WRITE);
Jason Gunthorpe (5): mm/hmm: make CONFIG_DEVICE_PRIVATE into a select mm/hmm: make hmm_range_fault return 0 or -1 drm/amdgpu: remove dead code after hmm_range_fault() mm/hmm: remove HMM_PFN_SPECIAL mm/hmm: remove the customizable pfn format from hmm_range_fault
Documentation/vm/hmm.rst | 28 ++-- arch/powerpc/Kconfig | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 56 +++---- drivers/gpu/drm/nouveau/Kconfig | 2 +- drivers/gpu/drm/nouveau/nouveau_dmem.c | 60 ++++++-- drivers/gpu/drm/nouveau/nouveau_dmem.h | 4 +- drivers/gpu/drm/nouveau/nouveau_svm.c | 59 ++++---- include/linux/hmm.h | 109 +++++--------- mm/Kconfig | 7 +- mm/hmm.c | 185 +++++++++++------------- 10 files changed, 229 insertions(+), 283 deletions(-)