amdgpu_trap_alloc() first acquires reservation_ww_class_mutex via
drm_exec, and then calls amdgpu_bo_create_kernel() while still holding
that lock. amdgpu_bo_create_kernel() internally calls
amdgpu_bo_create_reserved() which tries to acquire the same
reservation_ww_class_mutex again — causing a recursive lock and
triggering a lockdep deadlock warning on Navi44:
WARNING: possible recursive locking detected
6.19.0-kfd-root #38 Not tainted
--------------------------------------------
kworker/0:2/959 is trying to acquire lock:
ffff8bb17995d9e8 (reservation_ww_class_mutex){+.+.}-{4:4}, at:
amdgpu_bo_create_reserved+0xd2/0x2a0 [amdgpu]
but task is already holding lock:
ffffcc23c3397ba0 (reservation_ww_class_mutex){+.+.}-{4:4}, at:
amdgpu_trap_alloc+0xac/0x230 [amdgpu]
Possible unsafe locking scenario:
CPU0
----
lock(reservation_ww_class_mutex);
lock(reservation_ww_class_mutex);
*** DEADLOCK ***
Call Trace:
amdgpu_bo_create_kernel+0x1a/0x80 [amdgpu]
amdgpu_trap_alloc+0x18d/0x230 [amdgpu]
amdgpu_driver_open_kms+0x2f1/0x390 [amdgpu]
drm_file_alloc+0x20a/0x2d0 [drm]
drm_client_init+0x75/0x100 [drm]
amdgpu_amdkfd_drm_client_create+0x53/0x90 [amdgpu]
amdgpu_pci_probe+0x463/0x630 [amdgpu]
Fix this by setting bp.resv = vm->root.bo->tbo.base.resv when calling
amdgpu_bo_create() inside the drm_exec locked section. This causes
amdgpu_bo_create() to reuse the VM's already-held reservation object
instead of acquiring a new one, eliminating the recursive lock entirely.
The TMA BO requires no separate drm_exec_lock_obj() call since it shares
the VM's resv and is implicitly covered by amdgpu_vm_lock_pd().
v3 (Christian):
- Set bp.resv = vm->root.bo->tbo.base.resv so amdgpu_bo_create() reuses
the VM's already-held lock, keeping TMA BO creation inside the locked
section without a recursive acquire.
v2 (Christian):
- Use amdgpu_bo_create() instead of amdgpu_bo_create_kernel() for the
per-VM TMA BO; eviction is handled by the VM's BO list via
amdgpu_vm_bo_add(), no pinning needed.
Fixes: c50a45b2e98d ("drm/amdgpu: Add cwsr functions")
Cc: Lijo Lazar <[email protected]>
Cc: Christian König <[email protected]>
Cc: Alex Deucher <[email protected]>
Signed-off-by: Srinivasan Shanmugam <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c | 67 ++++++++++++++++--------
1 file changed, 46 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c
index 31f653ec3fb1..29eba2968350 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trap.c
@@ -377,6 +377,7 @@ static int amdgpu_trap_unmap_region(struct amdgpu_device
*adev,
int amdgpu_trap_alloc(struct amdgpu_device *adev, struct amdgpu_vm *vm,
struct amdgpu_trap_obj **trap_obj)
{
+ struct amdgpu_bo_param bp;
struct amdgpu_trap_obj *cwsr;
struct amdgpu_bo *bo;
struct drm_exec exec;
@@ -386,6 +387,7 @@ int amdgpu_trap_alloc(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
return -EOPNOTSUPP;
if (!vm || !trap_obj)
return -EINVAL;
+
cwsr = kzalloc(sizeof(*cwsr), GFP_KERNEL);
if (!cwsr)
return -ENOMEM;
@@ -401,35 +403,59 @@ int amdgpu_trap_alloc(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
dev_err(adev->dev,
"failed to reserve for CWSR allocs: err=%d\n",
r);
- goto err;
+ goto err_exec;
}
}
- r = amdgpu_bo_create_kernel(adev, AMDGPU_TRAP_TMA_MAX_SIZE, PAGE_SIZE,
- AMDGPU_GEM_DOMAIN_GTT, &cwsr->tma_bo, NULL,
- &cwsr->tma_cpu_addr);
+ /*
+ * Create TMA BO using the VM's already-locked resv so no new
+ * reservation_ww_class_mutex acquisition is needed — avoiding the
+ * recursive ww_mutex deadlock. Unpinned; eviction is handled via
+ * amdgpu_vm_bo_add() inside amdgpu_trap_map_region().
+ */
+ memset(&bp, 0, sizeof(bp));
+ bp.size = AMDGPU_TRAP_TMA_MAX_SIZE;
+ bp.byte_align = PAGE_SIZE;
+ bp.domain = AMDGPU_GEM_DOMAIN_GTT;
+ bp.flags = AMDGPU_GEM_CREATE_CPU_GTT_USWC |
+ AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
+ bp.type = ttm_bo_type_kernel;
+ bp.bo_ptr_size = sizeof(struct amdgpu_bo);
+ bp.resv = vm->root.bo->tbo.base.resv;
+
+ r = amdgpu_bo_create(adev, &bp, &cwsr->tma_bo);
if (r)
- goto err;
+ goto err_exec;
+
+ /* VM resv already held — kmap directly, no separate reserve needed */
+ r = amdgpu_bo_kmap(cwsr->tma_bo, &cwsr->tma_cpu_addr);
+ if (r)
+ goto err_tma;
+
+ memset(cwsr->tma_cpu_addr, 0, AMDGPU_TRAP_TMA_MAX_SIZE);
r = amdgpu_trap_map_region(adev, vm, cwsr, AMDGPU_TRAP_TMA);
if (r)
- goto err;
+ goto err_kmap;
+
r = amdgpu_trap_map_region(adev, vm, cwsr, AMDGPU_TRAP_TBA);
if (r) {
amdgpu_trap_unmap_region(adev, cwsr, AMDGPU_TRAP_TMA);
- goto err;
+ goto err_kmap;
}
-err:
drm_exec_fini(&exec);
- if (r) {
- amdgpu_bo_free_kernel(&cwsr->tma_bo, NULL, NULL);
- kfree(cwsr);
- *trap_obj = NULL;
- } else {
- *trap_obj = cwsr;
- }
+ *trap_obj = cwsr;
+ return 0;
+err_kmap:
+ amdgpu_bo_kunmap(cwsr->tma_bo);
+err_tma:
+ amdgpu_bo_unref(&cwsr->tma_bo);
+err_exec:
+ drm_exec_fini(&exec);
+ kfree(cwsr);
+ *trap_obj = NULL;
return r;
}
@@ -473,6 +499,7 @@ void amdgpu_trap_free(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
if (!trap_obj || !*trap_obj || !(*trap_obj)->tma_bo)
return;
+
tba_bo = adev->trap_info->isa_bo;
tma_bo = (*trap_obj)->tma_bo;
@@ -480,15 +507,11 @@ void amdgpu_trap_free(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
return;
drm_exec_init(&exec, 0, 0);
- drm_exec_until_all_locked(&exec)
- {
+ drm_exec_until_all_locked(&exec) {
r = amdgpu_vm_lock_pd(vm, &exec, 0);
if (likely(!r))
r = drm_exec_lock_obj(&exec, &tba_bo->tbo.base);
drm_exec_retry_on_contention(&exec);
- if (likely(!r))
- r = drm_exec_lock_obj(&exec, &tma_bo->tbo.base);
- drm_exec_retry_on_contention(&exec);
if (unlikely(r)) {
dev_err(adev->dev,
"failed to reserve CWSR BOs: err=%d\n", r);
@@ -500,7 +523,9 @@ void amdgpu_trap_free(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
amdgpu_trap_unmap_region(adev, *trap_obj, AMDGPU_TRAP_TMA);
err:
drm_exec_fini(&exec);
- amdgpu_bo_free_kernel(&(*trap_obj)->tma_bo, NULL, NULL);
+ /* TMA BO not pinned — release CPU mapping and free */
+ amdgpu_bo_kunmap((*trap_obj)->tma_bo);
+ amdgpu_bo_unref(&(*trap_obj)->tma_bo);
kfree(*trap_obj);
*trap_obj = NULL;
}
--
2.34.1
