On Tue, Jul 14, 2026 at 11:19 AM SHANMUGAM, SRINIVASAN
<[email protected]> wrote:
>
> AMD General
>
> Hi Alex,
>
> > -----Original Message-----
> > From: Alex Deucher <[email protected]>
> > Sent: Tuesday, July 14, 2026 8:10 PM
> > To: [email protected]
> > Cc: Deucher, Alexander <[email protected]>; SHANMUGAM,
> > SRINIVASAN <[email protected]>; Liang, Prike
> > <[email protected]>; Khatri, Sunil <[email protected]>; Lazar, Lijo
> > <[email protected]>
> > Subject: [PATCH] drm/amdgpu: properly account for resets with user queues
> >
> > We need to increment the reset counter, force fence completion, and set the
> > wedged event when a user queue is reset, but only for the guilty queue. We
> > don't
> > want additional events for collateral damage.
> >
> > Only increment the reset counter and set the wedged event in the top level
> > userq
> > reset callers (both KGD and KFD).
> > This aligns with how this is managed for kernel queue resets.
> >
> > Signed-off-by: Alex Deucher <[email protected]>
> > Cc: Srinivasan Shanmugam <[email protected]>
> > Cc: Prike Liang <[email protected]>
> > Cc: Sunil Khatri <[email protected]>
> > Cc: Lazar, Lijo <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 17 ++++++++++++++++-
> > drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 2 --
> > .../drm/amd/amdkfd/kfd_device_queue_manager.c | 14 ++++++++++++++
> > 3 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > index 6aa75da27f912..0a0ebcdc271f8 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > @@ -123,6 +123,8 @@ static void amdgpu_userq_hang_detect_work(struct
> > work_struct *work)
> > struct amdgpu_device *adev = uq_mgr->adev;
> > const struct amdgpu_userq_funcs *userq_funcs =
> > adev->userq_funcs[queue->queue_type];
> > + struct drm_wedge_task_info *info = NULL;
> > + struct amdgpu_task_info *ti = NULL;
> > bool gpu_reset = false;
> >
> > if (unlikely(adev->debug_disable_gpu_ring_reset)) { @@ -137,6 +139,14
> > @@ static void amdgpu_userq_hang_detect_work(struct work_struct *work)
> > if (!amdgpu_gpu_recovery)
> > return;
> >
> > + if (queue->vm && queue->vm->pasid) {
> > + ti = amdgpu_vm_get_task_info_pasid(adev, queue->vm->pasid);
> > + if (ti) {
> > + amdgpu_vm_print_task_info(adev, ti);
> > + info = &ti->task;
> > + }
> > + }
> > +
>
> `amdgpu_vm_get_task_info_pasid()` returns a referenced
> `amdgpu_task_info`, and the existing `amdgpu_job_timedout()` path pairs
> it with `amdgpu_vm_put_task_info(ti)` on the exit path.
>
> Since `info` points into `ti`, should we keep the reference until after
> `drm_dev_wedged_event()` and then add the matching
> `amdgpu_vm_put_task_info(ti)` on the common exit path here?
Good catch. Fixed.
>
>
> > if (amdgpu_userq_is_reset_type_supported(adev, queue->queue_type,
> >
> > AMDGPU_RESET_TYPE_PER_QUEUE)) {
> > int r;
> > @@ -146,8 +156,13 @@ static void amdgpu_userq_hang_detect_work(struct
> > work_struct *work)
> > queue, NULL, NULL);
> > else
> > r = userq_funcs->reset(queue);
> > - if (r)
> > + if (r) {
> > gpu_reset = true;
> > + } else {
> > + atomic_inc(&adev->gpu_reset_counter);
> > + amdgpu_userq_fence_driver_force_completion(queue);
> > + drm_dev_wedged_event(adev_to_drm(adev),
> > DRM_WEDGE_RECOVERY_NONE, info);
> > + }
> > } else {
> > gpu_reset = true;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > index d3d79bbaf9eb7..a1371a9f961d1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > @@ -234,9 +234,7 @@ int mes_userq_reset_queue(struct amdgpu_device *adev,
> > r = mes_userq_unmap(uq);
> > if (r)
> > return r;
> > - atomic_inc(&adev->gpu_reset_counter);
> >
> > amdgpu_userq_fence_driver_force_completion(uq);
> > - drm_dev_wedged_event(adev_to_drm(adev),
> > DRM_WEDGE_RECOVERY_NONE, NULL);
> > break;
> > }
> > }
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > index 0419fe869d4b0..0ae2d951bd21e 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> > @@ -448,6 +448,9 @@ int kfd_reset_queue_mes(struct device_queue_manager
> > *dqm, int queue_type, static int reset_queues_mes(struct
> > device_queue_manager
> > *dqm, struct queue *q) {
> > struct amdgpu_device *adev = (struct amdgpu_device *)dqm->dev->adev;
> > + struct drm_wedge_task_info *info = NULL;
> > + struct amdgpu_task_info *ti = NULL;
> > + struct kfd_process_device *pdd;
> > unsigned int num_hung = 0;
> > int r = 0;
> > struct mes_remove_queue_input queue_input; @@ -476,6 +479,17 @@
> > static int reset_queues_mes(struct device_queue_manager *dqm, struct queue
> > *q)
> > r = amdgpu_gfx_reset_mes_compute(adev, NULL, NULL, NULL,
> > &num_hung, &queue_input);
> > if (r)
> > goto fail;
> > + pdd = kfd_get_process_device_data(q->device, q->process);
> > + if (pdd) {
> > + ti = amdgpu_vm_get_task_info_pasid(adev, pdd->pasid);
> > + if (ti) {
> > + amdgpu_vm_print_task_info(adev, ti);
> > + info = &ti->task;
> > + }
> > + }
>
> Same here: the task-info reference appears to need a matching
> `amdgpu_vm_put_task_info(ti)` after the last use of `info`.
Fixed.
>
> > +
> > + atomic_inc(&adev->gpu_reset_counter);
> > + drm_dev_wedged_event(adev_to_drm(adev),
> > DRM_WEDGE_RECOVERY_NONE,
> > +info);
>
> The existing KFD reset event is conditioned on num_hung, while
> amdgpu_gfx_reset_mes_compute() can also handle the known bad queue through
> queue_input when num_hung == 0.
>
> Could you pls clarify what condition should determine whether the reset
> counter and wedged event are emitted in this path? Should successful removal
> of the known bad queue also count as the recovery event?
Yes, we should align on whether a bad queue was found or not. Fixed.
Thanks!
Alex
>
> Thanks,
> Srini
