Quoting Tvrtko Ursulin (2020-08-07 12:26:41) > > On 07/08/2020 09:32, Chris Wilson wrote: > > 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); > > @@ -242,7 +248,9 @@ 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); > > + clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags); > > + if (__signal_request(rq)) > > + signal = __llist_add(&rq->signal_node, signal); > > Presumably here you count on no possible races, allowing for a more > optimized, custom, __llist_add. It needs a comment at minimum, or even > better just use llist_add. It's a purely local singly linked list here. We own the request->signal_node as that is locked by the b->irq_lock and the clear_bit, and signal is a local variable. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx