AMD General
> -----Original Message-----
> From: SHANMUGAM, SRINIVASAN
> Sent: Tuesday, July 14, 2026 7:43 PM
> To: Koenig, Christian <[email protected]>; Deucher, Alexander
> <[email protected]>
> Cc: [email protected]
> Subject: RE: [PATCH v9 5/5] drm/amdgpu: Record QUEUE_RESET WAIT_EVENT
> notifications
>
>
>
> > -----Original Message-----
> > From: Koenig, Christian <[email protected]>
> > Sent: Tuesday, July 14, 2026 6:36 PM
> > To: SHANMUGAM, SRINIVASAN <[email protected]>;
> Deucher,
> > Alexander <[email protected]>
> > Cc: [email protected]
> > Subject: Re: [PATCH v9 5/5] drm/amdgpu: Record QUEUE_RESET
> WAIT_EVENT
> > notifications
> >
> > On 7/14/26 10:10, Srinivasan Shanmugam wrote:
> > > The queue reset path already resolves the affected queue directly
> > > from the doorbell and funnels successful reset handling through
> > > amdgpu_userq_handle_hung_queue(). Reuse that common handling point
> > > to create the corresponding WAIT_EVENT record.
> > >
> > > Keeping the notification in the common helper ensures that queue
> > > state is updated before userspace is notified and that both EVENTFD
> > > and WAIT_EVENT observe the same reset. It also avoids duplicating
> > > the doorbell lookup or adding a queue scan in the MES reset path.
> > >
> > > No MES or GFX interrupt changes are needed.
> > >
> > > Signed-off-by: Srinivasan Shanmugam <[email protected]>
> > > Reviewed-by: Alex Deucher <[email protected]>
> > > ---
> > > drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 6 ++++++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > > index 28f9d3fe0e80..9e1d03eb31d7 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > > @@ -166,6 +166,7 @@ void amdgpu_userq_handle_hung_queue(struct
> > amdgpu_device *adev,
> > > struct amdgpu_usermode_queue *queue) {
> > > struct amdgpu_eventfd_mgr *eventfd_mgr;
> > > + struct amdgpu_wait_event_mgr *wait_event_mgr;
> > >
> > > queue->state = AMDGPU_USERQ_STATE_HUNG;
> > > atomic_inc(&adev->gpu_reset_counter);
> > > @@ -176,6 +177,11 @@ void amdgpu_userq_handle_hung_queue(struct
> > amdgpu_device *adev,
> > > amdgpu_eventfd_signal(eventfd_mgr,
> > > DRM_AMDGPU_EVENT_TYPE_QUEUE_RESET,
> > > queue);
> > > +
> > > + wait_event_mgr = amdgpu_userq_wait_event_mgr(queue->userq_mgr);
> > > + amdgpu_wait_event_add(wait_event_mgr,
> > > + DRM_AMDGPU_EVENT_TYPE_QUEUE_RESET,
> > > + queue);
> >
> > How are we making sure here that this wait_event_mgr doesn't go out of
> > scope?
>
> Good point. wait_event_mgr is part of the owning amdgpu_fpriv, not the queue
> itself.
> In the reset path, we currently take a reference to the queue, which
> guarantees that
> the queue remains valid while we handle the reset.
> However, that reference does not explicitly guarantee that the owning
> amdgpu_fpriv
> (and therefore wait_event_mgr) is still alive.
> I'll review the lifetime rules and teardown sequence to make sure the manager
> cannot be freed while a reset notification is still in progress.
> If additional synchronization or a reference is needed, I'll include that in
> the next
> revision.
Hi Christian,
Thanks for pointing this out.
I reviewed the lifetime and teardown sequence in more detail. wait_event_mgr is
part of the owning amdgpu_fpriv, so it is not allocated or freed independently.
Its lifetime is tied to the corresponding drm_file.
During file teardown, the USERQ manager and queues are torn down before
amdgpu_wait_event_mgr_fini() is called. Queue teardown removes the producer
lookup paths, and amdgpu_wait_event_mgr_fini() marks the manager dead, removes
pending records, releases their queue references, and wakes blocked waiters
before amdgpu_fpriv is freed.
Based on this teardown ordering, the producer should no longer be able to
access wait_event_mgr after it is finalized, so an additional manager reference
does not appear to be necessary.
I've also documented this ownership and teardown ordering in Patch 2 ("Add
wait-event manager and per-file lifetime plumbing") in
https://patchwork.freedesktop.org/patch/740465/?series=164813&rev=11
Thanks,
Srini
>
> Thanks,
> Srini
>
> >
> > Regards,
> > Christian.
