All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Subject: [Intel-gfx] [PATCH 26/62] drm/i915: Teach the i915_dependency to use a double-lock
Date: Wed, 23 Dec 2020 11:10:50 +0000	[thread overview]
Message-ID: <20201223111126.3338-26-chris@chris-wilson.co.uk> (raw)
In-Reply-To: <20201223111126.3338-1-chris@chris-wilson.co.uk>

Currently, we construct and teardown the i915_dependency chains using a
global spinlock. As the lists are entirely local, it should be possible
to use an double-lock with an explicit nesting [signaler -> waiter,
always] and so avoid the costly convenience of a global spinlock.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_request.c         |  2 +-
 drivers/gpu/drm/i915/i915_scheduler.c       | 63 ++++++++++++++-------
 drivers/gpu/drm/i915/i915_scheduler.h       |  2 +-
 drivers/gpu/drm/i915/i915_scheduler_types.h |  2 +
 4 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index a2ea0aec8680..8408b7317ebc 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -330,7 +330,7 @@ bool i915_request_retire(struct i915_request *rq)
 	intel_context_unpin(rq->context);
 
 	free_capture_list(rq);
-	i915_sched_node_fini(&rq->sched);
+	i915_sched_node_retire(&rq->sched);
 	i915_request_put(rq);
 
 	return true;
diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
index 264ed53821d8..8391841d927d 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.c
+++ b/drivers/gpu/drm/i915/i915_scheduler.c
@@ -19,6 +19,17 @@ static struct i915_global_scheduler {
 
 static DEFINE_SPINLOCK(schedule_lock);
 
+static struct i915_sched_node *node_get(struct i915_sched_node *node)
+{
+	i915_request_get(container_of(node, struct i915_request, sched));
+	return node;
+}
+
+static void node_put(struct i915_sched_node *node)
+{
+	i915_request_put(container_of(node, struct i915_request, sched));
+}
+
 static const struct i915_request *
 node_to_request(const struct i915_sched_node *node)
 {
@@ -353,6 +364,8 @@ void i915_request_set_priority(struct i915_request *rq, int prio)
 
 void i915_sched_node_init(struct i915_sched_node *node)
 {
+	spin_lock_init(&node->lock);
+
 	INIT_LIST_HEAD(&node->signalers_list);
 	INIT_LIST_HEAD(&node->waiters_list);
 	INIT_LIST_HEAD(&node->link);
@@ -377,10 +390,17 @@ i915_dependency_alloc(void)
 	return kmem_cache_alloc(global.slab_dependencies, GFP_KERNEL);
 }
 
+static void
+rcu_dependency_free(struct rcu_head *rcu)
+{
+	kmem_cache_free(global.slab_dependencies,
+			container_of(rcu, typeof(struct i915_dependency), rcu));
+}
+
 static void
 i915_dependency_free(struct i915_dependency *dep)
 {
-	kmem_cache_free(global.slab_dependencies, dep);
+	call_rcu(&dep->rcu, rcu_dependency_free);
 }
 
 bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
@@ -390,24 +410,27 @@ bool __i915_sched_node_add_dependency(struct i915_sched_node *node,
 {
 	bool ret = false;
 
-	spin_lock_irq(&schedule_lock);
+	/* The signal->lock is always the outer lock in this double-lock. */
+	spin_lock(&signal->lock);
 
 	if (!node_signaled(signal)) {
 		INIT_LIST_HEAD(&dep->dfs_link);
 		dep->signaler = signal;
-		dep->waiter = node;
+		dep->waiter = node_get(node);
 		dep->flags = flags;
 
 		/* All set, now publish. Beware the lockless walkers. */
+		spin_lock_nested(&node->lock, SINGLE_DEPTH_NESTING);
 		list_add_rcu(&dep->signal_link, &node->signalers_list);
 		list_add_rcu(&dep->wait_link, &signal->waiters_list);
+		spin_unlock(&node->lock);
 
 		/* Propagate the chains */
 		node->flags |= signal->flags;
 		ret = true;
 	}
 
-	spin_unlock_irq(&schedule_lock);
+	spin_unlock(&signal->lock);
 
 	return ret;
 }
@@ -429,39 +452,36 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
 	return 0;
 }
 
-void i915_sched_node_fini(struct i915_sched_node *node)
+void i915_sched_node_retire(struct i915_sched_node *node)
 {
 	struct i915_dependency *dep, *tmp;
 
-	spin_lock_irq(&schedule_lock);
-
 	/*
 	 * Everyone we depended upon (the fences we wait to be signaled)
 	 * should retire before us and remove themselves from our list.
 	 * However, retirement is run independently on each timeline and
-	 * so we may be called out-of-order.
+	 * so we may be called out-of-order. As we need to avoid taking
+	 * the signaler's lock, just mark up our completion and be wary
+	 * in traversing the signalers->waiters_list.
 	 */
-	list_for_each_entry_safe(dep, tmp, &node->signalers_list, signal_link) {
-		GEM_BUG_ON(!list_empty(&dep->dfs_link));
-
-		list_del_rcu(&dep->wait_link);
-		if (dep->flags & I915_DEPENDENCY_ALLOC)
-			i915_dependency_free(dep);
-	}
-	INIT_LIST_HEAD(&node->signalers_list);
 
 	/* Remove ourselves from everyone who depends upon us */
+	spin_lock(&node->lock);
 	list_for_each_entry_safe(dep, tmp, &node->waiters_list, wait_link) {
+		struct i915_sched_node *w = dep->waiter;
+
 		GEM_BUG_ON(dep->signaler != node);
-		GEM_BUG_ON(!list_empty(&dep->dfs_link));
 
+		spin_lock_nested(&w->lock, SINGLE_DEPTH_NESTING);
 		list_del_rcu(&dep->signal_link);
+		spin_unlock(&w->lock);
+		node_put(w);
+
 		if (dep->flags & I915_DEPENDENCY_ALLOC)
 			i915_dependency_free(dep);
 	}
-	INIT_LIST_HEAD(&node->waiters_list);
-
-	spin_unlock_irq(&schedule_lock);
+	INIT_LIST_HEAD_RCU(&node->waiters_list);
+	spin_unlock(&node->lock);
 }
 
 void i915_request_show_with_schedule(struct drm_printer *m,
@@ -512,8 +532,7 @@ static struct i915_global_scheduler global = { {
 int __init i915_global_scheduler_init(void)
 {
 	global.slab_dependencies = KMEM_CACHE(i915_dependency,
-					      SLAB_HWCACHE_ALIGN |
-					      SLAB_TYPESAFE_BY_RCU);
+					      SLAB_HWCACHE_ALIGN);
 	if (!global.slab_dependencies)
 		return -ENOMEM;
 
diff --git a/drivers/gpu/drm/i915/i915_scheduler.h b/drivers/gpu/drm/i915/i915_scheduler.h
index ccee506c9a26..a045be784c67 100644
--- a/drivers/gpu/drm/i915/i915_scheduler.h
+++ b/drivers/gpu/drm/i915/i915_scheduler.h
@@ -33,7 +33,7 @@ int i915_sched_node_add_dependency(struct i915_sched_node *node,
 				   struct i915_sched_node *signal,
 				   unsigned long flags);
 
-void i915_sched_node_fini(struct i915_sched_node *node);
+void i915_sched_node_retire(struct i915_sched_node *node);
 
 void i915_request_set_priority(struct i915_request *request, int prio);
 
diff --git a/drivers/gpu/drm/i915/i915_scheduler_types.h b/drivers/gpu/drm/i915/i915_scheduler_types.h
index 343ed44d5ed4..623bf41fcf35 100644
--- a/drivers/gpu/drm/i915/i915_scheduler_types.h
+++ b/drivers/gpu/drm/i915/i915_scheduler_types.h
@@ -60,6 +60,7 @@ struct i915_sched_attr {
  * others.
  */
 struct i915_sched_node {
+	spinlock_t lock; /* protect the lists */
 	struct list_head signalers_list; /* those before us, we depend upon */
 	struct list_head waiters_list; /* those after us, they depend upon us */
 	struct list_head link;
@@ -75,6 +76,7 @@ struct i915_dependency {
 	struct list_head signal_link;
 	struct list_head wait_link;
 	struct list_head dfs_link;
+	struct rcu_head rcu;
 	unsigned long flags;
 #define I915_DEPENDENCY_ALLOC		BIT(0)
 #define I915_DEPENDENCY_EXTERNAL	BIT(1)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  parent reply	other threads:[~2020-12-23 11:12 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-23 11:10 [Intel-gfx] [PATCH 01/62] drm/i915/gt: Replace direct submit with direct call to tasklet Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 02/62] drm/i915/gt: Use virtual_engine during execlists_dequeue Chris Wilson
2020-12-24 12:07   ` Matthew Auld
2020-12-23 11:10 ` [Intel-gfx] [PATCH 03/62] drm/i915/gt: Decouple inflight virtual engines Chris Wilson
2020-12-24 12:57   ` Matthew Auld
2020-12-23 11:10 ` [Intel-gfx] [PATCH 04/62] drm/i915/gt: Defer schedule_out until after the next dequeue Chris Wilson
2020-12-24 13:28   ` Matthew Auld
2020-12-23 11:10 ` [Intel-gfx] [PATCH 05/62] drm/i915/gt: Remove virtual breadcrumb before transfer Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 06/62] drm/i915/gt: Shrink the critical section for irq signaling Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 07/62] drm/i915/gt: Resubmit the virtual engine on schedule-out Chris Wilson
2020-12-24 13:49   ` Matthew Auld
2020-12-23 11:10 ` [Intel-gfx] [PATCH 08/62] drm/i915/gt: Simplify virtual engine handling for execlists_hold() Chris Wilson
2020-12-24 13:52   ` Matthew Auld
2020-12-23 11:10 ` [Intel-gfx] [PATCH 09/62] drm/i915/gt: ce->inflight updates are now serialised Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 10/62] drm/i915/selftests: Confirm CS_TIMESTAMP / CTX_TIMESTAMP share a clock Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 11/62] drm/i915/gt: Consolidate the CS timestamp clocks Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 12/62] drm/i915: Drop i915_request.lock serialisation around await_start Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 13/62] drm/i915: Drop i915_request.lock requirement for intel_rps_boost() Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 14/62] drm/i915/gem: Reduce ctx->engine_mutex for reading the clone source Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 15/62] drm/i915/gem: Reduce ctx->engines_mutex for get_engines() Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 16/62] drm/i915: Reduce test_and_set_bit to set_bit in i915_request_submit() Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 17/62] drm/i915/gt: Drop atomic for engine->fw_active tracking Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 18/62] drm/i915/gt: Extract busy-stats for ring-scheduler Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 19/62] drm/i915/gt: Convert stats.active to plain unsigned int Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 20/62] drm/i915/gt: Refactor heartbeat request construction and submission Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 21/62] drm/i915/gt: Do not suspend bonded requests if one hangs Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 22/62] drm/i915/gt: Remove timeslice suppression Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 23/62] drm/i915: Strip out internal priorities Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 24/62] drm/i915: Remove I915_USER_PRIORITY_SHIFT Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 25/62] drm/i915: Replace engine->schedule() with a known request operation Chris Wilson
2020-12-23 11:10 ` Chris Wilson [this message]
2020-12-23 11:10 ` [Intel-gfx] [PATCH 27/62] drm/i915: Restructure priority inheritance Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 28/62] drm/i915/selftests: Measure set-priority duration Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 29/62] drm/i915/selftests: Exercise priority inheritance around an engine loop Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 30/62] drm/i915: Improve DFS for priority inheritance Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 31/62] drm/i915: Extract request submission from execlists Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 32/62] drm/i915: Extract request rewinding " Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 33/62] drm/i915: Extract request suspension from the execlists backend Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 34/62] drm/i915: Extract the ability to defer and rerun a request later Chris Wilson
2020-12-23 11:10 ` [Intel-gfx] [PATCH 35/62] drm/i915: Fix the iterative dfs for defering requests Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 36/62] drm/i915: Move common active lists from engine to i915_scheduler Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 37/62] drm/i915: Move scheduler queue Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 38/62] drm/i915: Move tasklet from execlists to sched Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 39/62] drm/i915: Replace priolist rbtree with a skiplist Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 40/62] drm/i915: Wrap cmpxchg64 with try_cmpxchg64() helper Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 41/62] drm/i915: Fair low-latency scheduling Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 42/62] drm/i915/gt: Specify a deadline for the heartbeat Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 43/62] drm/i915: Extend the priority boosting for the display with a deadline Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 44/62] drm/i915/gt: Skip over completed active execlists, again Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 45/62] drm/i915/gt: Support virtual engine queues Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 46/62] drm/i915: Move saturated workload detection back to the context Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 47/62] drm/i915: Bump default timeslicing quantum to 5ms Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 48/62] drm/i915/gt: Wrap intel_timeline.has_initial_breadcrumb Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 49/62] drm/i915/gt: Track timeline GGTT offset separately from subpage offset Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 50/62] drm/i915/gt: Add timeline "mode" Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 51/62] drm/i915/gt: Use indices for writing into relative timelines Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 52/62] drm/i915/selftests: Exercise relative timeline modes Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 53/62] drm/i915/gt: Use ppHWSP for unshared non-semaphore related timelines Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 54/62] Restore "drm/i915: drop engine_pin/unpin_breadcrumbs_irq" Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 55/62] drm/i915/gt: Couple tasklet scheduling for all CS interrupts Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 56/62] drm/i915/gt: Support creation of 'internal' rings Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 57/62] drm/i915/gt: Use client timeline address for seqno writes Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 58/62] drm/i915/gt: Infrastructure for ring scheduling Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 59/62] drm/i915/gt: Enable busy-stats for ring-scheduler Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 60/62] drm/i915/gt: Implement ring scheduler for gen6/7 Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 61/62] drm/i915/gt: Enable ring scheduling " Chris Wilson
2020-12-23 11:11 ` [Intel-gfx] [PATCH 62/62] drm/i915: Mark up protected uses of 'i915_request_completed' Chris Wilson
2020-12-23 14:20 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/62] drm/i915/gt: Replace direct submit with direct call to tasklet Patchwork
2020-12-23 14:22 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-12-23 14:50 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-12-23 17:57 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201223111126.3338-26-chris@chris-wilson.co.uk \
    --to=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.