On 9/15/26 09:42, Srinivasan Shanmugam wrote:
> 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]>
Reviewed-by: Christian König <[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;
> }
