Re: [PATCH 3/6] drm/gpu/xe: Ignore dma_fenc_signal() return code

Philipp Stanner
2025-11-27T13:51:52Z

On Thu, 2025-11-27 at 14:37 +0100, Andi Shyti wrote:
> Hi Matt,
>
> On Wed, Nov 26, 2025 at 03:56:32PM -0800, Matthew Brost wrote:
> > On Wed, Nov 26, 2025 at 11:56:57PM +0100, Andi Shyti wrote:
> > > > @@ -85,7 +85,6 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> > > > @@ -93,9 +92,9 @@ void xe_hw_fence_irq_finish(struct xe_hw_fence_irq *irq)
> > > >   spin_lock_irqsave(&irq->lock, flags);
> > > >   list_for_each_entry_safe(fence, next, &irq->pending, irq_link) {
> > > >   list_del_init(&fence->irq_link);
> > > > - err = dma_fence_signal_locked(&fence->dma);
> > >
> > > why don't we do
> > >
> > > XE_WARN_ON(dma_fence_signal_locked(..))

because it's impossible because the series is about removing the return
codes from the dma_fence_signal_* functions.

> > >
> >
> > IIRC the above statement can compile out. So the patch looks correct to me.
>
> you have defined XE_WARN_ON as WARN_ON that should always
> evaluate the content and, depending on the configuration, it
> prints the logs or not.
>
> What I don't like from this patch is that we end up checking
> twice for the DMA_FENCE_FLAG_SIGNALED_BIT bit.

Depends on what you mean by "we". The Xe code checks it only once, with
dma_fence_test_signaled_flag(). The dma_fence backend checks it yet
again, as it always does, to avoid signaling a signaled fence.

That's not racy here, however, because the fence lock is already being
held, as evidenced by the current usage of dma_fence_signal_locked().

P.