On 07/08/2020 12:54, Chris Wilson wrote:
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.
True.. just a comment then please and with that:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx