From nobody Mon May 25 01:58:59 2026 Received: from mail-lj1-f174.google.com (mail-lj1-f174.google.com [209.85.208.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6D7E9407CE4 for ; Tue, 19 May 2026 16:16:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.174 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779207363; cv=none; b=nmdnr6ZSMKEhYv0TvT7VtphQ3CubKAvtIGvLcWp2lI0zIDYQpTtGvBgkoRODF9+4XF+tymQEI86wBTfQ/HNnbsKQ7HmXEm4Sig3XIuGyaKyDeEFd7D7zacK1O7ybE2bjEdpZL1ijm4AP27jpdGEadpXHyEl2rZO49CuKqyYadlI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779207363; c=relaxed/simple; bh=8oB0ZVPBhFN4AWxuaWgssbg7aCzCyr+6nVcFQx4xqh4=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=VA0XJgszfac+EOaqxGicJ1g29AkA/5ZK9CAbbjsVGdEv+dpk5sjNKZ5armCdeHZvo1B0uHqEsQQ1+R/YAkvyBaUFbMubKW1kebF7TKiQdfkDrbeTy3crF645YPEc50j6cv8NlPdoUjX8VPAUkV3/y6KHUjaihmzdIr/ySDFO14s= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=W5+mwzCg; arc=none smtp.client-ip=209.85.208.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="W5+mwzCg" Received: by mail-lj1-f174.google.com with SMTP id 38308e7fff4ca-38e7d983f91so40111541fa.2 for ; Tue, 19 May 2026 09:16:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779207360; x=1779812160; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=0uDjM1EcMOi30GaXc68xaMNS0uV90e3QyVWS7Pj4PBk=; b=W5+mwzCgB74OTmUjx9htEHCA5OSrgtL9HU+6/2ZHDm9tBrcMK9r5jmQpcK/gj9fCOX nTzYYIfra6Kw5dMPz/QqYRYCZIC3p1ub5DVCQY3OrfiXsrwVCRQmd2vv4d+Y7fcBVbMj Dp/MOr7fzVXv1GoUTTbdgqgaeByvP8YhI5MdTuVO1aMNMp7oUdBMDCiaxc3SsijXdVtB 1aRiA9s4Rsk+6cnRthOwbIOFxX5x6wbu8f9uUmrSUAZP9BmNeZQ2+UFdoiROFd4lVCb4 V4fXut396x+UgnnzWsQufmr0M1kOak0AnuyECbAz69XEr7tVIng5ORzejZvuJD8A3dM4 vANA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779207360; x=1779812160; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=0uDjM1EcMOi30GaXc68xaMNS0uV90e3QyVWS7Pj4PBk=; b=YED5FS0MlsGHJgacGPyrY54GgpYbJwh3QrbgVrAumXVD04xOPyOytLv9Mlyg7R9+EW ai4D9oMLr2AuiC66U0m+vxVvybluyFTZQpnu3ozWUdQ1ZeZmCkXkJCSisT/wegm/+rlw sOAA5vtZ9XjpY2j+huaahhQO7Quzqt+j6krvr83qv9hHniBLmJnRwOoZBRBfBogRajqZ MVic96StLnrPFCFYVaKoqyW3mQeqIefHTree7bCZQ05ezBACrcSO8/5zxbu+l41NPf+n Zsobs00gR5qKWjmr3J9debZDe705JRjAEBIk+1kC9H72unG6uVQU32HnYuBFNvqAD5kF 7bxg== X-Forwarded-Encrypted: i=1; AFNElJ9MwIZjRwUUDlX4/62i0LYrZ4PO5T5MliS7Hvj/CkCMkxQHOHml4b0S88/PlrGaPFZCkFy7iJkFeoRddKk=@vger.kernel.org X-Gm-Message-State: AOJu0YxwOYKBHJvYyQlLKrhrGLTL633J/yO0vF0i3bQgsCH0L6NpQJwf CDPlJLcBRenK+3YohF3qKYIxFNGNRXCc/0D0Uk3bqiGvfYtD2sTnibn+ X-Gm-Gg: Acq92OG2+mdeg2A1pgqoNN8XkzUt6sADyvzMfxLo07DRFUdB+ovmXNmyQ+8TBxMN66k xjVelaoVU2Uc9FogMXy9vOk1SDV8SY3dsCn3tXe0vLOhuPqVXuwsBmK88DON6LVohJn9hPSYagL Tx4ZhWSK/MulVSrblokEG4wPgr0qXcTvd8a7N97lrrjmManT1akn/V2vc3rxLOxhZMuRsOXzG3d pzfTfX9c3PDlYlFcGe9l+Fxv/5NS11hIMni6NI2cp9MsvAO921W/y1IgTiYcpfLUrB+fkqcrO6D Ei58F5wSGUKUwAAkrZ0CrAmTXdylmcsKU4TsTOd3RgC/kpyZpGYihq7T/RXtzmGk6pbfbrztY/D PbfuUir6AqSIJBpkj/Oeo6nHF3VjJmZmfPhScGKnAY7KnXIwDKABBsir8tVcR+l1aVgf3YrrKkM 7S8yhXEtXmqT5Wl59lxlH8PDAHXjwClzUvNMzzc+DkJyE= X-Received: by 2002:a05:651c:1543:b0:393:77ef:9079 with SMTP id 38308e7fff4ca-39561f57db0mr66667601fa.28.1779207359359; Tue, 19 May 2026 09:15:59 -0700 (PDT) Received: from localhost ([188.234.148.119]) by smtp.gmail.com with ESMTPSA id 38308e7fff4ca-395882c41c1sm20823091fa.12.2026.05.19.09.15.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 May 2026 09:15:57 -0700 (PDT) From: Mikhail Gavrilov To: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Cc: Mikhail Gavrilov , =?UTF-8?q?Christian=20K=C3=B6nig?= , Alex Deucher , David Airlie , Simona Vetter , Sumit Semwal , Pierre-Eric Pelloux-Prayer , linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org Subject: [PATCH v2] drm/amdgpu: fix recursive ww_mutex acquire in amdgpu_devcoredump_format Date: Tue, 19 May 2026 21:15:40 +0500 Message-ID: <20260519161541.19994-1-mikhail.v.gavrilov@gmail.com> X-Mailer: git-send-email 2.54.0 In-Reply-To: <20260429143743.50743-1-mikhail.v.gavrilov@gmail.com> References: <20260429143743.50743-1-mikhail.v.gavrilov@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable When dumping IB contents from a hung job, amdgpu_devcoredump_format() acquires the VM root PD's reservation lock via amdgpu_vm_lock_by_pasid() and then, for each IB referenced by the job, calls amdgpu_bo_reserve() on the BO that backs the IB. Both reservations are taken on reservation_ww_class_mutex objects but neither uses a ww_acquire_ctx, which trips lockdep: WARNING: possible recursive locking detected -------------------------------------------- kworker/u128:0 is trying to acquire lock: ffff88838b16e1f0 (reservation_ww_class_mutex){+.+.}-{4:4}, at: amdgpu_devcoredump_format+0x1594/0x23f0 [amdgpu] but task is already holding lock: ffff8882f82681f0 (reservation_ww_class_mutex){+.+.}-{4:4}, at: amdgpu_devcoredump_format+0x1594/0x23f0 [amdgpu] Possible unsafe locking scenario: CPU0 ---- lock(reservation_ww_class_mutex); lock(reservation_ww_class_mutex); *** DEADLOCK *** May be due to missing lock nesting notation Workqueue: events_unbound amdgpu_devcoredump_deferred_work [amdgpu] Call Trace: __ww_mutex_lock.constprop.0 ww_mutex_lock amdgpu_bo_reserve amdgpu_devcoredump_format+0x1594 [amdgpu] amdgpu_devcoredump_deferred_work+0xea [amdgpu] The two reservations are on different BOs in the captured trace, so the splat is a lockdep-correctness warning, not an observed deadlock. It becomes a real self-deadlock whenever the IB BO shares its dma_resv with the root PD (the always-valid case, see amdgpu_vm_is_bo_always_valid()): amdgpu_bo_reserve(abo) re-acquires the same ww_mutex without a ticket and blocks forever. Fix it in two steps: 1. Collect per-IB BO references under the root PD's reservation, then release the root before locking the IB BOs. The walk over the VM mapping tree must remain under the root lock (mappings can be torn down without it), but the actual content copies do not. 2. Lock all the IB BOs together using drm_exec(9) with a single ww_acquire_ctx. DRM_EXEC_IGNORE_DUPLICATES handles the case where IB BOs share a dma_resv (e.g. always-valid BOs). Each lock attempt is now a top-level acquire under one ticket, with retry-on- contention handled by drm_exec; the recursive ww_mutex condition is gone. The collect/lock/release logic is factored out into three small helpers (amdgpu_devcoredump_{collect,lock,release}_ib_refs) to keep the main function readable and within the kernel coding style indentation guideline. This also fixes a BO refcount leak in the original code: when amdgpu_bo_reserve() failed, control jumped to free_ib_content without running amdgpu_bo_unref(). In the new structure the per-IB BO refs are released unconditionally in the cleanup helper. Reproducer (~150 LoC libdrm_amdgpu): submit a single GFX IB containing PACKET3_INDIRECT_BUFFER chained at GPU VA 0 and wait for the fence. The TDR fires within ~10 s and the deferred coredump worker produces the splat above on every invocation. v2: switch from per-IB amdgpu_bo_reserve() to drm_exec for the IB BO locking as suggested by Christian K=C3=B6nig; the snapshot approach for collecting BO references under the root PD's reservation is retained. Fixes: 7b15fc2d1f1a ("drm/amdgpu: dump job ibs in the devcoredump") Suggested-by: Christian K=C3=B6nig Signed-off-by: Mikhail Gavrilov --- .../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c | 187 ++++++++++++++---- 1 file changed, 148 insertions(+), 39 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c b/drivers/gpu= /drm/amd/amdgpu/amdgpu_dev_coredump.c index d386bc775d03..9ac958cf09fd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c @@ -24,6 +24,7 @@ =20 #include #include +#include #include "amdgpu_dev_coredump.h" #include "atom.h" =20 @@ -207,6 +208,108 @@ static void amdgpu_devcoredump_fw_info(struct amdgpu_= device *adev, } } =20 +struct amdgpu_devcoredump_ib_ref { + struct amdgpu_bo *bo; + u64 offset; +}; + +/* + * Walk the VM's mapping tree under the root PD's reservation to obtain th= e BO + * that backs each IB and pin it with a refcount. The root PD reservation = is + * dropped before this function returns; the caller can then lock each IB = BO + * via drm_exec without nesting reservations on reservation_ww_class_mutex. + * + * Returns an array of num_ibs entries (each ib_refs[i].bo may be NULL if = its + * mapping was not found), or NULL on allocation failure / VM lookup failu= re. + * The caller must release the BO refs and free the array via + * amdgpu_devcoredump_release_ib_refs(). + */ +static struct amdgpu_devcoredump_ib_ref * +amdgpu_devcoredump_collect_ib_refs(struct amdgpu_device *adev, + struct amdgpu_coredump_info *coredump) +{ + struct amdgpu_devcoredump_ib_ref *ib_refs; + struct amdgpu_bo_va_mapping *mapping; + struct amdgpu_bo *root; + struct amdgpu_vm *vm; + u64 va_start; + + ib_refs =3D kcalloc(coredump->num_ibs, sizeof(*ib_refs), GFP_KERNEL); + if (!ib_refs) + return NULL; + + vm =3D amdgpu_vm_lock_by_pasid(adev, &root, coredump->pasid); + if (!vm) { + kfree(ib_refs); + return NULL; + } + + for (int i =3D 0; i < coredump->num_ibs; i++) { + va_start =3D coredump->ibs[i].gpu_addr & AMDGPU_GMC_HOLE_MASK; + mapping =3D amdgpu_vm_bo_lookup_mapping(vm, va_start / AMDGPU_GPU_PAGE_S= IZE); + if (!mapping) + continue; + + ib_refs[i].bo =3D amdgpu_bo_ref(mapping->bo_va->base.bo); + ib_refs[i].offset =3D va_start - + mapping->start * AMDGPU_GPU_PAGE_SIZE; + } + + amdgpu_bo_unreserve(root); + amdgpu_bo_unref(&root); + + return ib_refs; +} + +static void +amdgpu_devcoredump_release_ib_refs(struct amdgpu_devcoredump_ib_ref *ib_re= fs, + int num_ibs) +{ + if (!ib_refs) + return; + + for (int i =3D 0; i < num_ibs; i++) + if (ib_refs[i].bo) + amdgpu_bo_unref(&ib_refs[i].bo); + kfree(ib_refs); +} + +/* + * Lock all collected IB BOs together using a single drm_exec ticket. This + * eliminates the nested ww_mutex acquire that lockdep flags as recursive + * locking (and that becomes a real self-deadlock for IB BOs sharing their + * dma_resv with the root PD). + * + * Returns 0 if drm_exec was initialised and the BOs are locked; the caller + * must call drm_exec_fini() on success. Returns non-zero on failure, in w= hich + * case drm_exec is already torn down. + */ +static int +amdgpu_devcoredump_lock_ib_refs(struct drm_exec *exec, + struct amdgpu_devcoredump_ib_ref *ib_refs, + int num_ibs) +{ + int r =3D 0; + + drm_exec_init(exec, DRM_EXEC_IGNORE_DUPLICATES, num_ibs); + drm_exec_until_all_locked(exec) { + r =3D 0; + for (int i =3D 0; i < num_ibs; i++) { + if (!ib_refs[i].bo) + continue; + r =3D drm_exec_lock_obj(exec, &ib_refs[i].bo->tbo.base); + drm_exec_retry_on_contention(exec); + if (r) + break; + } + if (r) + break; + } + if (r) + drm_exec_fini(exec); + return r; +} + static ssize_t amdgpu_devcoredump_format(char *buffer, size_t count, struct amdgpu_coredu= mp_info *coredump) { @@ -214,13 +317,9 @@ amdgpu_devcoredump_format(char *buffer, size_t count, = struct amdgpu_coredump_inf struct drm_printer p; struct drm_print_iterator iter; struct amdgpu_vm_fault_info *fault_info; - struct amdgpu_bo_va_mapping *mapping; struct amdgpu_ip_block *ip_block; struct amdgpu_res_cursor cursor; - struct amdgpu_bo *abo, *root; - uint64_t va_start, offset; struct amdgpu_ring *ring; - struct amdgpu_vm *vm; u32 *ib_content; uint8_t *kptr; int ver, i, j, r; @@ -343,43 +442,52 @@ amdgpu_devcoredump_format(char *buffer, size_t count,= struct amdgpu_coredump_inf drm_printf(&p, "VRAM is lost due to GPU reset!\n"); =20 if (coredump->num_ibs) { - /* Don't try to lookup the VM or map the BOs when calculating the - * size required to store the devcoredump. + struct amdgpu_devcoredump_ib_ref *ib_refs =3D NULL; + struct drm_exec exec; + bool ibs_locked =3D false; + + /* + * Collect the BO that backs each IB under the root PD's + * reservation, drop the root reservation, then lock all the + * IB BOs together in one drm_exec ticket. This avoids nesting + * amdgpu_bo_reserve() inside the root PD's reservation, which + * would be a recursive reservation_ww_class_mutex acquire + * without a ww_acquire_ctx (lockdep splat, and a real + * self-deadlock for always-valid BOs that share their dma_resv + * with the root PD). + * + * Skip lookup/locking entirely on the sizing pass: it does not + * write IB content, and the size estimate doesn't depend on + * whether the BOs are reachable. */ - if (sizing_pass) - vm =3D NULL; - else - vm =3D amdgpu_vm_lock_by_pasid(adev, &root, coredump->pasid); + if (!sizing_pass) { + ib_refs =3D amdgpu_devcoredump_collect_ib_refs(adev, coredump); + if (ib_refs) { + r =3D amdgpu_devcoredump_lock_ib_refs(&exec, ib_refs, + coredump->num_ibs); + if (!r) + ibs_locked =3D true; + } + } + + for (int i =3D 0; i < coredump->num_ibs; i++) { + struct amdgpu_bo *abo =3D ibs_locked ? ib_refs[i].bo : NULL; + u64 offset =3D ibs_locked ? ib_refs[i].offset : 0; + bool emit_content =3D sizing_pass; =20 - for (int i =3D 0; i < coredump->num_ibs && (sizing_pass || vm); i++) { ib_content =3D kvmalloc_array(coredump->ibs[i].ib_size_dw, 4, GFP_KERNEL); if (!ib_content) continue; =20 - /* vm=3DNULL can only happen when 'sizing_pass' is true. Skip to the - * drm_printf() calls (ib_content doesn't need to be initialized - * as its content won't be written anywhere). - */ - if (!vm) + if (!abo) goto output_ib_content; =20 - va_start =3D coredump->ibs[i].gpu_addr & AMDGPU_GMC_HOLE_MASK; - mapping =3D amdgpu_vm_bo_lookup_mapping(vm, va_start / AMDGPU_GPU_PAGE_= SIZE); - if (!mapping) - goto free_ib_content; - - offset =3D va_start - (mapping->start * AMDGPU_GPU_PAGE_SIZE); - abo =3D amdgpu_bo_ref(mapping->bo_va->base.bo); - r =3D amdgpu_bo_reserve(abo, false); - if (r) - goto free_ib_content; - if (abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS) { off =3D 0; =20 if (abo->tbo.resource->mem_type !=3D TTM_PL_VRAM) - goto unreserve_abo; + goto output_ib_content; =20 amdgpu_res_first(abo->tbo.resource, offset, coredump->ibs[i].ib_size_dw * 4, @@ -391,12 +499,13 @@ amdgpu_devcoredump_format(char *buffer, size_t count,= struct amdgpu_coredump_inf off +=3D cursor.size; amdgpu_res_next(&cursor, cursor.size); } + emit_content =3D true; } else { r =3D ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size), &abo->kmap); if (r) - goto unreserve_abo; + goto output_ib_content; =20 kptr =3D amdgpu_bo_kptr(abo); kptr +=3D offset; @@ -404,23 +513,23 @@ amdgpu_devcoredump_format(char *buffer, size_t count,= struct amdgpu_coredump_inf coredump->ibs[i].ib_size_dw * 4); =20 amdgpu_bo_kunmap(abo); + emit_content =3D true; } =20 output_ib_content: drm_printf(&p, "\nIB #%d 0x%llx %d dw\n", i, coredump->ibs[i].gpu_addr, coredump->ibs[i].ib_size_dw); - for (int j =3D 0; j < coredump->ibs[i].ib_size_dw; j++) - drm_printf(&p, "0x%08x\n", ib_content[j]); -unreserve_abo: - if (vm) - amdgpu_bo_unreserve(abo); -free_ib_content: + if (emit_content) { + for (int j =3D 0; j < coredump->ibs[i].ib_size_dw; j++) + drm_printf(&p, "0x%08x\n", ib_content[j]); + } kvfree(ib_content); } - if (vm) { - amdgpu_bo_unreserve(root); - amdgpu_bo_unref(&root); - } + + if (ibs_locked) + drm_exec_fini(&exec); + + amdgpu_devcoredump_release_ib_refs(ib_refs, coredump->num_ibs); } =20 return count - iter.remain; --=20 2.54.0