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?

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.

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?

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
> > > > > > > > > >

Reply via email to