On Thu, Nov 27, 2025 at 02:51:39PM +0100, Philipp Stanner wrote:
> On Thu, 2025-11-27 at 14:37 +0100, Andi Shyti wrote:
> > 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.
oh yes, the last patch. Sorry, I went on reviewing and lost the
final target from sight.
> > > >
> > >
> > > 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().
I haven't said it's racy, I just didn't like that we are testing
for the DMA_FENCE_FLAG_SIGNALED_BIT twice. On the other hand,
with dma_fence_signal_locked() being void, I wouldn't know how to
do it better. So that I guess it's fine.
Andi
