Re: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the end of a multi ring reset

["Timur Kristóf" <timur.kristof@gmail.com>]

On Wed, 2026-11-04 at 02:46 +0000, Zhang, Jesse(Jie) wrote:
> > > +           r = amdgpu_ring_reset_helper_end(ring, ring ==
> > > guilty_ring ?
> > > +                                            guilty_fence :
> > > ring->guilty_fence);
> > 
> > Can you help me understand what was wrong with the line before?
> > I think guilty_fence == ring->guilty_fence when ring == guilty_ring
> > no?
> 
> Usually yes, but not when the soft reset follows a failed ring reset
> of the
> same job.
>  amdgpu_ring_reset(ring, vmid, job->hw_fence)
>    -> amdgpu_ring_reset_helper_begin(ring, F)
>    -> amdgpu_ring_backup_unprocessed_commands(ring, F)
>       sets ring->guilty_fence = F.
> 
> So by the time the end helper runs, ring->guilty_fence no longer
> equals the
> guilty fence: the begin helper cleared it.

I think I see the problem.

I re-read the queue reset code and the helpers. During a queue reset,
it makes a backup and saves the guilty fence but if the queue reset
fails, it doesn't restore the contents, and on a subsequent call it
clears the guilty fence and the backed up contents.

Unfortunately I think this commit doesn't completely fix that because
the reset begin helper will still clear the backed up contents on the
second call (sets ring_backup_entries_to_copy = 0).

I think the correct fix would be to make sure we don't repeat the ring
reset begin helper:
https://gitlab.freedesktop.org/Venemo/linux/-/commit/86f2bf222caee0da267f9d9e12362513fe4f7f9e
Please take a look and if you agree feel free to take that patch in
your series.

Timur