On 10/30/26 04:27, [email protected] wrote:
> 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.
That is definately a good catch, but at least of hand it looks like the wrong
approach to solving this.
That the eviction fence worker blocks for the reset to finish is correct and
should happen like this.
The problem looks more like we are trying to unmap something from the reset and
that is clearly not allowed when it needs to acquire the mmap semaphore.
Regards,
Christian.
>
> 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);
