[PATCH 9/9] rcu-signal
Chris Wilson
chris at chris-wilson.co.uk
Sat Aug 8 17:56:40 UTC 2020
---
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 101 +++++-------------
drivers/gpu/drm/i915/gt/intel_context.c | 1 -
drivers/gpu/drm/i915/gt/intel_context_types.h | 3 +-
drivers/gpu/drm/i915/i915_request.h | 5 +-
4 files changed, 33 insertions(+), 77 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index a0cfb036c6ef..29a063ad7c6c 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -114,22 +114,23 @@ static void add_signaling_context(struct intel_breadcrumbs *b,
{
lockdep_assert_held(&ce->signal_lock);
lockdep_assert_held(&b->signalers_lock);
- GEM_BUG_ON(ce->signaler != NULL);
- ce->signaler = b;
list_add_rcu(&ce->signal_link, &b->signalers);
}
-static void remove_signaling_context(struct intel_breadcrumbs *b,
+static bool remove_signaling_context(struct intel_breadcrumbs *b,
struct intel_context *ce)
{
lockdep_assert_held(&ce->signal_lock);
- GEM_BUG_ON(ce->signaler != b);
+
+ if (!list_empty(&ce->signals))
+ return false;
spin_lock(&b->signalers_lock);
list_del_rcu(&ce->signal_link);
- ce->signaler = NULL;
spin_unlock(&b->signalers_lock);
+
+ return true;
}
static inline bool __request_completed(const struct i915_request *rq)
@@ -248,67 +249,37 @@ static void signal_irq_work(struct irq_work *work)
rcu_read_lock();
list_for_each_entry_rcu(ce, &b->signalers, signal_link) {
- struct list_head *pos, *next;
- bool release = false;
+ struct i915_request *rq;
- /*
- * Since the signal_irq_worker may in fact be running on
- * different cores simultaneously, just skip over the other
- * thread if it is in the middle of processing the signals.
- * We will then advance to the next context, and the other
- * core will then skip over us, splitting the work between
- * the multiple cores that just happen to processing the
- * worker simultaneously.
- *
- * The other use of ce->signal_lock to add new requests
- * to be signaled will always kick the worker afterwards
- * ensuring that ce->signals will always be processed at
- * least once (even after being skipped).
- */
- if (!spin_trylock(&ce->signal_lock))
- continue;
+ list_for_each_entry_rcu(rq, &ce->signals, signal_link) {
+ bool release;
- if (ce->signaler != b) /* Transfered to a new engine/owner? */
- goto unlock;
-
- list_for_each_safe(pos, next, &ce->signals) {
- struct i915_request *rq =
- list_entry(pos, typeof(*rq), signal_link);
-
- GEM_BUG_ON(!check_signal_order(ce, rq));
if (!__request_completed(rq))
break;
+ if (!test_and_clear_bit(I915_FENCE_FLAG_SIGNAL,
+ &rq->fence.flags))
+ continue;
+
/*
* Queue for execution after dropping the signaling
* spinlock as the callback chain may end up adding
* more signalers to the same context or engine.
*/
- clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
+ spin_lock(&ce->signal_lock);
+ list_del_rcu(&rq->signal_link);
+ release = remove_signaling_context(b, ce);
+ spin_unlock(&ce->signal_lock);
+
if (__signal_request(rq))
/* We own signal_node now, xfer to local list */
signal = slist_add(&rq->signal_node, signal);
- }
- /*
- * We process the list deletion in bulk, only using a list_add
- * (not list_move) above but keeping the status of
- * rq->signal_link known with the I915_FENCE_FLAG_SIGNAL bit.
- */
- if (!list_is_first(pos, &ce->signals)) {
- /* Advance the list to the first incomplete request */
- __list_del_many(&ce->signals, pos);
- if (&ce->signals == pos) { /* now empty */
+ if (release) {
add_retire(b, ce->timeline);
- remove_signaling_context(b, ce);
- release = true;
+ intel_context_put(ce);
}
}
-
-unlock:
- spin_unlock(&ce->signal_lock);
- if (release)
- intel_context_put(ce);
}
rcu_read_unlock();
@@ -464,7 +435,7 @@ static void insert_breadcrumb(struct i915_request *rq)
break;
}
}
- list_add(&rq->signal_link, pos);
+ list_add_rcu(&rq->signal_link, pos);
GEM_BUG_ON(!check_signal_order(ce, rq));
set_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
@@ -504,26 +475,19 @@ bool i915_request_enable_breadcrumb(struct i915_request *rq)
void i915_request_cancel_breadcrumb(struct i915_request *rq)
{
struct intel_context *ce = rq->context;
- bool release = false;
+ bool release;
- if (!test_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags))
+ if (!test_and_clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags))
return;
spin_lock(&ce->signal_lock);
- if (test_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags)) {
- clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
-
- list_del(&rq->signal_link);
- if (list_empty(&ce->signals)) {
- remove_signaling_context(rq->engine->breadcrumbs, ce);
- release = true;
- }
-
- i915_request_put(rq);
- }
+ list_del_rcu(&rq->signal_link);
+ release = remove_signaling_context(rq->engine->breadcrumbs, ce);
spin_unlock(&ce->signal_lock);
if (release)
intel_context_put(ce);
+
+ i915_request_put(rq);
}
static void print_signals(struct intel_breadcrumbs *b, struct drm_printer *p)
@@ -535,22 +499,13 @@ static void print_signals(struct intel_breadcrumbs *b, struct drm_printer *p)
rcu_read_lock();
list_for_each_entry_rcu(ce, &b->signalers, signal_link) {
- spin_lock_irq(&ce->signal_lock);
-
- if (ce->signaler != b)
- goto unlock;
-
- list_for_each_entry(rq, &ce->signals, signal_link) {
+ list_for_each_entry_rcu(rq, &ce->signals, signal_link)
drm_printf(p, "\t[%llx:%llx%s] @ %dms\n",
rq->fence.context, rq->fence.seqno,
i915_request_completed(rq) ? "!" :
i915_request_started(rq) ? "*" :
"",
jiffies_to_msecs(jiffies - rq->emitted_jiffies));
- }
-
-unlock:
- spin_unlock_irq(&ce->signal_lock);
}
rcu_read_unlock();
}
diff --git a/drivers/gpu/drm/i915/gt/intel_context.c b/drivers/gpu/drm/i915/gt/intel_context.c
index 960c5f4ee2ff..c3869f2dfdfb 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -273,7 +273,6 @@ static void __intel_context_ctor(void *arg)
/* NB ce->signal_link/lock is used under RCU */
spin_lock_init(&ce->signal_lock);
INIT_LIST_HEAD(&ce->signals);
- ce->signaler = NULL;
atomic_set(&ce->pin_count, 0);
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 8c756413d92b..79a549face8a 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -64,10 +64,9 @@ struct intel_context {
* we add the context to the breadcrumbs worker, and remove it
* upon completion/cancellation of the last request.
*/
- spinlock_t signal_lock; /* Taken under RCU */
- struct intel_breadcrumbs *signaler; /* Confirms ownership during RCU */
struct list_head signal_link; /* Accessed under RCU */
struct list_head signals; /* Guarded by signal_lock */
+ spinlock_t signal_lock; /* protects signals, the list of requests */
struct i915_vma *state;
struct intel_ring *ring;
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 874af6db6103..94eb67ad43a1 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -179,7 +179,10 @@ struct i915_request {
union {
struct list_head signal_link;
- struct llist_node signal_node;
+ struct {
+ struct list_head *signal_rcu;
+ struct llist_node signal_node;
+ };
};
/*
--
2.20.1
More information about the Intel-gfx-trybot
mailing list