AMD General
Regards,
Prike
> -----Original Message-----
> From: Alex Deucher <[email protected]>
> Sent: Friday, September 4, 2026 3:28 AM
> To: Liang, Prike <[email protected]>
> Cc: [email protected]; Deucher, Alexander
> <[email protected]>; Koenig, Christian <[email protected]>;
> Prosyak, Vitaly <[email protected]>
> Subject: Re: [PATCH 01/18] drm/amdgpu: Remove separate guilty compute userq
> reset
>
> On Wed, Sep 2, 2026 at 8:50 AM Prike Liang <[email protected]> wrote:
> >
> > amdgpu_mes_detect_and_reset_hung_queues() already detects the guilty
> > compute user queue and resets it through mes_userq_reset_queue(). The
> > additional reset via
> > mes_userq_reset() is unnecessary, so remove it to unify the compute
> > userq reset.
>
> The problem is that amdgpu_mes_detect_and_reset_hung_queues() won't reset the
> queue in some cases. Detect_and_reset() attempts to preempt the queues and if
> they fail to preempt they are considered hung, however, there are queues
> which can
> be preempted which are in a state which won't make progress so the protected
> fence
> will never signal.
> That's why we have this special case.
Thank you for the detailed background. Regarding
amdgpu_mes_detect_and_reset_hung_queues() in amdgpu_gfx_reset_mes_compute(),
this function is only responsible for detecting hung queues, not resetting
them. On the MES firmware side, a hung queue is identified by reading and
comparing the MQD status over a query time span. Because of this, there is a
possibility that a spurious hang detection gets scheduled, triggered by a userq
signaling timeout. Such timeout cases may originate from a long-running
submission or a slow, heavy queue execution. For these false positive timeout
scenarios, we could consider preempting and restoring the userq rather than
blindly issuing a reset or alternatively, having userspace skip emitting a
tracked fence, as you proposed in a separate review thread.
I will investigate further and draft a proper solution for handling these false
timeout cases. Until that is finalized, I will drop this patch.
Regards,
Prike
> Alex
>
> >
> > Signed-off-by: Prike Liang <[email protected]>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 5 -----
> > drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 2 --
> > 2 files changed, 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index a6f95ff47d24..5c3be851ac84 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -2384,11 +2384,6 @@ int amdgpu_gfx_reset_mes_compute(struct
> amdgpu_device *adev,
> > deferred_end[n_deferred].fence = guilty_fence;
> > n_deferred++;
> > }
> > - if (uq) {
> > - r = mes_userq_reset(uq);
> > - if (r)
> > - goto out;
> > - }
> > for (i = 0; i < num_hung; i++) {
> > struct amdgpu_ring *hr = NULL;
> > struct amdgpu_fence *hf = NULL; diff --git
> > a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > index 7f334f718cd8..83a438d8e117 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > @@ -249,8 +249,6 @@ int mes_userq_reset_queue(struct amdgpu_device
> > *adev,
> >
> > xa_for_each(&adev->userq_doorbell_xa, uq_id, uq) {
> > if (uq->queue_type == queue_type) {
> > - if (uq == guilty_uq)
> > - continue;
> > if (uq->doorbell_index == db) {
> > uq->state = AMDGPU_USERQ_STATE_HUNG;
> > if (use_mmio)
> > --
> > 2.34.1
> >
