[PATCH] drm/i915: Debug active fence returned as previous last active

Janusz Krzysztofik janusz.krzysztofik at linux.intel.com
Thu Dec 1 15:54:07 UTC 2022


After updating an active fence along its timeline, we expect to get back a
reference to a fence that was the last active one before the update, or
NULL if there was none.  As a third possibility, we can theoretically get
a reference to the new fence passed for activation if it was already in
place and have been left intact.  However, no users make a distinction
between the two non-NULL cases before processing the return value.

Warn once when the last active fence returned or a request it belongs to
is the same as the new one just activated, so we can identify those cases
and double check correctness of our current algorithms.

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik at linux.intel.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 1 +
 drivers/gpu/drm/i915/gt/intel_engine_pm.c      | 6 ++++--
 drivers/gpu/drm/i915/i915_active.c             | 8 ++++++--
 drivers/gpu/drm/i915/i915_request.c            | 1 +
 4 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 29e9e8d5b6fec..5c4c3cea31d2a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -3063,6 +3063,7 @@ static int eb_request_add(struct i915_execbuffer *eb, struct i915_request *rq,
 	__i915_request_queue(rq, &attr);
 
 	/* Try to clean up the client's timeline after submitting the request */
+	WARN_ON_ONCE(prev == rq);
 	if (prev)
 		retire_requests(tl, prev);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index e971b153fda97..a59d23091d0ce 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -141,7 +141,7 @@ __queue_and_release_pm(struct i915_request *rq,
 static bool switch_to_kernel_context(struct intel_engine_cs *engine)
 {
 	struct intel_context *ce = engine->kernel_context;
-	struct i915_request *rq;
+	struct i915_request *rq, *prev;
 	bool result = true;
 
 	/*
@@ -210,7 +210,9 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
 
 	/* Install ourselves as a preemption barrier */
 	rq->sched.attr.priority = I915_PRIORITY_BARRIER;
-	if (likely(!__i915_request_commit(rq))) { /* engine should be idle! */
+	prev = __i915_request_commit(rq);
+	WARN_ON_ONCE(prev == rq);
+	if (likely(!prev)) { /* engine should be idle! */
 		/*
 		 * Use an interrupt for precise measurement of duration,
 		 * otherwise we rely on someone else retiring all the requests
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index 7412abf166a8c..2d62c493f11d1 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -428,7 +428,7 @@ replace_barrier(struct i915_active *ref, struct i915_active_fence *active)
 
 int i915_active_add_request(struct i915_active *ref, struct i915_request *rq)
 {
-	struct dma_fence *fence = &rq->fence;
+	struct dma_fence *fence = &rq->fence, *prev;
 	struct i915_active_fence *active;
 	int err;
 
@@ -447,7 +447,9 @@ int i915_active_add_request(struct i915_active *ref, struct i915_request *rq)
 		RCU_INIT_POINTER(active->fence, NULL);
 		atomic_dec(&ref->count);
 	}
-	if (!__i915_active_fence_set(active, fence))
+	prev = __i915_active_fence_set(active, fence);
+	WARN_ON_ONCE(prev == fence);
+	if (!prev)
 		__i915_active_acquire(ref);
 
 out:
@@ -469,6 +471,7 @@ __i915_active_set_fence(struct i915_active *ref,
 
 	rcu_read_lock();
 	prev = __i915_active_fence_set(active, fence);
+	WARN_ON_ONCE(prev == fence);
 	if (prev)
 		prev = dma_fence_get_rcu(prev);
 	else
@@ -1077,6 +1080,7 @@ int i915_active_fence_set(struct i915_active_fence *active,
 	/* Must maintain timeline ordering wrt previous active requests */
 	rcu_read_lock();
 	fence = __i915_active_fence_set(active, &rq->fence);
+	WARN_ON_ONCE(fence == &rq->fence);
 	if (fence) /* but the previous fence may not belong to that timeline! */
 		fence = dma_fence_get_rcu(fence);
 	rcu_read_unlock();
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index f949a9495758a..54b455d51b89f 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1675,6 +1675,7 @@ __i915_request_ensure_ordering(struct i915_request *rq,
 
 	prev = to_request(__i915_active_fence_set(&timeline->last_request,
 						  &rq->fence));
+	WARN_ON_ONCE(prev == rq);
 
 	if (prev && !__i915_request_is_complete(prev)) {
 		bool uses_guc = intel_engine_uses_guc(rq->engine);
-- 
2.25.1



More information about the Intel-gfx-trybot mailing list