Re: [PATCH 1/2] drm/amdgpu: Remove separate guilty compute userq reset

On Tue, Aug 4, 2026 at 4:16=E2=80=AFAM Liang, Prike <[email protected]> w=
rote:
>
> AMD General
>
> Regards,
>       Prike
>
> > -----Original Message-----
> > From: Alex Deucher <[email protected]>
> > Sent: Monday, August 3, 2026 9:40 PM
> > To: Liang, Prike <[email protected]>
> > Cc: [email protected]; Zhang, Jesse(Jie) <[email protected]=
om>;
> > Liu, Shaoyun <[email protected]>; Deucher, Alexander
> > <[email protected]>; Koenig, Christian <[email protected]=
m>
> > Subject: Re: [PATCH 1/2] drm/amdgpu: Remove separate guilty compute use=
rq reset
> >
> > On Mon, Aug 3, 2026 at 9:36=E2=80=AFAM Liang, Prike <[email protected]=
m> wrote:
> > >
> > > AMD General
> > >
> > > Regards,
> > >       Prike
> > >
> > > > -----Original Message-----
> > > > From: Alex Deucher <[email protected]>
> > > > Sent: Monday, August 3, 2026 9:23 PM
> > > > To: Liang, Prike <[email protected]>
> > > > Cc: [email protected]; Zhang, Jesse(Jie)
> > > > <[email protected]>; Liu, Shaoyun <[email protected]>; Deucher,
> > > > Alexander <[email protected]>; Koenig, Christian
> > > > <[email protected]>
> > > > Subject: Re: [PATCH 1/2] drm/amdgpu: Remove separate guilty compute
> > > > userq reset
> > > >
> > > > On Mon, Aug 3, 2026 at 5:25=E2=80=AFAM Liang, Prike <Prike.Liang@am=
d.com> wrote:
> > > > >
> > > > > AMD General
> > > > >
> > > > > As for the hung userq, it should be identified by the MES reset
> > > > > API with the
> > > > hang_detect_only setting. However, it is unlikely to miss detecting
> > > > the invalid opcode hang case, especially given that the userq
> > > > invalid opcode IGT test has not been implemented yet.
> > > > >
> > > > > Hi @Liu, Shaoyun, are you aware of any known userq hang scenarios
> > > > > that cannot
> > > > be identified by MES API hang_detect_only? If not, could you please
> > > > help review the following patch, which unifies the userq reset path=
 for hung
> > queues?
> > > > >
> > > >
> > > > We added it in the first place to deal with those cases.  There can
> > > > be queues which are not hung, but will never complete and hence
> > > > never signal their fence.  E.g., you can have a queue that is
> > > > waiting on a memory location that MES can preempt, but due to a bug
> > > > elsewhere that memory location will never change so the fence will =
never signal.
> > >
> > > Thank you for the input. However, for fake timeout cases such as the =
long shader
> > scenario, we should identify the fake hang cases by checking whether th=
e guilty
> > queue appears in the hang list, or whether the queue rptr is still upda=
ting? If so,
> > preempt the queue rather than resetting it?
> > >
> >
> > We have to assume that if we end up in the queue reset path that the qu=
eue is hung.
> > The fences have to signal in finite time.  If we preempt the queue that=
 won't signal
> > the fence so we'll just end up in the queue reset path again.  Preempti=
on of a queue
> > that won't make progress only makes sense if fences are not involved.
>
> The most cases relevant to userq fence timeout and reset worker likely to=
 be scheduled when a userq fence polling period expires. If a long running =
shader is detected, the driver may need to try preempting the queue within =
a few retry cycles. If the userq fence remains unsignaled after the retries=
 are exhausted, the driver can either return -ETIME to userspace for furthe=
r handling or fall back to resetting the queue directly. Meanwhile, If the =
preemption succeeds and the queue completes its work during the subsequent =
restore process, no further reset is necessary for the guilty queue?
>

If an application wants to run long running jobs they shouldn't use
protected fences in the first place.  If they don't use protected
fences, then it should behave like KFD queues.  If there is some
operation that needs to happen the queues will get preempted and then
will continue later.  DMA fences need to signal in finite time so we
can't just keep pushing them off.  Other kernel paging operations may
depend on them signalling.

Alex

> In the longer term, we may need to introduce a more robust mechanism to d=
istinguish slow queue cases from genuine hangs, handling them appropriately=
 via extending time slice for completing the queue submission, userspace dr=
iven resubmission, or dropping the work through a queue reset. If it makes =
sense and right way to do, then I will work on implementing the solution fo=
r such slow/long queue cases.
>
> Thanks,
> Prike
>
> > Alex
> >
> > > >
> > > > Alex
> > > >
> > > > > Regards,
> > > > >       Prike
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Liang, Prike
> > > > > > Sent: Thursday, July 23, 2026 2:38 PM
> > > > > > To: [email protected]; Zhang, Jesse(Jie)
> > > > > > <[email protected]>
> > > > > > Cc: Deucher, Alexander <[email protected]>; Koenig,
> > > > > > Christian <[email protected]>
> > > > > > Subject: RE: [PATCH 1/2] drm/amdgpu: Remove separate guilty
> > > > > > compute userq reset
> > > > > >
> > > > > > I checked each different userq hang cases, and the guilty userq
> > > > > > can be identified by the MES firmware and report correctly.
> > > > > > @Zhang,
> > > > > > Jesse(Jie) could you further check as well at you side?
> > > > > >
> > > > > > If there're some hang queues miss identified by MES firmware,
> > > > > > then the correct thing is to further debug from MES firmware
> > > > > > side rather than have such strange reset sequence and this rese=
t
> > > > > > workaround should be
> > > > cleaned sooner or later.
> > > > > >
> > > > > >
> > > > > > Regards,
> > > > > >       Prike
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Liang, Prike <[email protected]>
> > > > > > > Sent: Wednesday, July 15, 2026 2:31 PM
> > > > > > > To: [email protected]
> > > > > > > Cc: Deucher, Alexander <[email protected]>; Koenig,
> > > > > > > Christian <[email protected]>; Liang, Prike
> > > > > > > <[email protected]>
> > > > > > > Subject: [PATCH 1/2] drm/amdgpu: Remove separate guilty
> > > > > > > compute userq reset
> > > > > > >
> > > > > > > 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.
> > > > > > >
> > > > > > > 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 1e275c2e7dd3..4f2d5ff2f7be 100644
> > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > > > > > > @@ -2315,11 +2315,6 @@ int amdgpu_gfx_reset_mes_compute(struc=
t
> > > > > > > amdgpu_device *adev,
> > > > > > >             deferred_end[n_deferred].fence =3D guilty_fence;
> > > > > > >             n_deferred++;
> > > > > > >     }
> > > > > > > -   if (uq) {
> > > > > > > -           r =3D mes_userq_reset(uq);
> > > > > > > -           if (r)
> > > > > > > -                   goto out;
> > > > > > > -   }
> > > > > > >     for (i =3D 0; i < num_hung; i++) {
> > > > > > >             struct amdgpu_ring *hr =3D NULL;
> > > > > > >             struct amdgpu_fence *hf =3D NULL; diff --git
> > > > > > > a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > > > > > > b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > > > > > > index b6bfa3974839..fab21d4275f3 100644
> > > > > > > --- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
> > > > > > > @@ -226,8 +226,6 @@ int mes_userq_reset_queue(struct
> > > > > > > amdgpu_device *adev,
> > > > > > >
> > > > > > >     xa_for_each(&adev->userq_doorbell_xa, uq_id, uq) {
> > > > > > >             if (uq->queue_type =3D=3D queue_type) {
> > > > > > > -                   if (uq =3D=3D guilty_uq)
> > > > > > > -                           continue;
> > > > > > >                     if (uq->doorbell_index =3D=3D db) {
> > > > > > >                             uq->state =3D AMDGPU_USERQ_STATE_=
HUNG;
> > > > > > >                             if (use_mmio)
> > > > > > > --
> > > > > > > 2.34.1
> > > > >