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? > 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`. > + > + 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? Thanks, Srini
