From: Honglei Huang <[email protected]> Make amdgpu_vm's eviction lock a read/write semaphore so it can be shared with drm_gpusvm's notifier lock: VM updates and eviction take it for read, MMU notifier invalidation and eviction take it for write.
Pass the held eviction lock through to amdgpu_vm_update_pdes() in the SVM context so callers that already hold it do not try to reacquire it. Fix the eviction_lock argument at this call site: it must be the 3-argument form (adev, vm, eviction_lock) introduced by "recheck range validity after page table allocation" - a stray leftover 4th argument (an old "immediate" bool) was being passed here, which would otherwise fail to build. Suggested-by: Christian König <[email protected]> Suggested-by: Huang Rui <[email protected]> Signed-off-by: Yu Huang <[email protected]> Signed-off-by: Honglei Huang <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c | 37 ++++++++++---- drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c | 51 ++++++++----------- 2 files changed, 49 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c index c7810e6d3035f..5ae8a649644ec 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c @@ -157,10 +157,13 @@ amdgpu_svm_lookup_by_pasid(struct amdgpu_device *adev, uint32_t pasid) amdgpu_pasid_lock(&irqflags); fpriv = amdgpu_pasid_get_fpriv_locked(pasid); vm = fpriv ? &fpriv->vm : NULL; - if (vm && vm->svm) { - svm = vm->svm; + /* + * eviction_lock changes when SVM is enabled, so acquire vm->svm + * after the new lock alias is published. + */ + svm = vm ? smp_load_acquire(&vm->svm) : NULL; + if (svm) kref_get(&svm->refcount); - } amdgpu_pasid_unlock(irqflags); return svm; @@ -501,6 +504,8 @@ static int amdgpu_svm_init_compute(struct amdgpu_device *adev, struct amdgpu_svm *svm; int ret; + dma_resv_assert_held(vm->root.bo->tbo.base.resv); + if (vm->svm) return 0; @@ -535,7 +540,9 @@ static int amdgpu_svm_init_compute(struct amdgpu_device *adev, 1UL << (svm->default_granularity + PAGE_SHIFT), svm->xnack_enabled ? "enabled" : "disabled"); - vm->svm = svm; + vm->eviction_lock = &svm->gpusvm.notifier_lock; + /* Publish the context after its notifier lock alias. */ + smp_store_release(&vm->svm, svm); return 0; err_free: @@ -611,31 +618,43 @@ void amdgpu_svm_close(struct amdgpu_vm *vm) * amdgpu_svm_fini() - Finalize and release a VM's SVM context * @vm: The VM whose SVM context is being torn down. * - * Close the context, tear down the embedded drm_gpusvm under the SVM lock, - * destroy the attribute tree and work queues, and drop the context - * reference. Safe to call on a VM without an SVM context. + * Tear down the embedded drm_gpusvm, attribute tree and work queues, and drop + * the context reference. */ void amdgpu_svm_fini(struct amdgpu_vm *vm) { struct amdgpu_svm *svm = vm->svm; + struct amdgpu_bo *root; if (!svm) return; amdgpu_svm_close(vm); amdgpu_svm_lock(svm); + + root = amdgpu_bo_ref(vm->root.bo); + amdgpu_bo_reserve(root, true); + drm_gpusvm_fini(&svm->gpusvm); + + vm->eviction_lock = &vm->default_eviction_lock; + /* Publish the lock reset before making SVM unavailable. */ + smp_store_release(&vm->svm, NULL); + amdgpu_svm_unlock(svm); amdgpu_svm_attr_tree_destroy(svm->attr_tree); amdgpu_svm_work_fini(svm); - vm->svm = NULL; amdgpu_svm_put(svm); + + amdgpu_bo_unreserve(root); + amdgpu_bo_unref(&root); } bool amdgpu_svm_is_enabled(struct amdgpu_vm *vm) { - return vm->svm != NULL; + /* Pairs with publishing vm->svm after its lock alias is initialized. */ + return smp_load_acquire(&vm->svm) != NULL; } static int amdgpu_svm_copy_attrs(const struct drm_amdgpu_gem_svm *args, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c index a1aa84974a43a..787afe0767e5a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c @@ -65,16 +65,14 @@ amdgpu_svm_range_zap_ptes(struct amdgpu_svm *svm, unsigned long last_page) { struct dma_fence *fence = NULL; - unsigned int flags; int ret; if (last_page < start_page) return 0; - flags = memalloc_noreclaim_save(); ret = amdgpu_vm_unmap_range(svm->adev, svm->vm, true, NULL, - start_page, last_page, NULL, &fence); - memalloc_noreclaim_restore(flags); + start_page, last_page, + svm->vm->eviction_lock, &fence); if (fence) { dma_fence_wait(fence, false); @@ -210,20 +208,11 @@ amdgpu_svm_range_update_gpu_range(struct amdgpu_svm *svm, mapped_pages += seg_pages; is_last_seg = mapped_pages == npages; - /* - * The struct hmm_range hmm_range field inside amdgpu_hmm_range - * is drm_gpusvm internal state. It is not exposed to the - * implementing driver and is managed entirely by the framework - * as part of the framework logic. This amdgpu_vm_map_range call - * always runs inside the drm_gpusvm notifier, so omitting - * hmm_range here is safe. - */ - ret = amdgpu_vm_map_range(svm->adev, svm->vm, - flush_tlb && is_last_seg, true, NULL, - start_page, last_page, pte_flags, - 0, entry->addr, NULL, NULL, NULL, - NULL, NULL, - wait_fence && is_last_seg ? fence : NULL); + ret = amdgpu_vm_map_range( + svm->adev, svm->vm, flush_tlb && is_last_seg, true, + NULL, start_page, last_page, pte_flags, 0, entry->addr, + NULL, NULL, NULL, svm->vm->eviction_lock, base, + wait_fence && is_last_seg ? fence : NULL); if (ret) return ret; } @@ -308,14 +297,12 @@ int amdgpu_svm_range_update_mapping(struct amdgpu_svm *svm, { struct drm_exec exec; struct dma_fence *fence = NULL; - unsigned int flags; int ret; ret = amdgpu_svm_range_lock_vm_pd(svm, &exec, intr); if (ret) return ret; - flags = memalloc_noreclaim_save(); drm_gpusvm_notifier_lock(&svm->gpusvm); if (!amdgpu_svm_range_pages_valid(svm, range)) { @@ -327,24 +314,25 @@ int amdgpu_svm_range_update_mapping(struct amdgpu_svm *svm, wait, wait ? &fence : NULL); } - drm_gpusvm_notifier_unlock(&svm->gpusvm); - memalloc_noreclaim_restore(flags); - - if (!ret && fence) - dma_fence_wait(fence, intr); - dma_fence_put(fence); - if (!ret) - ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, NULL); + ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, + svm->vm->eviction_lock); if (!ret) { - if (flush_tlb) - amdgpu_svm_flush_tlb(svm); WRITE_ONCE(range->attr_flags, attrs->flags); WRITE_ONCE(range->gpu_mapped, true); range->validate_timestamp = ktime_get_boottime(); } + drm_gpusvm_notifier_unlock(&svm->gpusvm); + + if (!ret && fence) + dma_fence_wait(fence, intr); + dma_fence_put(fence); + + if (!ret && flush_tlb) + amdgpu_svm_flush_tlb(svm); + drm_exec_fini(&exec); return ret; } @@ -585,9 +573,12 @@ amdgpu_svm_range_invalidate_interval(struct amdgpu_svm *svm, crosses_boundary ? "ATTR DESTROY" : "ATTR ZAP PTE"); + /* Use the unified eviction/MMU notifier lock for PTE zaps. */ + down_write(svm->vm->eviction_lock); ret = amdgpu_svm_range_zap_ptes(svm, svm_range, drm_gpusvm_range_start(range) >> PAGE_SHIFT, (drm_gpusvm_range_end(range) >> PAGE_SHIFT) - 1); + up_write(svm->vm->eviction_lock); if (ret < 0) { drm_exec_fini(&exec); return ret; -- 2.53.0
