[PATCH 8/8] signlock
Chris Wilson
chris at chris-wilson.co.uk
Sat Jul 18 16:13:21 UTC 2020
---
drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 133 ++++++++++--------
drivers/gpu/drm/i915/gt/intel_context.c | 1 +
drivers/gpu/drm/i915/gt/intel_context_types.h | 1 +
3 files changed, 74 insertions(+), 61 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index be6a2fcde4f0..b0937baaf64c 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -143,21 +143,24 @@ 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;
+ struct intel_context *ce;
signal = NULL;
if (!llist_empty(&b->signaled_requests))
signal = llist_del_all(&b->signaled_requests);
- spin_lock(&b->irq_lock);
-
- if (b->irq_armed && list_empty(&b->signalers))
- __intel_breadcrumbs_disarm_irq(b);
+ if (list_empty(&b->signalers) && READ_ONCE(b->irq_armed)) {
+ spin_lock(&b->irq_lock);
+ if (b->irq_armed)
+ __intel_breadcrumbs_disarm_irq(b);
+ spin_unlock(&b->irq_lock);
+ }
- list_for_each_entry_safe(ce, cn, &b->signalers, signal_link) {
- GEM_BUG_ON(list_empty(&ce->signals));
+ rcu_read_lock();
+ list_for_each_entry_rcu(ce, &b->signalers, signal_link) {
+ struct list_head *pos, *next;
+ spin_lock(&ce->signal_lock);
list_for_each_safe(pos, next, &ce->signals) {
struct i915_request *rq =
list_entry(pos, typeof(*rq), signal_link);
@@ -190,14 +193,17 @@ static void signal_irq_work(struct irq_work *work)
/* Advance the list to the first incomplete request */
__list_del_many(&ce->signals, pos);
if (&ce->signals == pos) { /* now empty */
- list_del_init(&ce->signal_link);
+ spin_lock(&b->irq_lock);
+ list_del_rcu(&ce->signal_link);
+ spin_unlock(&b->irq_lock);
add_retire(b, ce->timeline);
intel_context_put(ce);
}
}
- }
- spin_unlock(&b->irq_lock);
+ spin_unlock(&ce->signal_lock);
+ }
+ rcu_read_unlock();
llist_for_each_safe(signal, sn, signal) {
struct i915_request *rq =
@@ -289,8 +295,7 @@ void intel_breadcrumbs_free(struct intel_breadcrumbs *b)
kfree(b);
}
-static void insert_breadcrumb(struct i915_request *rq,
- struct intel_breadcrumbs *b)
+static void insert_breadcrumb(struct i915_request *rq)
{
struct intel_context *ce = rq->context;
struct list_head *pos;
@@ -307,6 +312,9 @@ static void insert_breadcrumb(struct i915_request *rq,
*/
if (__request_completed(rq)) {
if (__dma_fence_signal(&rq->fence)) {
+ struct intel_breadcrumbs *b =
+ READ_ONCE(rq->engine)->breadcrumbs;
+
if (llist_add(&rq->signal_node, &b->signaled_requests))
irq_work_queue(&b->irq_work);
} else {
@@ -315,8 +323,6 @@ static void insert_breadcrumb(struct i915_request *rq,
return;
}
- __intel_breadcrumbs_arm_irq(b);
-
/*
* We keep the seqno in retirement order, so we can break
* inside intel_engine_signal_breadcrumbs as soon as we've
@@ -340,8 +346,40 @@ static void insert_breadcrumb(struct i915_request *rq,
}
list_add(&rq->signal_link, pos);
if (pos == &ce->signals) { /* catch transitions from empty list */
+ struct intel_breadcrumbs *b;
+
intel_context_get(ce);
- list_move_tail(&ce->signal_link, &b->signalers);
+
+ /*
+ * rq->engine is locked by rq->engine->active.lock. That
+ * however is not known until after rq->engine has been
+ * dereferenced and the lock acquired. Hence we acquire the
+ * lock and then validate that rq->engine still matches the
+ * lock we hold for it.
+ *
+ * Here, we are using the breadcrumb lock as a proxy for the
+ * rq->engine->active.lock, and we know that since the
+ * breadcrumb will be serialised within i915_request_submit
+ * the engine cannot change while active as long as we hold
+ * the breadcrumb lock on that engine.
+ *
+ * From the dma_fence_enable_signaling() path, we are outside
+ * of the request submit/unsubmit path, and so we must be more
+ * careful to acquire the right lock.
+ */
+ b = READ_ONCE(rq->engine)->breadcrumbs;
+ spin_lock(&b->irq_lock);
+ while (unlikely(b != READ_ONCE(rq->engine)->breadcrumbs)) {
+ spin_unlock(&b->irq_lock);
+ b = READ_ONCE(rq->engine)->breadcrumbs;
+ spin_lock(&b->irq_lock);
+ }
+
+ list_add_rcu(&ce->signal_link, &b->signalers);
+ __intel_breadcrumbs_arm_irq(b);
+
+ spin_unlock(&b->irq_lock);
+
irq_work_queue(&b->irq_work); /* check after enabling irq */
}
GEM_BUG_ON(!check_signal_order(ce, rq));
@@ -351,7 +389,7 @@ static void insert_breadcrumb(struct i915_request *rq,
bool i915_request_enable_breadcrumb(struct i915_request *rq)
{
- struct intel_breadcrumbs *b;
+ struct intel_context *ce = rq->context;
/* Serialises with i915_request_retire() using rq->lock */
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &rq->fence.flags))
@@ -366,48 +404,17 @@ bool i915_request_enable_breadcrumb(struct i915_request *rq)
if (!test_bit(I915_FENCE_FLAG_ACTIVE, &rq->fence.flags))
return true;
- /*
- * rq->engine is locked by rq->engine->active.lock. That however
- * is not known until after rq->engine has been dereferenced and
- * the lock acquired. Hence we acquire the lock and then validate
- * that rq->engine still matches the lock we hold for it.
- *
- * Here, we are using the breadcrumb lock as a proxy for the
- * rq->engine->active.lock, and we know that since the breadcrumb
- * will be serialised within i915_request_submit/i915_request_unsubmit,
- * the engine cannot change while active as long as we hold the
- * breadcrumb lock on that engine.
- *
- * From the dma_fence_enable_signaling() path, we are outside of the
- * request submit/unsubmit path, and so we must be more careful to
- * acquire the right lock.
- */
- b = READ_ONCE(rq->engine)->breadcrumbs;
- spin_lock(&b->irq_lock);
- while (unlikely(b != READ_ONCE(rq->engine)->breadcrumbs)) {
- spin_unlock(&b->irq_lock);
- b = READ_ONCE(rq->engine)->breadcrumbs;
- spin_lock(&b->irq_lock);
- }
-
- /*
- * Now that we are finally serialised with request submit/unsubmit,
- * [with b->irq_lock] and with i915_request_retire() [via checking
- * SIGNALED with rq->lock] confirm the request is indeed active. If
- * it is no longer active, the breadcrumb will be attached upon
- * i915_request_submit().
- */
+ spin_lock(&ce->signal_lock);
if (test_bit(I915_FENCE_FLAG_ACTIVE, &rq->fence.flags))
- insert_breadcrumb(rq, b);
-
- spin_unlock(&b->irq_lock);
+ insert_breadcrumb(rq);
+ spin_unlock(&ce->signal_lock);
return true;
}
void i915_request_cancel_breadcrumb(struct i915_request *rq)
{
- struct intel_breadcrumbs *b = rq->engine->breadcrumbs;
+ struct intel_context *ce = rq->context;
/*
* We must wait for b->irq_lock so that we know the interrupt handler
@@ -415,20 +422,22 @@ void i915_request_cancel_breadcrumb(struct i915_request *rq)
* the DMA_FENCE_FLAG_SIGNALED_BIT/I915_FENCE_FLAG_SIGNAL dance (if
* required).
*/
- spin_lock(&b->irq_lock);
+ spin_lock(&ce->signal_lock);
if (test_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags)) {
- struct intel_context *ce = rq->context;
-
- list_del(&rq->signal_link);
+ list_del_rcu(&rq->signal_link);
if (list_empty(&ce->signals)) {
- list_del_init(&ce->signal_link);
+ struct intel_breadcrumbs *b = rq->engine->breadcrumbs;
+
+ spin_lock(&b->irq_lock);
+ list_del_rcu(&ce->signal_link);
+ spin_unlock(&b->irq_lock);
intel_context_put(ce);
}
clear_bit(I915_FENCE_FLAG_SIGNAL, &rq->fence.flags);
i915_request_put(rq);
}
- spin_unlock(&b->irq_lock);
+ spin_unlock(&ce->signal_lock);
}
void intel_engine_print_breadcrumbs(struct intel_engine_cs *engine,
@@ -443,8 +452,9 @@ void intel_engine_print_breadcrumbs(struct intel_engine_cs *engine,
drm_printf(p, "Signals:\n");
- spin_lock_irq(&b->irq_lock);
- list_for_each_entry(ce, &b->signalers, signal_link) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(ce, &b->signalers, signal_link) {
+ spin_lock_irq(&ce->signal_lock);
list_for_each_entry(rq, &ce->signals, signal_link) {
drm_printf(p, "\t[%llx:%llx%s] @ %dms\n",
rq->fence.context, rq->fence.seqno,
@@ -453,6 +463,7 @@ void intel_engine_print_breadcrumbs(struct intel_engine_cs *engine,
"",
jiffies_to_msecs(jiffies - rq->emitted_jiffies));
}
+ spin_unlock_irq(&ce->signal_lock);
}
- spin_unlock_irq(&b->irq_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 52db2bde44a3..c3010c8eb966 100644
--- a/drivers/gpu/drm/i915/gt/intel_context.c
+++ b/drivers/gpu/drm/i915/gt/intel_context.c
@@ -297,6 +297,7 @@ intel_context_init(struct intel_context *ce,
ce->vm = i915_vm_get(engine->gt->vm);
+ spin_lock_init(&ce->signal_lock);
INIT_LIST_HEAD(&ce->signal_link);
INIT_LIST_HEAD(&ce->signals);
diff --git a/drivers/gpu/drm/i915/gt/intel_context_types.h b/drivers/gpu/drm/i915/gt/intel_context_types.h
index 4954b0df4864..a78c1c225ce3 100644
--- a/drivers/gpu/drm/i915/gt/intel_context_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_context_types.h
@@ -51,6 +51,7 @@ struct intel_context {
struct i915_address_space *vm;
struct i915_gem_context __rcu *gem_context;
+ spinlock_t signal_lock;
struct list_head signal_link;
struct list_head signals;
--
2.20.1
More information about the Intel-gfx-trybot
mailing list