[PATCH 2/2] drm/i915/active: Preallocate idle barriers more carefully

[Janusz Krzysztofik]

When we collect barriers for preallocating them, we reuse either idle or
non-idle ones, whichever we find.  In case of non-idle barriers, we
depend on their successful deletion from their barrier tasks lists as an
indication of them not being used concurrently by other threads.  However,
in case of idle barriers, we neither perform any similar checks nor take
any preventive countermeasures, then we risk unexpected races with other
threads, e.g. those trying to select an idle barrier for using it as a
fence tracker.  We may then end up overwriting their fence pointer and/or
corrupting their list node pointers, or they can hurt us in a similar way.

Whenever an idle barrier is selected for preallocation, set it up
immediately as non-idle -- mark it with our ERR_PTR(-EAGAIN) barrier
marker and increase its composite tracker active count.  Serialize that
operation with potential concurrent updates of active fence pointer, and
skip the node if it has been already claimed by another thread.

While being at it, ensure that preallocated_barriers list is still empty
when we populate it with preallocated barriers.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
---
 drivers/gpu/drm/i915/i915_active.c | 46 ++++++++++++++++--------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 3186204fefd05..c6ae39b4767b2 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -788,8 +788,12 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
 	 * node kept alive (as we reuse before parking). We prefer to reuse
 	 * completely idle barriers (less hassle in manipulating the llists),
 	 * but otherwise any will do.
+	 *
+	 * We reuse the request field to mark this as being our proto-node.
 	 */
-	if (ref->cache && is_idle_barrier(ref->cache, idx)) {
+	if (ref->cache && is_idle_barrier(ref->cache, idx) &&
+	    !cmpxchg(__active_fence_slot(&ref->cache->base), NULL, ERR_PTR(-EAGAIN))) {
+		__i915_active_acquire(ref);
 		p = &ref->cache->node;
 		goto match;
 	}
@@ -800,8 +804,11 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
 		struct active_node *node =
 			rb_entry(p, struct active_node, node);
 
-		if (is_idle_barrier(node, idx))
+		if (is_idle_barrier(node, idx) &&
+		    !cmpxchg(__active_fence_slot(&node->base), NULL, ERR_PTR(-EAGAIN))) {
+			__i915_active_acquire(ref);
 			goto match;
+		}
 
 		prev = p;
 		if (node->timeline < idx)
@@ -827,8 +834,11 @@ static struct active_node *reuse_idle_barrier(struct i915_active *ref, u64 idx)
 		if (node->timeline < idx)
 			continue;
 
-		if (is_idle_barrier(node, idx))
+		if (is_idle_barrier(node, idx) &&
+		    !cmpxchg(__active_fence_slot(&node->base), NULL, ERR_PTR(-EAGAIN))) {
+			__i915_active_acquire(ref);
 			goto match;
+		}
 
 		/*
 		 * The list of pending barriers is protected by the
@@ -889,29 +899,23 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 			if (!node)
 				goto unwind;
 
-			RCU_INIT_POINTER(node->base.fence, NULL);
+			/* Mark this as being our unconnected proto-node */
+			RCU_INIT_POINTER(node->base.fence, ERR_PTR(-EAGAIN));
 			node->base.cb.func = node_retire;
 			node->timeline = idx;
 			node->ref = ref;
-		}
-
-		if (!i915_active_fence_isset(&node->base)) {
-			/*
-			 * Mark this as being *our* unconnected proto-node.
-			 *
-			 * Since this node is not in any list, and we have
-			 * decoupled it from the rbtree, we can reuse the
-			 * request to indicate this is an idle-barrier node
-			 * and then we can use the rb_node and list pointers
-			 * for our tracking of the pending barrier.
-			 */
-			RCU_INIT_POINTER(node->base.fence, ERR_PTR(-EAGAIN));
-			node->base.cb.node.prev = (void *)engine;
 			__i915_active_acquire(ref);
+		} else {
+			GEM_BUG_ON(rcu_access_pointer(node->base.fence) != ERR_PTR(-EAGAIN));
 		}
-		GEM_BUG_ON(rcu_access_pointer(node->base.fence) != ERR_PTR(-EAGAIN));
 
-		GEM_BUG_ON(barrier_to_engine(node) != engine);
+		/*
+		 * Since this node is not in any list, we have decoupled it
+		 * from the rbtree, and we reuse the request to indicate
+		 * this is a barrier node, then we can use list pointers
+		 * for our tracking of the pending barrier.
+		 */
+		node->base.cb.node.prev = (void *)engine;
 		first = barrier_to_ll(node);
 		first->next = prev;
 		if (!last)
@@ -920,7 +924,7 @@ int i915_active_acquire_preallocate_barrier(struct i915_active *ref,
 	}
 
 	GEM_BUG_ON(!llist_empty(&ref->preallocated_barriers));
-	llist_add_batch(first, last, &ref->preallocated_barriers);
+	GEM_BUG_ON(!llist_add_batch(first, last, &ref->preallocated_barriers));
 
 	return 0;
 
-- 
2.25.1