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

On Wed, Aug 12, 2026 at 5:18 AM Liang, Prike <[email protected]> wrote:
>
> AMD General
>
> Regards,
>       Prike
>
> > -----Original Message-----
> > From: Alex Deucher <[email protected]>
> > Sent: Friday, August 7, 2026 10:38 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 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.
>
> Yes, this approach can also handle long-running jobs submitted from userspace. However, after revisiting the Mesa code, it appears that a fence is always emitted for each userq submission, and I could not find any code for identifying long-running submissions. How should long-running submissions be identified in userspace?

Userspace should not use protected fences in that case.  In theory
compositors that use explicit sync do not need to use the protected
fence.  Someone just needs to handle this case in mesa.

>
> If a long-running submission does not use a protected fence, hang_detect_work should not be scheduled for that submission, and this cleanup patch should not affect the corresponding long-running userq. This should therefore be treated as a separate case requiring further investigation, including how to identify long-running userq submissions and avoid attaching protected fences to them.
>

All we need to do is check for the presence of a fence on the queue
(the application would have called the fence IOCTL).  If the fence is
present, then we'd reset the queue.  If no fence, and MES hang_detect
doesn't detect the queue as hung, then there is no need to reset it.

> Regarding the cleanup, if all hung user queues can be identified through hang_detect_only, do we still need to handle the guilty userq separately? Could we simply reset all guilty queues by traversing the list of hung doorbells, as proposed in this cleanup patch?

If there are no protected fences, then yes, all we need is MES hang detect.

Alex

>
> Regards,
> Prike
>
> > 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
> > > > > > > > > > >