Re: [PATCH 02/11] drm/amdgpu: remove deadlocks from amdgpu_userq_pre_reset

["Khatri, Sunil" <sukhatri@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]


On 21-04-2026 06:25 pm, Christian König wrote:
> The purpose of a GPU reset is to make sure that fence can be signaled
> again and the signal and resume workers can make progress again.
>
> So waiting for the resume worker or any fence in the GPU reset path is
> just utterly nonsense.
>
> Signed-off-by: Christian König<christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 26 +++++++++++------------
>   1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 8f48520cb822..b632bc3c952b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1496,23 +1496,21 @@ void amdgpu_userq_pre_reset(struct amdgpu_device *adev)
>   {
>   	const struct amdgpu_userq_funcs *userq_funcs;
>   	struct amdgpu_usermode_queue *queue;
> -	struct amdgpu_userq_mgr *uqm;
>   	unsigned long queue_id;
>   
> +	/* TODO: We probably need a new lock for the queue state */
>   	xa_for_each(&adev->userq_doorbell_xa, queue_id, queue) {
> -		uqm = queue->userq_mgr;
> -		cancel_delayed_work_sync(&uqm->resume_work);
> -		if (queue->state == AMDGPU_USERQ_STATE_MAPPED) {
> -			amdgpu_userq_wait_for_last_fence(queue);
> -			userq_funcs = adev->userq_funcs[queue->queue_type];
> -			userq_funcs->unmap(queue);
> -			/* just mark all queues as hung at this point.
> -			 * if unmap succeeds, we could map again
> -			 * in amdgpu_userq_post_reset() if vram is not lost
> -			 */
> -			queue->state = AMDGPU_USERQ_STATE_HUNG;
> -			amdgpu_userq_fence_driver_force_completion(queue);
> -		}
> +		if (queue->state != AMDGPU_USERQ_STATE_MAPPED)
> +			continue;

If the queue is in prempt state and if at that time we are in this 
function we should still be doing force completion for work in those 
queue else the waiters will keep waiting.

> +
> +		userq_funcs = adev->userq_funcs[queue->queue_type];
> +		userq_funcs->unmap(queue);
GPU is already hung if we are here and observation is we are unable to 
unmap as we have tried to reset via the fw and that failed to atleast 
thats what i have seen. Could we skip unmap ???
> +		/* just mark all queues as hung at this point.
> +		 * if unmap succeeds, we could map again
> +		 * in amdgpu_userq_post_reset() if vram is not lost
> +		 */
> +		queue->state = AMDGPU_USERQ_STATE_HUNG;
> +		amdgpu_userq_fence_driver_force_completion(queue);

we should be calling completion irrespective of queue state here. The 
GPU atleast the queue is hung and fw has failed to reset. We have to 
release the fences by foce completion.

Regards
Sunil Khatri

>   	}
>   }
>   

[-- Attachment #2: Type: text/html, Size: 3612 bytes --]