mirrors/linux
2
0
Fork 0
mirror of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git synced 2026-07-25 05:00:48 -04:00
Code Issues Packages Projects Releases Wiki Activity

drm/amdgpu: fix recursive ww_mutex acquire in amdgpu_devcoredump_format

Browse Source 
When dumping IB contents from a hung job, amdgpu_devcoredump_format()
acquired the VM root PD's reservation via amdgpu_vm_lock_by_pasid() and
then, for each IB, called amdgpu_bo_reserve() on the BO backing the IB.
Both reservations are reservation_ww_class_mutex objects and neither
used 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. With amdgpu.gpu_recovery=0 the timeout handler
refires every ~2 s and each invocation produces this splat, drowning the
kernel ring buffer.

Now that amdgpu_vm_lock_by_pasid() takes a drm_exec context, move the IB
dumping into a separate helper that locks the root PD and every IB BO
together in a single drm_exec ticket. DRM_EXEC_IGNORE_DUPLICATES handles
IB BOs that share a dma_resv (e.g. always-valid BOs, or two IBs backed
by the same BO). Every lock is now a top-level acquire under one
ww_acquire_ctx, so the recursive ww_mutex condition is gone, and the
per-IB amdgpu_bo_reserve()/amdgpu_bo_unref() dance -- including a BO
refcount leak on the amdgpu_bo_reserve() failure path -- is removed.

Fixes: 7b15fc2d1f ("drm/amdgpu: dump job ibs in the devcoredump")
Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
(cherry picked from commit d6bf4242731219ee08ce54c365631e395486651e)
This commit is contained in:
Mikhail Gavrilov
2026-05-29 11:47:39 +05:00
committed by Alex Deucher
parent 9920249a52
commit 7152b248dc
1 changed files with 126 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

215
drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
View File

@@ -24,6 +24,7 @@
#include <linux/devcoredump.h>
#include <linux/utsname.h>
#include <drm/drm_exec.h>
#include "amdgpu_dev_coredump.h"
#include "atom.h"
@@ -207,23 +208,137 @@ static void amdgpu_devcoredump_fw_info(struct amdgpu_device *adev,
}
}
static void
amdgpu_devcoredump_print_ibs(struct drm_printer *p,
struct amdgpu_coredump_info *coredump,
bool sizing_pass)
{
struct amdgpu_device *adev = coredump->adev;
struct amdgpu_bo_va_mapping *mapping;
struct amdgpu_bo *abo;
struct drm_exec exec;
struct amdgpu_vm *vm;
u32 *ib_content;
u64 va_start, offset;
u8 *kptr;
u32 off;
int r;
/*
* On the sizing pass there is no VM to look up and no BO to lock; the
* size estimate doesn't depend on whether the IB BOs are reachable.
* Just emit the per-IB headers (the content is not written anywhere).
*/
if (sizing_pass) {
for (int i = 0; i < coredump->num_ibs; i++) {
drm_printf(p, "\nIB #%d 0x%llx %d dw\n", i,
coredump->ibs[i].gpu_addr,
coredump->ibs[i].ib_size_dw);
}
return;
}
/*
* Lock the VM root PD and every IB BO together in a single drm_exec
* ticket. Reserving the IB BOs one by one while the root PD is held
* would be a recursive reservation_ww_class_mutex acquire without a
* ww_acquire_ctx, which trips lockdep and self-deadlocks for IB BOs
* that share their dma_resv with the root PD (always-valid BOs).
*/
drm_exec_init(&exec, DRM_EXEC_IGNORE_DUPLICATES, 1 + coredump->num_ibs);
drm_exec_until_all_locked(&exec) {
vm = amdgpu_vm_lock_by_pasid(adev, coredump->pasid, &exec);
if (!vm)
goto unlock;
for (int i = 0; i < coredump->num_ibs; i++) {
u64 pfn = (coredump->ibs[i].gpu_addr &
AMDGPU_GMC_HOLE_MASK) / AMDGPU_GPU_PAGE_SIZE;
mapping = amdgpu_vm_bo_lookup_mapping(vm, pfn);
if (!mapping)
continue;
abo = mapping->bo_va->base.bo;
r = drm_exec_lock_obj(&exec, &abo->tbo.base);
drm_exec_retry_on_contention(&exec);
if (r)
goto unlock;
}
}
for (int i = 0; i < coredump->num_ibs; i++) {
bool emit_content = false;
ib_content = kvmalloc_array(coredump->ibs[i].ib_size_dw, 4,
GFP_KERNEL);
if (!ib_content)
continue;
va_start = coredump->ibs[i].gpu_addr & AMDGPU_GMC_HOLE_MASK;
mapping = amdgpu_vm_bo_lookup_mapping(vm,
va_start / AMDGPU_GPU_PAGE_SIZE);
if (!mapping)
goto output_ib_content;
abo = mapping->bo_va->base.bo;
offset = va_start - mapping->start * AMDGPU_GPU_PAGE_SIZE;
if (abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS) {
struct amdgpu_res_cursor cursor;
off = 0;
if (abo->tbo.resource->mem_type != TTM_PL_VRAM)
goto output_ib_content;
amdgpu_res_first(abo->tbo.resource, offset,
coredump->ibs[i].ib_size_dw * 4, &cursor);
while (cursor.remaining) {
amdgpu_device_mm_access(adev, cursor.start / 4,
&ib_content[off], cursor.size / 4,
false);
off += cursor.size;
amdgpu_res_next(&cursor, cursor.size);
}
emit_content = true;
} else {
r = ttm_bo_kmap(&abo->tbo, 0, PFN_UP(abo->tbo.base.size),
&abo->kmap);
if (r)
goto output_ib_content;
kptr = amdgpu_bo_kptr(abo);
kptr += offset;
memcpy(ib_content, kptr, coredump->ibs[i].ib_size_dw * 4);
amdgpu_bo_kunmap(abo);
emit_content = true;
}
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);
if (emit_content) {
for (int j = 0; j < coredump->ibs[i].ib_size_dw; j++)
drm_printf(p, "0x%08x\n", ib_content[j]);
}
kvfree(ib_content);
}
unlock:
drm_exec_fini(&exec);
}
static ssize_t
amdgpu_devcoredump_format(char *buffer, size_t count, struct amdgpu_coredump_info *coredump)
{
struct amdgpu_device *adev = coredump->adev;
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;
int ver, i, j;
u32 ring_idx, off;
bool sizing_pass;
@@ -343,86 +458,8 @@ amdgpu_devcoredump_format(char *buffer, size_t count, struct amdgpu_coredump_inf
else if (coredump->reset_vram_lost)
drm_printf(&p, "VRAM is lost due to GPU reset!\n");
if (coredump->num_ibs) {
/* Don't try to lookup the VM or map the BOs when calculating the
* size required to store the devcoredump.
*/
if (sizing_pass)
vm = NULL;
else
vm = amdgpu_vm_lock_by_pasid(adev, &root, coredump->pasid);
for (int i = 0; i < coredump->num_ibs && (sizing_pass || vm); i++) {
ib_content = kvmalloc_array(coredump->ibs[i].ib_size_dw, 4,
GFP_KERNEL);
if (!ib_content)
continue;
/* vm=NULL 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)
goto output_ib_content;
va_start = coredump->ibs[i].gpu_addr & AMDGPU_GMC_HOLE_MASK;
mapping = amdgpu_vm_bo_lookup_mapping(vm, va_start / AMDGPU_GPU_PAGE_SIZE);
if (!mapping)
goto free_ib_content;
offset = va_start - (mapping->start * AMDGPU_GPU_PAGE_SIZE);
abo = amdgpu_bo_ref(mapping->bo_va->base.bo);
r = amdgpu_bo_reserve(abo, false);
if (r)
goto free_ib_content;
if (abo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS) {
off = 0;
if (abo->tbo.resource->mem_type != TTM_PL_VRAM)
goto unreserve_abo;
amdgpu_res_first(abo->tbo.resource, offset,
coredump->ibs[i].ib_size_dw * 4,
&cursor);
while (cursor.remaining) {
amdgpu_device_mm_access(adev, cursor.start / 4,
&ib_content[off], cursor.size / 4,
false);
off += cursor.size;
amdgpu_res_next(&cursor, cursor.size);
}
} else {
r = ttm_bo_kmap(&abo->tbo, 0,
PFN_UP(abo->tbo.base.size),
&abo->kmap);
if (r)
goto unreserve_abo;
kptr = amdgpu_bo_kptr(abo);
kptr += offset;
memcpy(ib_content, kptr,
coredump->ibs[i].ib_size_dw * 4);
amdgpu_bo_kunmap(abo);
}
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 = 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:
kvfree(ib_content);
}
if (vm) {
amdgpu_bo_unreserve(root);
amdgpu_bo_unref(&root);
}
}
if (coredump->num_ibs)
amdgpu_devcoredump_print_ibs(&p, coredump, sizing_pass);
return count - iter.remain;
}