search for: hmm_range_unregister

Checking range->valid is trivial and has no meaningful cost, but nicely simplifies the fastpath in typical callers. Also remove the hmm_vma_range_done function, which now is a trivial wrapper around hmm_range_unregister. Signed-off-by: Christoph Hellwig <hch at lst.de> --- drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- include/linux/hmm.h | 11 +---------- mm/hmm.c | 6 +++++- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/nou...

Checking range->valid is trivial and has no meaningful cost, but nicely simplifies the fastpath in typical callers. Also remove the hmm_vma_range_done function, which now is a trivial wrapper around hmm_range_unregister. Signed-off-by: Christoph Hellwig <hch at lst.de> Reviewed-by: Ralph Campbell <rcampbell at nvidia.com> --- drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- include/linux/hmm.h | 11 +---------- mm/hmm.c | 7 ++++++- 3 files changed, 8 ins...

...2019 at 11:44:58AM -0700, Christoph Hellwig wrote: > Checking range->valid is trivial and has no meaningful cost, but > nicely simplifies the fastpath in typical callers. It should not be the typical caller.. > hmm_vma_range_done function, which now is a trivial wrapper around > hmm_range_unregister. > > Signed-off-by: Christoph Hellwig <hch at lst.de> > Reviewed-by: Ralph Campbell <rcampbell at nvidia.com> > drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- > include/linux/hmm.h | 11 +---------- > mm/hmm.c | 7 +++++...

Hi Jérôme, Ben and Jason, below is a series against the hmm tree which fixes up the mmap_sem locking in nouveau and while at it also removes leftover legacy HMM APIs only used by nouveau.

...ue); > if (ret <= 0) { > - if (ret == -EBUSY || !ret) { > - up_read(&range->vma->vm_mm->mmap_sem); > - ret = -EBUSY; > - } else if (ret == -EAGAIN) > + if (ret == 0) > ret = -EBUSY; > + up_read(&range->vma->vm_mm->mmap_sem); > hmm_range_unregister(range); > return ret; Hum.. The caller does this: again: ret = nouveau_range_fault(&svmm->mirror, &range); if (ret == 0) { mutex_lock(&svmm->mutex); if (!nouveau_range_done(&range)) { mutex_unlock(&svmm->mutex); goto again; And we can't c...

On Wed, Jul 03, 2019 at 07:00:50PM +0000, Jason Gunthorpe wrote: > I don't think the API should be encouraging some shortcut here.. > > We can't do the above pattern because the old hmm_vma API didn't allow > it, which is presumably a reason why it is obsolete. > > I'd rather see drivers move to a consistent pattern so we can then > easily hoist the seqcount

...39; *should* be possible even if range_fault failed. But that is not for this patch.. > > ret = hmm_range_fault(range, true); > > if (ret <= 0) { > > if (ret == 0) > > ret = -EBUSY; > > - up_read(&range->vma->vm_mm->mmap_sem); > > hmm_range_unregister(range); > > This would hold mmap_sem over hmm_range_unregister, which can lead > to deadlock if we call exit_mmap and then acquire mmap_sem again. That reminds me, this code is also leaking hmm_range_unregister() in the success path, right? I think the right way to structure this is to...

Hi Jérôme, Ben and Jason, below is a series against the hmm tree which fixes up the mmap_sem locking in nouveau and while at it also removes leftover legacy HMM APIs only used by nouveau. Changes since v1: - don't return the valid state from hmm_range_unregister - additional nouveau cleanups

...also removes leftover legacy HMM APIs only used by nouveau. The first 4 patches are a bug fix for nouveau, which I suspect should go into this merge window even if the code is marked as staging, just to avoid people copying the breakage. Changes since v1: - don't return the valid state from hmm_range_unregister - additional nouveau cleanups

...veau/nouveau_svm.c +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c @@ -475,6 +475,47 @@ nouveau_svm_fault_cache(struct nouveau_svm *svm, fault->inst, fault->addr, fault->access); } +static inline bool nouveau_range_done(struct hmm_range *range) +{ + bool ret = hmm_range_valid(range); + + hmm_range_unregister(range); + return ret; +} + +static int +nouveau_range_fault(struct hmm_mirror *mirror, struct hmm_range *range, + bool block) +{ + long ret; + + range->default_flags = 0; + range->pfn_flags_mask = -1UL; + + ret = hmm_range_register(range, mirror, + range->start, range->end, +...

...nouveau_svm { struct nouveau_drm *drm; struct mutex mutex; @@ -475,6 +482,47 @@ nouveau_svm_fault_cache(struct nouveau_svm *svm, fault->inst, fault->addr, fault->access); } +static inline bool nouveau_range_done(struct hmm_range *range) +{ + bool ret = hmm_range_valid(range); + + hmm_range_unregister(range); + return ret; +} + +static int +nouveau_range_fault(struct hmm_mirror *mirror, struct hmm_range *range, + bool block) +{ + long ret; + + range->default_flags = 0; + range->pfn_flags_mask = -1UL; + + ret = hmm_range_register(range, mirror, + range->start, range->end, +...

...merge window even if the code is marked as staging, just to avoid people copying the breakage. Changes since v2: - new patch from Jason to document FAULT_FLAG_ALLOW_RETRY semantics better - remove -EAGAIN handling in nouveau earlier Changes since v1: - don't return the valid state from hmm_range_unregister - additional nouveau cleanups

Hi Dan, Jérôme and Jason, below is a series that cleans up the dev_pagemap interface so that it is more easily usable, which removes the need to wrap it in hmm and thus allowing to kill a lot of code Note: this series is on top of Linux 5.2-rc6 and has some minor conflicts with the hmm tree that are easy to resolve. Diffstat summary: 34 files changed, 379 insertions(+), 1016 deletions(-) Git

...+ return -EAGAIN; + } + + ret = hmm_range_fault(range, block); + if (ret <= 0) { + if (ret == -EBUSY || !ret) { + /* Same as above, drop mmap_sem to match old API. */ + up_read(&range->vma->vm_mm->mmap_sem); + ret = -EBUSY; + } else if (ret == -EAGAIN) + ret = -EBUSY; + hmm_range_unregister(range); + return ret; + } + return 0; +} + static int nouveau_svm_fault(struct nvif_notify *notify) { @@ -649,7 +701,7 @@ nouveau_svm_fault(struct nvif_notify *notify) range.values = nouveau_svm_pfn_values; range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT; again: - ret = hmm_vma_fault(&a...

...nge_fault(range, block); > + if (ret <= 0) { > + if (ret == -EBUSY || !ret) { > + /* Same as above, drop mmap_sem to match old API. */ > + up_read(&range->vma->vm_mm->mmap_sem); > + ret = -EBUSY; > + } else if (ret == -EAGAIN) > + ret = -EBUSY; > + hmm_range_unregister(range); > + return ret; > + } > + return 0; > +} > + > static int > nouveau_svm_fault(struct nvif_notify *notify) > { > @@ -649,7 +701,7 @@ nouveau_svm_fault(struct nvif_notify *notify) > range.values = nouveau_svm_pfn_values; > range.pfn_shift = NVIF...

...- /* - * No need to check hmm_range_wait_until_valid() return value - * on retry we will get proper error with hmm_range_fault() - */ - hmm_range_wait_until_valid(&range, TIMEOUT_IN_MSEC); - goto again; - } - hmm_range_unregister(&range); + if (ret == -EBUSY) + goto again; return ret; } + up_read(&mm->mmap_sem); + take_lock(driver->update); - if (!hmm_range_valid(&range)) { + if (mmu_interval_read_retry(&ni, range.notifier_seq) {...

...Ben and Jason, >> >> below is a series against the hmm tree which fixes up the mmap_sem >> locking in nouveau and while at it also removes leftover legacy HMM APIs >> only used by nouveau. >> >> Changes since v1: >> - don't return the valid state from hmm_range_unregister >> - additional nouveau cleanups > > Ralph, since most of this is nouveau could you contribute a > Tested-by? Thanks > > Jason > I can test things fairly easily but with all the different patches, conflicts, and personal git trees, can you specify the git tree and bran...

...t. Goto again can only happen if nouveau_range_fault was successful, in which case we did not drop mmap_sem. Also: > ret = hmm_range_fault(range, true); > if (ret <= 0) { > if (ret == 0) > ret = -EBUSY; > - up_read(&range->vma->vm_mm->mmap_sem); > hmm_range_unregister(range); This would hold mmap_sem over hmm_range_unregister, which can lead to deadlock if we call exit_mmap and then acquire mmap_sem again.

..._MIRROR int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm); void hmm_mirror_unregister(struct hmm_mirror *mirror); @@ -383,14 +390,6 @@ void hmm_mirror_unregister(struct hmm_mirror *mirror); int hmm_range_register(struct hmm_range *range, struct hmm_mirror *mirror); void hmm_range_unregister(struct hmm_range *range); -/* - * Retry fault if non-blocking, drop mmap_sem and return -EAGAIN in that case. - */ -#define HMM_FAULT_ALLOW_RETRY (1 << 0) - -/* Don't fault in missing PTEs, just snapshot the current state. */ -#define HMM_FAULT_SNAPSHOT (1 << 1) - long hmm_rang...

...*/ struct hmm_range { + struct mmu_interval_notifier *notifier; + unsigned long notifier_seq; struct hmm *hmm; struct list_head list; unsigned long start; diff --git a/mm/hmm.c b/mm/hmm.c index 6b0136665407a3..8d060c5dabe37b 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -858,6 +858,14 @@ void hmm_range_unregister(struct hmm_range *range) } EXPORT_SYMBOL(hmm_range_unregister); +static bool needs_retry(struct hmm_range *range) +{ + if (range->notifier) + return mmu_interval_check_retry(range->notifier, + range->notifier_seq); + return !range->valid; +} + static const struct mm_walk_ops...