On 9/23/2026 5:57 PM, Kuehling, Felix wrote:
On 2026-09-23 17:28, Mukul Joshi wrote:
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.

That looks like a lot of complexity for a rollback. Would it be easier to move validate_and_fence before mapping the BO? That way, if validate_and_fence fails, there is less to roll back.

Sure. That makes sense. I think that approach is being attempted by another engineer.

I will ask him to send the patch out for review.

I will drop this approach for now.


Regards,

Mukul

Regards,
  Felix



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;
  }

Reply via email to