search for: amdgpu_ttm_tt_get_user_pages_don

...+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c > @@ -67,21 +67,15 @@ static bool amdgpu_mn_invalidate_gfx(struct mmu_range_notifier *mrn, > struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); > long r; > > - /* > - * FIXME: Must hold some lock shared with > - * amdgpu_ttm_tt_get_user_pages_done() > - */ > - mmu_range_set_seq(mrn, cur_seq); > + mutex_lock(&adev->notifier_lock); > > - /* FIXME: Is this necessary? */ > - if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm, range->start, > - range->end)) > - return true; > + mmu_range_set_seq(mr...

...u/amdgpu_mn.c >> @@ -67,21 +67,15 @@ static bool amdgpu_mn_invalidate_gfx(struct mmu_range_notifier *mrn, >> struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev); >> long r; >> >> - /* >> - * FIXME: Must hold some lock shared with >> - * amdgpu_ttm_tt_get_user_pages_done() >> - */ >> - mmu_range_set_seq(mrn, cur_seq); >> + mutex_lock(&adev->notifier_lock); >> >> - /* FIXME: Is this necessary? */ >> - if (!amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm, range->start, >> - range->end)) >> - return...

...Okay, I wrote it like this: > > if (mmu_notifier_range_blockable(range)) > mutex_lock(&adev->notifier_lock); > else if (!mutex_trylock(&adev->notifier_lock)) > return false; > >>> Also, do you know if notifier_lock is held while calling >>> amdgpu_ttm_tt_get_user_pages_done()? Can we add a 'lock assert held' >>> to amdgpu_ttm_tt_get_user_pages_done()? >> >> gpu side hold notifier_lock but kfd side doesn't. kfd side doesn't check >> amdgpu_ttm_tt_get_user_pages_done/mmu_range_read_retry return value but >> check mem-&gt...

...it up > > > yes, thanks. Okay, I wrote it like this: if (mmu_notifier_range_blockable(range)) mutex_lock(&adev->notifier_lock); else if (!mutex_trylock(&adev->notifier_lock)) return false; > > Also, do you know if notifier_lock is held while calling > > amdgpu_ttm_tt_get_user_pages_done()? Can we add a 'lock assert held' > > to amdgpu_ttm_tt_get_user_pages_done()? > > gpu side hold notifier_lock but kfd side doesn't. kfd side doesn't check > amdgpu_ttm_tt_get_user_pages_done/mmu_range_read_retry return value but > check mem->invalid flag wh...

...; -#endif }; /** @@ -763,7 +761,6 @@ struct amdgpu_ttm_tt { * Calling function must call amdgpu_ttm_tt_userptr_range_done() once and only * once afterwards to stop HMM tracking */ -#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR) #define MAX_RETRY_HMM_RANGE_FAULT 16 @@ -892,7 +889,6 @@ bool amdgpu_ttm_tt_get_user_pages_done(struct ttm_tt *ttm) return r; } -#endif /** * amdgpu_ttm_tt_set_user_pages - Copy pages in, putting old pages as necessary. @@ -970,12 +966,10 @@ static void amdgpu_ttm_tt_unpin_userptr(struct ttm_tt *ttm) sg_free_table(ttm->sg); -#if IS_ENABLED(CONFIG_DRM_AMDGPU_USERPTR) if (...

On Tue, Oct 29, 2019 at 07:22:37PM +0000, Yang, Philip wrote: > Hi Jason, > > I did quick test after merging amd-staging-drm-next with the > mmu_notifier branch, which includes this set changes. The test result > has different failures, app stuck intermittently, GUI no display etc. I > am understanding the changes and will try to figure out the cause. Thanks! I'm not

...rm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > @@ -1738,6 +1738,10 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info, > return ret; > } > > + /* > + * FIXME: Cannot ignore the return code, must hold > + * notifier_lock > + */ > amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm); > > /* Mark the BO as valid unless it was invalidated > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > index 2e53feed40e230..76771f5f0b60ab 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/driver...

.../amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -1738,6 +1738,10 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info, return ret; } + /* + * FIXME: Cannot ignore the return code, must hold + * notifier_lock + */ amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm); /* Mark the BO as valid unless it was invalidated diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 2e53feed40e230..76771f5f0b60ab 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c...

On Wed, Jul 03, 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

...ma 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 lock scheme into some common mmu notifier > code, as discussed. So you don't like the version in amdgpu_ttm_tt_get_user_pages_done in linux-next either? I can remove this and just move hmm_vma_range_done to nouveau instead. Let me know if you have other comments before I resend. Note that I'll probably be offline Thu-Sun this week.

.../amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -1738,6 +1738,10 @@ static int update_invalid_user_pages(struct amdkfd_process_info *process_info, return ret; } + /* + * FIXME: Cannot ignore the return code, must hold + * notifier_lock + */ amdgpu_ttm_tt_get_user_pages_done(bo->tbo.ttm); /* Mark the BO as valid unless it was invalidated diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 82823d9a8ba887..22c989bca7514c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c...

...range; mmput(mm); @@ -877,7 +863,7 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages) out_unlock: up_read(&mm->mmap_sem); out_free_pfns: - kvfree(range->pfns); + kvfree(range->hmm_pfns); out_free_ranges: kfree(range); out: @@ -902,7 +888,7 @@ bool amdgpu_ttm_tt_get_user_pages_done(struct ttm_tt *ttm) DRM_DEBUG_DRIVER("user_pages_done 0x%llx pages 0x%lx\n", gtt->userptr, ttm->num_pages); - WARN_ONCE(!gtt->range || !gtt->range->pfns, + WARN_ONCE(!gtt->range || !gtt->range->hmm_pfns, "No user pages to check\n"); if (gtt...

...range; mmput(mm); @@ -877,7 +863,7 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages) out_unlock: up_read(&mm->mmap_sem); out_free_pfns: - kvfree(range->pfns); + kvfree(range->hmm_pfns); out_free_ranges: kfree(range); out: @@ -902,7 +888,7 @@ bool amdgpu_ttm_tt_get_user_pages_done(struct ttm_tt *ttm) DRM_DEBUG_DRIVER("user_pages_done 0x%llx pages 0x%lx\n", gtt->userptr, ttm->num_pages); - WARN_ONCE(!gtt->range || !gtt->range->pfns, + WARN_ONCE(!gtt->range || !gtt->range->hmm_pfns, "No user pages to check\n"); if (gtt...

...amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages) > out_unlock: > up_read(&mm->mmap_sem); > out_free_pfns: > - kvfree(range->pfns); > + kvfree(range->hmm_pfns); > out_free_ranges: > kfree(range); > out: > @@ -902,7 +888,7 @@ bool amdgpu_ttm_tt_get_user_pages_done(struct ttm_tt *ttm) > DRM_DEBUG_DRIVER("user_pages_done 0x%llx pages 0x%lx\n", > gtt->userptr, ttm->num_pages); > > - WARN_ONCE(!gtt->range || !gtt->range->pfns, > + WARN_ONCE(!gtt->range || !gtt->range->hmm_pfns, > "No user pages...

Hi Jérôme, Ben, Felix and Jason, below is a series against the hmm tree which cleans up various minor bits and allows HMM_MIRROR to be built on all architectures. Diffstat: 11 files changed, 94 insertions(+), 210 deletions(-) A git tree is also available at: git://git.infradead.org/users/hch/misc.git hmm-cleanups.2 Gitweb:

From: Jason Gunthorpe <jgg at 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

From: Jason Gunthorpe <jgg at mellanox.com> 8 of the mmu_notifier using drivers (i915_gem, radeon_mn, umem_odp, hfi1, scif_dma, vhost, gntdev, hmm) drivers are using a common pattern where they only use invalidate_range_start/end and immediately check the invalidating range against some driver data structure to tell if the driver is interested. Half of them use an interval_tree, the others

From: Jason Gunthorpe <jgg at 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

From: Jason Gunthorpe <jgg at mellanox.com> 8 of the mmu_notifier using drivers (i915_gem, radeon_mn, umem_odp, hfi1, scif_dma, vhost, gntdev, hmm) drivers are using a common pattern where they only use invalidate_range_start/end and immediately check the invalidating range against some driver data structure to tell if the driver is interested. Half of them use an interval_tree, the others