[PATCH 3/3] drm/i915/active: Optimize barrier tasks lists locking
Janusz Krzysztofik
janusz.krzysztofik at linux.intel.com
Thu Feb 9 23:42:09 UTC 2023
Drop locking where not needed or not critical. In other cases, add inline
comments explaining why we take the lock.
Also, fix missing spin_lock_init(&mock_engine->base.barriers_lock).
Intended to be squashed with "drm/i915/active: Serialize access to barrier
tasks lists" once positively verified on trybot.
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 ++--
.../gpu/drm/i915/gt/intel_engine_heartbeat.c | 4 ++++
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 9 +++++----
drivers/gpu/drm/i915/gt/mock_engine.c | 2 ++
drivers/gpu/drm/i915/i915_active.c | 17 ++++++++++++++---
5 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index b6fb07c188d16..49142ab29accc 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1421,9 +1421,8 @@ void intel_engine_cleanup_common(struct intel_engine_cs *engine)
if (engine->kernel_context)
intel_engine_destroy_pinned_context(engine->kernel_context);
- spin_lock_irqsave(&engine->barriers_lock, flags);
+ /* No concurrent tasks expected on driver remove, no need to lock */
GEM_BUG_ON(!llist_empty(&engine->barrier_tasks));
- spin_unlock_irqrestore(&engine->barriers_lock, flags);
cleanup_status_page(engine);
intel_wa_list_free(&engine->ctx_wa_list);
@@ -2260,6 +2259,7 @@ void intel_engine_dump(struct intel_engine_cs *engine,
drm_printf(m, "*** WEDGED ***\n");
drm_printf(m, "\tAwake? %d\n", atomic_read(&engine->wakeref.count));
+ /* Serialize against ____active_del_barrier() for debugging accuracy */
spin_lock_irqsave(&engine->barriers_lock, flags);
drm_printf(m, "\tBarriers?: %s\n",
str_yes_no(!llist_empty(&engine->barrier_tasks)));
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
index 9c3c8c431f8d8..ae1d4ffff2139 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -381,6 +381,10 @@ int intel_engine_flush_barriers(struct intel_engine_cs *engine)
unsigned long flags;
int err;
+ /*
+ * Serialize against ____active_del_barrier()
+ * or we risk the barriers not flushed.
+ */
spin_lock_irqsave(&engine->barriers_lock, flags);
err = !llist_empty(&engine->barrier_tasks);
spin_unlock_irqrestore(&engine->barriers_lock, flags);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 708f8907f410b..6c0266850a590 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -237,10 +237,11 @@ static void call_idle_barriers(struct intel_engine_cs *engine)
struct llist_node *node, *next;
unsigned long flags;
- spin_lock_irqsave(&engine->barriers_lock, flags);
- node = llist_del_all(&engine->barrier_tasks);
- spin_unlock_irqrestore(&engine->barriers_lock, flags);
- llist_for_each_safe(node, next, node) {
+ /*
+ * Locking around llist_del_all() not needed as long as we always
+ * call ____active_del_barrier() with engine's wakeref acquired.
+ */
+ llist_for_each_safe(node, next, llist_del_all(&engine->barrier_tasks)) {
struct dma_fence_cb *cb =
container_of((struct list_head *)node,
typeof(*cb), node);
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index c0637bf799a33..383a27b64634f 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -394,6 +394,8 @@ int mock_engine_init(struct intel_engine_cs *engine)
{
struct intel_context *ce;
+ spin_lock_init(&engine->barriers_lock);
+
INIT_LIST_HEAD(&engine->pinned_contexts_list);
engine->sched_engine = i915_sched_engine_create(ENGINE_MOCK);
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index b4c36ba198524..9346f772a1871 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -372,6 +372,8 @@ static bool ____active_del_barrier(struct i915_active *ref,
struct llist_node *pos, *next;
unsigned long flags;
+ /* Must be serialized via pm wakeref with call_idle_barriers() */
+ GEM_BUG_ON(!intel_engine_pm_is_awake(engine));
GEM_BUG_ON(node->timeline != engine->kernel_context->timeline->fence_context);
/*
@@ -884,6 +886,9 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
struct llist_node *prev = first;
struct active_node *node;
+ /* Serialize with call_idle_barriers() via pm wakeref */
+ intel_engine_pm_get(engine);
+
rcu_read_lock();
node = reuse_idle_barrier(ref, idx);
rcu_read_unlock();
@@ -919,7 +924,6 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
first->next = prev;
if (!last)
last = first;
- intel_engine_pm_get(engine);
}
GEM_BUG_ON(!llist_empty(&ref->preallocated_barriers));
@@ -979,9 +983,12 @@ void i915_active_acquire_barrier(struct i915_active *ref)
spin_unlock_irqrestore(&ref->tree_lock, flags);
GEM_BUG_ON(!intel_engine_pm_is_awake(engine));
- spin_lock_irqsave(&engine->barriers_lock, flags);
+ /*
+ * As long as we serialize processing of barrier_tasks inside
+ * ____active_del_barrier(), it should be safe to add a new node
+ * without locking, even while the llist is temporarily emptied.
+ */
llist_add(barrier_to_ll(node), &engine->barrier_tasks);
- spin_unlock_irqrestore(&engine->barriers_lock, flags);
intel_engine_pm_put_delay(engine, 2);
}
}
@@ -1001,6 +1008,10 @@ void i915_request_add_active_barriers(struct i915_request *rq)
GEM_BUG_ON(intel_engine_is_virtual(engine));
GEM_BUG_ON(i915_request_timeline(rq) != engine->kernel_context->timeline);
+ /*
+ * Serialize against ____active_del_barrier() or we risk
+ * the barriers left intact, not replaced with the request.
+ */
spin_lock_irqsave(&engine->barriers_lock, flags);
node = llist_del_all(&engine->barrier_tasks);
spin_unlock_irqrestore(&engine->barriers_lock, flags);
--
2.25.1
More information about the Intel-gfx-trybot
mailing list