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

On Fri, Aug 7, 2026 at 10:11 AM Liang, Prike <[email protected]> wrote:
>
> AMD General
>
> Regards,
>       Prike
>
> > -----Original Message-----
> > From: Alex Deucher <[email protected]>
> > Sent: Friday, August 7, 2026 9:47 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 Fri, Aug 7, 2026 at 9:39 AM Liang, Prike <[email protected]> wrote:
> > >
> > > AMD General
> > >
> > >
> > >
> > > Regards,
> > >       Prike
> > >
> > > > -----Original Message-----
> > > > From: Alex Deucher <[email protected]>
> > > > Sent: Tuesday, August 4, 2026 10:18 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 Tue, Aug 4, 2026 at 4:16 AM Liang, Prike <[email protected]> wrote:
> > > > >
> > > > > 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]>; 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 9:36 AM Liang, Prike <[email protected]>
> > 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 AM Liang, Prike
> > > > > > > > <[email protected]>
> > > > 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 the guilty queue appears in the hang list, or whether
> > > > > > the queue rptr is still updating? 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 queue 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.  Preemption 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
> > > > further 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.
> > >
> > > Regarding the protected fences, do you mean the userq fence that triggers the
> > hang detection work queue?
> > >
> >
> > Yes.
> >
> > > If so, the current implementation calls amdgpu_userq_fence_init() at fence emit
> > time (i.e., at each queue submission), which both initializes the dma_fence structure
> > and starts the hang detection timer for that submission. Once the timeout expires,
> > amdgpu_userq_hang_detect_work() fires and triggers a queue reset regardless of
> > whether the queue is actually hung or simply long-running. But there's a core
> > problem for the kernel to distinguish a hung queue from a slow one at fence emit
> > time, since both look identical an unsignaled fence with an active queue.
> > >
> >
> > That is the intended behavior. If you opt into using a dma fence, you need to abide
> > by its semantics.
> >
> > > However, we can defer that determination to the first timeout expiry in
> > amdgpu_userq_hang_detect_work(). At that point we have more information to work
> > with:
> > >
> > >
> > > Check for real hang: query rptr and rptr has not updated since the fence was
> > emitted, the queue is genuinely stuck (frozen waves, infinite loop). This is a real
> > hang, reset immediately and forcing completing the fences.
> > > Check for forward progress: if rptr has updated but the fence is still unsignaled, the
> > queue is making progress, so it is long running, not hung. In this case:
> > >
> > > Extend the timeout (reschedule hang_detect_work with a longer
> > > interval) Track retry count if the queue still has not completed after N extended
> > cycles, escalate: Preempt the queue, force completing the fence and return -ETIME
> > to userspace then let the application decide: resubmit, split the workload, or drop.
> > >
> > > Avoid unnecessary resets: if preemption succeeds and the queue resumes and
> > completes within the extended window, cancel the reset entirely since the queue
> > was slow, not broken.
> > >
> > >
> > > Do you think this is the right way for handling long-running tasks to resolve the
> > userq timeout issue, or does it align with your idea?
> >
> > If you want to run a super long running job, then you shouldn't use a dma fence in
> > the first place.  There might be a job that takes 2 minutes or even an hour to run, but
> > you can't wait that long for a dma fence.  If you do, the kernel might run out of
> > memory and the OOM killer will start killing processes or you could deadlock.  If you
> > want to run a long job, don't use a dma fence.  If you don't use a dma fence, then the
> > kernel can preempt the queues, do whatever memory management it needs to and
> > then it can resume the queues.  dma fences have to signal in finite time.
>
> Got it. So to skip attaching a dma_fence to a long-running submission, does this require an explicit flag from userspace at queue creation, or is there another mechanism to identify it on the kernel side?
>

Userspace would simply not call the userq signal IOCTL if they want to
run long running jobs.

Alex

> Thanks,
> Prike
> > Alex
> >
> > >
> > > Thanks,
> > > Prike
> > >
> > > Avoid unnecessary resets: if preemption succeeds and the queue resumes
> > > and completes within the extended window, cancel the reset entirely —
> > > the queue was slow, not broken.>
> > > > Alex
> > > >
> > > > > In the longer term, we may need to introduce a more robust
> > > > > mechanism to
> > > > distinguish slow queue cases from genuine hangs, handling them
> > > > appropriately via extending time slice for completing the queue
> > > > submission, userspace driven 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 for 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 reset 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(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 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 == queue_type) {
> > > > > > > > > > > -                   if (uq == guilty_uq)
> > > > > > > > > > > -                           continue;
> > > > > > > > > > >                     if (uq->doorbell_index == db) {
> > > > > > > > > > >                             uq->state = AMDGPU_USERQ_STATE_HUNG;
> > > > > > > > > > >                             if (use_mmio)
> > > > > > > > > > > --
> > > > > > > > > > > 2.34.1
> > > > > > > > >