* [PATCH 7.1.y 1/2] drm/amdgpu: fix recursive ww_mutex acquire in amdgpu_devcoredump_format
2026-08-20 11:05 FAILED: patch "[PATCH] drm/amdgpu: Allocate coredump ring buffers per ring" failed to apply to 7.1-stable tree gregkh
@ 2026-08-24 18:28 ` Sasha Levin
2026-08-24 18:28 ` [PATCH 7.1.y 2/2] drm/amdgpu: Allocate coredump ring buffers per ring Sasha Levin
0 siblings, 1 reply; 3+ messages in thread
From: Sasha Levin @ 2026-08-24 18:28 UTC (permalink / raw)
To: stable; +Cc: Mikhail Gavrilov, Christian König, Alex Deucher, Sasha Levin
From: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
[ Upstream commit 7152b248dc3c8d5fa8629e99ed5655dd41b51562 ]
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: 7b15fc2d1f1a ("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)
Stable-dep-of: e40ff9840fa8 ("drm/amdgpu: Allocate coredump ring buffers per ring")
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
.../gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c | 215 ++++++++++--------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 91 +++++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
3 files changed, 184 insertions(+), 124 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dev_coredump.c
index dba7fca574271..5ae9fde80e510 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 @@
#include <generated/utsrelease.h>
#include <linux/devcoredump.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;
@@ -347,86 +462,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;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 97c5151bada3f..beefa05bf49b8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2947,47 +2947,56 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
}
/**
- * amdgpu_vm_lock_by_pasid - return an amdgpu_vm and its root bo from a pasid, if possible.
+ * amdgpu_vm_lock_by_pasid - look up a VM by PASID and lock its root PD
* @adev: amdgpu device pointer
- * @root: root BO of the VM
* @pasid: PASID of the VM
- * The caller needs to unreserve and unref the root bo on success.
+ * @exec: drm_exec context to lock the root PD in
+ *
+ * Must be called from within a drm_exec_until_all_locked() loop; the caller
+ * runs drm_exec_retry_on_contention() afterwards. The drm_exec context holds
+ * a reference on the root BO until it is finalised.
+ *
+ * Return: the VM on success, or NULL if the PASID has no VM, the VM is being
+ * torn down, or locking the root PD failed.
*/
struct amdgpu_vm *amdgpu_vm_lock_by_pasid(struct amdgpu_device *adev,
- struct amdgpu_bo **root, u32 pasid)
+ u32 pasid, struct drm_exec *exec)
{
unsigned long irqflags;
+ struct amdgpu_bo *root;
struct amdgpu_vm *vm;
int r;
xa_lock_irqsave(&adev->vm_manager.pasids, irqflags);
vm = xa_load(&adev->vm_manager.pasids, pasid);
- *root = vm ? amdgpu_bo_ref(vm->root.bo) : NULL;
+ root = vm ? amdgpu_bo_ref(vm->root.bo) : NULL;
xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags);
- if (!*root)
+ if (!root)
return NULL;
- r = amdgpu_bo_reserve(*root, true);
- if (r)
- goto error_unref;
+ r = drm_exec_lock_obj(exec, &root->tbo.base);
+ if (r) {
+ amdgpu_bo_unref(&root);
+ return NULL;
+ }
/* Double check that the VM still exists */
xa_lock_irqsave(&adev->vm_manager.pasids, irqflags);
vm = xa_load(&adev->vm_manager.pasids, pasid);
- if (vm && vm->root.bo != *root)
+ if (vm && vm->root.bo != root)
vm = NULL;
xa_unlock_irqrestore(&adev->vm_manager.pasids, irqflags);
- if (!vm)
- goto error_unlock;
+ if (!vm) {
+ drm_exec_unlock_obj(exec, &root->tbo.base);
+ amdgpu_bo_unref(&root);
+ return NULL;
+ }
- return vm;
-error_unlock:
- amdgpu_bo_unreserve(*root);
+ /* The drm_exec context holds its own reference on the root BO. */
+ amdgpu_bo_unref(&root);
-error_unref:
- amdgpu_bo_unref(root);
- return NULL;
+ return vm;
}
/**
@@ -3009,33 +3018,49 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
uint64_t ts, bool write_fault)
{
bool is_compute_context = false;
- struct amdgpu_bo *root;
+ struct drm_exec exec;
uint64_t value, flags;
struct amdgpu_vm *vm;
int r;
- vm = amdgpu_vm_lock_by_pasid(adev, &root, pasid);
- if (!vm)
+ drm_exec_init(&exec, 0, 1);
+ drm_exec_until_all_locked(&exec) {
+ vm = amdgpu_vm_lock_by_pasid(adev, pasid, &exec);
+ drm_exec_retry_on_contention(&exec);
+ if (!vm)
+ break;
+ }
+ if (!vm) {
+ drm_exec_fini(&exec);
return false;
+ }
is_compute_context = vm->is_compute_context;
if (is_compute_context) {
- /* Unreserve root since svm_range_restore_pages might try to reserve it. */
- /* TODO: rework svm_range_restore_pages so that this isn't necessary. */
- amdgpu_bo_unreserve(root);
+ /* Release the root PD lock since svm_range_restore_pages
+ * might try to take it.
+ * TODO: rework svm_range_restore_pages so that this isn't
+ * necessary.
+ */
+ drm_exec_fini(&exec);
if (!svm_range_restore_pages(adev, pasid, vmid,
- node_id, addr >> PAGE_SHIFT, ts, write_fault)) {
- amdgpu_bo_unref(&root);
+ node_id, addr >> PAGE_SHIFT, ts, write_fault))
return true;
- }
- amdgpu_bo_unref(&root);
/* Re-acquire the VM lock, could be that the VM was freed in between. */
- vm = amdgpu_vm_lock_by_pasid(adev, &root, pasid);
- if (!vm)
+ drm_exec_init(&exec, 0, 1);
+ drm_exec_until_all_locked(&exec) {
+ vm = amdgpu_vm_lock_by_pasid(adev, pasid, &exec);
+ drm_exec_retry_on_contention(&exec);
+ if (!vm)
+ break;
+ }
+ if (!vm) {
+ drm_exec_fini(&exec);
return false;
+ }
}
addr /= AMDGPU_GPU_PAGE_SIZE;
@@ -3059,7 +3084,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
value = 0;
}
- r = dma_resv_reserve_fences(root->tbo.base.resv, 1);
+ r = dma_resv_reserve_fences(vm->root.bo->tbo.base.resv, 1);
if (r) {
pr_debug("failed %d to reserve fence slot\n", r);
goto error_unlock;
@@ -3073,12 +3098,10 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
r = amdgpu_vm_update_pdes(adev, vm, true);
error_unlock:
- amdgpu_bo_unreserve(root);
+ drm_exec_fini(&exec);
if (r < 0)
dev_err(adev->dev, "Can't handle page fault (%d)\n", r);
- amdgpu_bo_unref(&root);
-
return false;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index d083d7aab75c6..0c6e3e0368c77 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -593,7 +593,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
bool write_fault);
struct amdgpu_vm *amdgpu_vm_lock_by_pasid(struct amdgpu_device *adev,
- struct amdgpu_bo **root, u32 pasid);
+ u32 pasid, struct drm_exec *exec);
void amdgpu_vm_set_task_info(struct amdgpu_vm *vm);
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread