From: Vitaly Prosyak <[email protected]>
The eviction-fence worker runs inside a DMA-fence signalling section.
Userq eviction can block on reset_domain->sem while GPU recovery holds
that semaphore and invalidates mappings. Lockdep reports the resulting
dependency cycle through dma_fence_map.
[ +0.000295] amdgpu 0000:03:00.0: GPU reset begin!. Source: 5
[ +0.000233] ======================================================
[ +0.000001] WARNING: possible circular locking dependency detected
[ +0.000000] 7.2.0+ #97 Tainted: G W
[ +0.000001] ------------------------------------------------------
[ +0.000001] kworker/u128:1/238 is trying to acquire lock:
[ +0.000001] ffff897b4fcc2eb8 (&mapping->i_mmap_rwsem){++++}-{4:4}, at:
unmap_mapping_pages+0x6a/0xd0
[ +0.000005]
but task is already holding lock:
[ +0.000001] ffff897b0fd60ec0 (&reset_domain->sem){++++}-{4:4}, at:
amdgpu_device_lock_reset_domain+0x1c/0x30 [amdgpu]
[ +0.000294]
which lock already depends on the new lock.
[ +0.000000]
the existing dependency chain (in reverse order) is:
[ +0.000001]
-> #3 (&reset_domain->sem){++++}-{4:4}:
[ +0.000002] down_read+0x30/0x180
[ +0.000004] amdgpu_userq_unmap_helper+0x46/0x290 [amdgpu]
[ +0.000212] amdgpu_userq_evict+0x4e/0xf0 [amdgpu]
[ +0.000198] amdgpu_eviction_fence_suspend_worker+0x51/0xf0 [amdgpu]
[ +0.000176] process_one_work+0x22e/0x760
[ +0.000003] worker_thread+0x1db/0x3d0
[ +0.000001] kthread+0xfe/0x140
[ +0.000001] ret_from_fork+0x3bd/0x470
[ +0.000002] ret_from_fork_asm+0x1a/0x30
[ +0.000002]
-> #2 (dma_fence_map){++++}-{0:0}:
[ +0.000003] __dma_fence_might_wait+0x42/0xf0
[ +0.000002] dma_resv_lockdep+0x26d/0x3b0
[ +0.000002] do_one_initcall+0x74/0x3e0
[ +0.000002] kernel_init_freeable+0x3f8/0x6d0
[ +0.000002] kernel_init+0x1b/0x200
[ +0.000002] ret_from_fork+0x3bd/0x470
[ +0.000001] ret_from_fork_asm+0x1a/0x30
[ +0.000002]
-> #1 (mmu_notifier_invalidate_range_start){+.+.}-{0:0}:
[ +0.000002] __split_huge_pmd+0x86/0x190
[ +0.000002] vma_adjust_trans_huge+0x12b/0x1c0
[ +0.000002] __split_vma+0x236/0x370
[ +0.000002] vms_gather_munmap_vmas+0xe8/0x360
[ +0.000001] __mmap_region+0x2ab/0x1060
[ +0.000001] mmap_region+0xc4/0x180
[ +0.000001] do_mmap+0x48b/0x660
[ +0.000002] vm_mmap_pgoff+0x129/0x230
[ +0.000002] ksys_mmap_pgoff+0x1a2/0x270
[ +0.000001] __x64_sys_mmap+0x33/0x70
[ +0.000002] x64_sys_call+0x2194/0x26e0
[ +0.000001] do_syscall_64+0x103/0x6d0
[ +0.000001] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ +0.000002]
-> #0 (&mapping->i_mmap_rwsem){++++}-{4:4}:
[ +0.000002] __lock_acquire+0x15aa/0x2740
[ +0.000001] lock_acquire+0xc4/0x2f0
[ +0.000002] down_read+0x30/0x180
[ +0.000001] unmap_mapping_pages+0x6a/0xd0
[ +0.000001] unmap_mapping_range+0x25/0x40
[ +0.000002] amdgpu_device_gpu_recover+0x17a/0x400 [amdgpu]
[ +0.000168] amdgpu_debugfs_reset_work+0x68/0x90 [amdgpu]
[ +0.000167] process_one_work+0x22e/0x760
[ +0.000002] worker_thread+0x1db/0x3d0
[ +0.000001] kthread+0xfe/0x140
[ +0.000001] ret_from_fork+0x3bd/0x470
[ +0.000001] ret_from_fork_asm+0x1a/0x30
[ +0.000002]
other info that might help us debug this:
[ +0.000000] Chain exists of:
&mapping->i_mmap_rwsem --> dma_fence_map --> &reset_domain->sem
[ +0.000003] Possible unsafe locking scenario:
[ +0.000000] CPU0 CPU1
[ +0.000001] ---- ----
[ +0.000001] lock(&reset_domain->sem);
[ +0.000001] lock(dma_fence_map);
[ +0.000001] lock(&reset_domain->sem);
[ +0.000001] rlock(&mapping->i_mmap_rwsem);
[ +0.000001]
*** DEADLOCK ***
[ +0.000000] 3 locks held by kworker/u128:1/238:
[ +0.000001] #0: ffff897b50d5f940
((wq_completion)amdgpu-reset-dev){+.+.}-{0:0}, at: process_one_work+0x4a8/0x760
[ +0.000003] #1: ffffce9fc0a13e30
((work_completion)(&adev->reset_work)){+.+.}-{0:0}, at:
process_one_work+0x1ee/0x760
[ +0.000003] #2: ffff897b0fd60ec0 (&reset_domain->sem){++++}-{4:4}, at:
amdgpu_device_lock_reset_domain+0x1c/0x30 [amdgpu]
[ +0.000201]
stack backtrace:
[ +0.000001] CPU: 8 UID: 0 PID: 238 Comm: kworker/u128:1 Tainted: G W
7.2.0+ #97 PREEMPT(full)
[ +0.000002] Tainted: [W]=WARN
[ +0.000000] Hardware name: ASUS System Product Name/ROG MAXIMUS Z790 HERO,
BIOS 2703 10/17/2024
[ +0.000001] Workqueue: amdgpu-reset-dev amdgpu_debugfs_reset_work [amdgpu]
[ +0.000165] Call Trace:
[ +0.000000] <TASK>
[ +0.000001] dump_stack_lvl+0xa5/0x110
[ +0.000002] dump_stack+0x10/0x20
[ +0.000001] print_circular_bug+0x285/0x360
[ +0.000001] check_noncircular+0x14c/0x170
[ +0.000002] __lock_acquire+0x15aa/0x2740
[ +0.000001] ? add_lock_to_list+0x35/0x130
[ +0.000002] lock_acquire+0xc4/0x2f0
[ +0.000001] ? unmap_mapping_pages+0x6a/0xd0
[ +0.000002] ? lock_acquire+0xc4/0x2f0
[ +0.000001] ? amdgpu_device_lock_reset_domain+0x1c/0x30 [amdgpu]
[ +0.000196] down_read+0x30/0x180
[ +0.000001] ? unmap_mapping_pages+0x6a/0xd0
[ +0.000002] unmap_mapping_pages+0x6a/0xd0
[ +0.000002] unmap_mapping_range+0x25/0x40
[ +0.000001] amdgpu_device_gpu_recover+0x17a/0x400 [amdgpu]
[ +0.000163] amdgpu_debugfs_reset_work+0x68/0x90 [amdgpu]
[ +0.000169] process_one_work+0x22e/0x760
[ +0.000003] worker_thread+0x1db/0x3d0
[ +0.000000] ? __pfx_worker_thread+0x10/0x10
[ +0.000001] kthread+0xfe/0x140
[ +0.000001] ? __pfx_kthread+0x10/0x10
[ +0.000001] ret_from_fork+0x3bd/0x470
[ +0.000001] ? __pfx_kthread+0x10/0x10
[ +0.000001] ret_from_fork_asm+0x1a/0x30
[ +0.000002] </TASK>
Use a try-read-lock for eviction unmaps. If reset owns the semaphore,
leave the eviction fence unsignaled and retry the worker after a short
delay. Keep blocking read-locking for other userq unmap paths, which
must remain serialized with reset.
Convert suspend_work to delayed work so retries can be scheduled and
flushed during teardown.
Fixes: aebf196c550e ("drm/amdgpu/userq: wait for GPU reset before mapping or
unmapping queues")
Cc: Christian König <[email protected]>
Cc: Alex Deucher <[email protected]>
Cc: Prike Liang <[email protected]>
Signed-off-by: Vitaly Prosyak <[email protected]>
---
.../drm/amd/amdgpu/amdgpu_eviction_fence.c | 22 ++++++++++----
.../drm/amd/amdgpu/amdgpu_eviction_fence.h | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 30 ++++++++++++-------
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h | 2 +-
4 files changed, 38 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
index a0802012de49..fe45fa164e79 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
@@ -44,7 +44,7 @@ static bool amdgpu_eviction_fence_enable_signaling(struct
dma_fence *f)
{
struct amdgpu_eviction_fence *ev_fence = to_ev_fence(f);
- schedule_work(&ev_fence->evf_mgr->suspend_work);
+ schedule_delayed_work(&ev_fence->evf_mgr->suspend_work, 0);
return true;
}
@@ -58,13 +58,15 @@ static void
amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
{
struct amdgpu_eviction_fence_mgr *evf_mgr =
- container_of(work, struct amdgpu_eviction_fence_mgr,
+ container_of(to_delayed_work(work),
+ struct amdgpu_eviction_fence_mgr,
suspend_work);
struct amdgpu_fpriv *fpriv =
container_of(evf_mgr, struct amdgpu_fpriv, evf_mgr);
struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
struct dma_fence *ev_fence;
bool cookie;
+ int ret;
mutex_lock(&uq_mgr->userq_mutex);
@@ -79,7 +81,14 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct
*work)
cookie = dma_fence_begin_signalling();
ev_fence = amdgpu_evf_mgr_get_fence(evf_mgr);
- amdgpu_userq_evict(uq_mgr);
+ ret = amdgpu_userq_evict(uq_mgr);
+ if (ret == -EAGAIN) {
+ dma_fence_end_signalling(cookie);
+ dma_fence_put(ev_fence);
+ schedule_delayed_work(&evf_mgr->suspend_work, 1);
+ mutex_unlock(&uq_mgr->userq_mutex);
+ return;
+ }
/*
* Signaling the eviction fence must be done while holding the
@@ -167,14 +176,15 @@ void amdgpu_evf_mgr_init(struct amdgpu_eviction_fence_mgr
*evf_mgr)
evf_mgr->ev_fence_ctx = dma_fence_context_alloc(1);
evf_mgr->ev_fence = dma_fence_get_stub();
- INIT_WORK(&evf_mgr->suspend_work, amdgpu_eviction_fence_suspend_worker);
+ INIT_DELAYED_WORK(&evf_mgr->suspend_work,
+ amdgpu_eviction_fence_suspend_worker);
}
void amdgpu_evf_mgr_shutdown(struct amdgpu_eviction_fence_mgr *evf_mgr)
{
evf_mgr->shutdown = true;
/* Make sure that the shutdown is visible to the suspend work */
- flush_work(&evf_mgr->suspend_work);
+ flush_delayed_work(&evf_mgr->suspend_work);
}
void amdgpu_evf_mgr_flush_suspend(struct amdgpu_eviction_fence_mgr *evf_mgr)
@@ -182,7 +192,7 @@ void amdgpu_evf_mgr_flush_suspend(struct
amdgpu_eviction_fence_mgr *evf_mgr)
dma_fence_wait(rcu_dereference_protected(evf_mgr->ev_fence, true),
false);
/* Make sure that we are done with the last suspend work */
- flush_work(&evf_mgr->suspend_work);
+ flush_delayed_work(&evf_mgr->suspend_work);
}
void amdgpu_evf_mgr_fini(struct amdgpu_eviction_fence_mgr *evf_mgr)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
index 2a750add4e7b..d3c3d0769864 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
@@ -43,7 +43,7 @@ struct amdgpu_eviction_fence_mgr {
* Only signaled while holding the userq mutex.
*/
struct dma_fence __rcu *ev_fence;
- struct work_struct suspend_work;
+ struct delayed_work suspend_work;
bool shutdown;
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index ccf0ff490ac2..d652a3a7a2da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -351,16 +351,22 @@ static int amdgpu_userq_restore_helper(struct
amdgpu_usermode_queue *queue)
return r;
}
-static int amdgpu_userq_unmap_helper(struct amdgpu_usermode_queue *queue)
+static int amdgpu_userq_unmap_helper(struct amdgpu_usermode_queue *queue,
+ bool trylock)
{
struct amdgpu_userq_mgr *uq_mgr = queue->userq_mgr;
struct amdgpu_device *adev = uq_mgr->adev;
const struct amdgpu_userq_funcs *userq_funcs =
adev->userq_funcs[queue->queue_type];
- int r;
+ int r = 0;
/* The reset changes the queue state and the MES is unusable meanwhile
*/
- guard(rwsem_read)(&adev->reset_domain->sem);
+ if (trylock) {
+ if (!down_read_trylock(&adev->reset_domain->sem))
+ return -EAGAIN;
+ } else {
+ down_read(&adev->reset_domain->sem);
+ }
if ((queue->state == AMDGPU_USERQ_STATE_MAPPED) ||
(queue->state == AMDGPU_USERQ_STATE_PREEMPTED)) {
trace_amdgpu_userq_state_start(queue);
@@ -369,14 +375,16 @@ static int amdgpu_userq_unmap_helper(struct
amdgpu_usermode_queue *queue)
if (r) {
trace_amdgpu_userq_state_changed(queue,
AMDGPU_USERQ_STATE_HUNG);
queue->state = AMDGPU_USERQ_STATE_HUNG;
- return r;
+ goto unlock;
} else {
trace_amdgpu_userq_state_changed(queue,
AMDGPU_USERQ_STATE_UNMAPPED);
queue->state = AMDGPU_USERQ_STATE_UNMAPPED;
}
}
- return 0;
+unlock:
+ up_read(&adev->reset_domain->sem);
+ return r;
}
static int amdgpu_userq_map_helper(struct amdgpu_usermode_queue *queue)
@@ -563,7 +571,7 @@ amdgpu_userq_destroy(struct amdgpu_userq_mgr *uq_mgr,
struct amdgpu_usermode_que
#if defined(CONFIG_DEBUG_FS)
debugfs_remove_recursive(queue->debugfs_queue);
#endif
- r = amdgpu_userq_unmap_helper(queue);
+ r = amdgpu_userq_unmap_helper(queue, false);
atomic_dec(&uq_mgr->userq_count[queue->queue_type]);
amdgpu_userq_fence_driver_free(queue);
queue->fence_drv = NULL;
@@ -1296,7 +1304,9 @@ amdgpu_userq_evict_all(struct amdgpu_userq_mgr *uq_mgr)
/* Try to unmap all the queues in this process ctx */
xa_for_each(&uq_mgr->userq_xa, queue_id, queue) {
- r = amdgpu_userq_unmap_helper(queue);
+ r = amdgpu_userq_unmap_helper(queue, true);
+ if (r == -EAGAIN)
+ return r;
if (r)
ret = r;
}
@@ -1327,10 +1337,10 @@ amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr
*uq_mgr)
}
}
-void
+int
amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr)
{
- amdgpu_userq_evict_all(uq_mgr);
+ return amdgpu_userq_evict_all(uq_mgr);
}
int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct drm_file
*file_priv,
@@ -1422,7 +1432,7 @@ int amdgpu_userq_suspend(struct amdgpu_device *adev)
if (adev->in_s0ix)
r = amdgpu_userq_preempt_helper(queue);
else
- r = amdgpu_userq_unmap_helper(queue);
+ r = amdgpu_userq_unmap_helper(queue, false);
if (r)
return r;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
index 488dc21d7c81..4dcf6151de6a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
@@ -163,7 +163,7 @@ void amdgpu_userq_mgr_cancel_resume(struct amdgpu_userq_mgr
*userq_mgr);
void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr);
void amdgpu_userq_wait_for_signal(struct amdgpu_userq_mgr *uq_mgr);
-void amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr);
+int amdgpu_userq_evict(struct amdgpu_userq_mgr *uq_mgr);
void amdgpu_userq_ensure_ev_fence(struct amdgpu_userq_mgr *userq_mgr,
struct amdgpu_eviction_fence_mgr *evf_mgr);
--
2.55.0
