Quoting Tvrtko Ursulin (2020-07-22 14:05:35)
> 
> On 20/07/2020 10:23, Chris Wilson wrote:
> > @@ -198,7 +217,8 @@ static void signal_irq_work(struct irq_work *work)
> >                        * spinlock as the callback chain may end up adding
> >                        * more signalers to the same context or engine.
> >                        */
> > -                     __signal_request(rq, &signal);
> > +                     if (!__signal_request(rq, &signal))
> > +                             i915_request_put(rq);
> 
> Looks like __signal_request could do the request put but doesn't matter 
> hugely.

I ditch the __signal_request() wrapper as the two callers diverge a bit
more.

1:
        clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
        if (__dma_fence_signal(&rq->fence)) {
                rq->signal_node.next = signal;
                signal = &rq->signal_node;
        } else {
                i915_request_put(rq);
        }

2:

        if (__request_completed(rq)) {
                if (__dma_fence_signal(&rq->fence)) {
                        if (llist_add(&rq->signal_node, &b->signaled_requests))
                                irq_work_queue(&b->irq_work);
                } else {
                        i915_request_put(rq);
                }
                return;
        }

Not completely sold on that though. Keeping the i915_request_put() as
part of __signal_request() would remove the duplicate line there.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to