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.
