On Fri, Aug 21, 2026 at 5:09 AM Liang, Prike <[email protected]> wrote: > > AMD General > > Regards, > Prike > > > -----Original Message----- > > From: Alex Deucher <[email protected]> > > Sent: Wednesday, August 19, 2026 11:52 PM > > To: Liang, Prike <[email protected]> > > Cc: [email protected]; Deucher, Alexander > > <[email protected]>; Koenig, Christian <[email protected]> > > Subject: Re: [PATCH v2] drm/amdgpu/userq: fix lock missing for userq fence > > error > > set > > > > On Thu, Aug 13, 2026 at 11:50 PM Prike Liang <[email protected]> wrote: > > > > > > amdgpu_userq_fence_driver() and amdgpu_userq_fence_driver_destroy() > > > don't acquire the dma_fence spinlock, so locking the dma_fence lock > > > before test the signaled state and set error state. > > > > > > Signed-off-by: Prike Liang <[email protected]> > > > --- > > > .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 20 ++++++++++++------- > > > 1 file changed, 13 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > > > index 19af1adc2921..09e2079cfdbd 100644 > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c > > > @@ -191,14 +191,15 @@ void amdgpu_userq_fence_driver_destroy(struct kref > > *ref) > > > struct dma_fence *f; > > > > > > spin_lock_irqsave(&fence_drv->fence_list_lock, flags); > > > + lockdep_assert_held(&fence_drv->fence_list_lock); > > > list_for_each_entry_safe(fence, tmp, &fence_drv->fences, link) { > > > f = &fence->base; > > > - > > > - if (!dma_fence_is_signaled(f)) { > > > + spin_lock(dma_fence_spinlock(f)); > > > + if (!dma_fence_is_signaled_locked(f)) { > > > dma_fence_set_error(f, -ECANCELED); > > > - dma_fence_signal(f); > > > + dma_fence_signal_locked(f); > > > } > > > - > > > + spin_unlock(dma_fence_spinlock(f)); > > > list_del(&fence->link); > > > dma_fence_put(f); > > > } > > > @@ -423,11 +424,16 @@ amdgpu_userq_fence_driver_set_error(struct > > amdgpu_userq_fence *fence, > > > struct dma_fence *f; > > > > > > spin_lock_irqsave(&fence_drv->fence_list_lock, flags); > > > - > > > + lockdep_assert_held(&fence_drv->fence_list_lock); > > > f = rcu_dereference_protected(&fence->base, > > > > > > lockdep_is_held(&fence_drv->fence_list_lock)); > > > - if (f && !dma_fence_is_signaled_locked(f)) > > > - dma_fence_set_error(f, error); > > > + if (f) { > > > + /* nest f->lock inside fence_list_lock */ > > > + spin_lock(dma_fence_spinlock(f)); > > > > Why do we need to take the fence spinlock for userqs, but not for kernel > > queues? > > See amdgpu_fence.c > > For the kernel queue ring fence, we also need to hold the dma_fence lock when > calling dma_fence_is_signaled_locked(). The reason that does not assert the > lock missing for the kernel ring fence is the .signaled() callback is not > implemented, so dma_fence_signal_locked() will never be invoked. >
Sounds like we need to fix up the kernel queue fences as well. Acked-by: Alex Deucher <[email protected]> > Thanks, > Prike > > > Alex > > > > > + if (!dma_fence_is_signaled_locked(f)) > > > + dma_fence_set_error(f, error); > > > + spin_unlock(dma_fence_spinlock(f)); > > > + } > > > spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags); } > > > > > > -- > > > 2.34.1 > > >
