Quoting Tvrtko Ursulin (2020-07-31 16:06:55)
>
> On 30/07/2020 10:37, Chris Wilson wrote:
> > @@ -191,17 +188,19 @@ static void signal_irq_work(struct irq_work *work)
> > {
> > struct intel_breadcrumbs *b = container_of(work, typeof(*b), irq_work);
> > const ktime_t timestamp = ktime_get();
> > + struct llist_node *signal, *sn;
> > struct intel_context *ce, *cn;
> > struct list_head *pos, *next;
> > - LIST_HEAD(signal);
> > +
> > + signal = NULL;
> > + if (unlikely(!llist_empty(&b->signaled_requests)))
> > + signal = llist_del_all(&b->signaled_requests);
> >
> > spin_lock(&b->irq_lock);
> >
> > - if (list_empty(&b->signalers))
> > + if (!signal && list_empty(&b->signalers))
>
> The only open from previous round was on this change. If I understood
> your previous reply correctly, checking this or not simply controls the
> disarm point and is not related to this patch. With the check added now
> we would disarm later, because even already signaled requests would keep
> it armed. I would prefer this was a separate patch if you could possibly
> be convinced.
I considered that since we add to the lockless list and then queue the
irq work, that is a path that is not driven by the interrupt and so
causing an issue with the idea of the interrupt shadow. Having a simple
test I thought was a positive side-effect to filter out the early
irq_work.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
