amdgpu_gem_object_open() maps the BO into the caller's VM early
(amdgpu_vm_bo_add(), which sets abo->vm_bo) and only later validates and
attaches an eviction fence for dynamic dma-buf imports in compute VMs via
amdgpu_amdkfd_bo_validate_and_fence().
When that validate/fence step fails, the function returned the error
without undoing the VM mapping. Since .open() failed,
drm_gem_handle_create_tail() aborts the import and drops the last
reference, freeing the BO while it still has a bo_va attached. This trips
WARN_ON(abo->vm_bo) in amdgpu_bo_release_notify() and leaks the bo_va:
amdgpu 0004:04:00.0: validate_and_fence failed: -12
WARNING: CPU: 96 PID: 2868883 at amd/amdgpu/amdgpu_object.c:1343
amdgpu_bo_release_notify+0x15d/0x180 [amdgpu]
ttm_bo_put+0x149/0x350 [amdttm]
drm_gem_prime_fd_to_handle+0x132/0x1c0 [drm]
drm_ioctl+0x251/0x3d0 [drm]
Roll back the VM mapping on the error path: re-reserve the BO and PD, then
either delete the freshly added bo_va (also detaching the eviction fence)
or drop the extra ref_count taken for a pre-existing bo_va. Also clean up
the bo_va in the out_unlock path when the eviction fence attach itself
fails right after amdgpu_vm_bo_add(), which had the same leak.
Signed-off-by: Mukul Joshi <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 53 +++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 0d579517c03c..d52bbdc42b31 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -237,6 +237,7 @@ static int amdgpu_gem_object_open(struct drm_gem_object
*obj,
struct amdgpu_bo_va *bo_va;
struct mm_struct *mm;
struct drm_exec exec;
+ bool bo_va_added = false;
int r;
mm = amdgpu_ttm_tt_get_usermm(abo->tbo.ttm);
@@ -264,6 +265,7 @@ static int amdgpu_gem_object_open(struct drm_gem_object
*obj,
bo_va = amdgpu_vm_bo_find(vm, abo);
if (!bo_va) {
bo_va = amdgpu_vm_bo_add(adev, vm, abo);
+ bo_va_added = true;
r = amdgpu_evf_mgr_attach_fence(&fpriv->evf_mgr, abo);
if (r)
goto out_unlock;
@@ -304,9 +306,60 @@ static int amdgpu_gem_object_open(struct drm_gem_object
*obj,
}
}
mutex_unlock(&vm->process_info->lock);
+ if (r)
+ goto out_vm_bo_rollback;
+ return 0;
+
+out_vm_bo_rollback:
+ /*
+ * The open failed after we already mapped the BO into this VM. Undo
+ * that mapping, otherwise drm_gem_handle_create_tail() aborts the
+ * import and frees the BO while it still has a bo_va attached
+ * (abo->vm_bo != NULL), tripping WARN_ON(abo->vm_bo) in
+ * amdgpu_bo_release_notify() and leaking the bo_va.
+ */
+ drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 0);
+ drm_exec_until_all_locked(&exec) {
+ int r2 = drm_exec_prepare_obj(&exec, &abo->tbo.base, 1);
+
+ drm_exec_retry_on_contention(&exec);
+ if (unlikely(r2))
+ goto out_rollback_unlock;
+
+ r2 = amdgpu_vm_lock_pd(vm, &exec, 0);
+ drm_exec_retry_on_contention(&exec);
+ if (unlikely(r2))
+ goto out_rollback_unlock;
+ }
+
+ bo_va = amdgpu_vm_bo_find(vm, abo);
+ if (bo_va) {
+ if (bo_va_added) {
+ if (!amdgpu_vm_is_bo_always_valid(vm, abo))
+ amdgpu_evf_mgr_detach_fence(&fpriv->evf_mgr,
abo);
+ amdgpu_vm_bo_del(adev, bo_va);
+ amdgpu_vm_bo_update_shared(abo);
+ } else {
+ --bo_va->ref_count;
+ }
+ }
+
+out_rollback_unlock:
+ drm_exec_fini(&exec);
return r;
out_unlock:
+ /*
+ * If we already added a bo_va (e.g. the eviction fence attach below
+ * failed), remove it while still holding the reservations so the BO is
+ * not freed with a dangling abo->vm_bo.
+ */
+ if (bo_va_added) {
+ bo_va = amdgpu_vm_bo_find(vm, abo);
+ if (bo_va)
+ amdgpu_vm_bo_del(adev, bo_va);
+ amdgpu_vm_bo_update_shared(abo);
+ }
drm_exec_fini(&exec);
return r;
}
--
2.55.0
