Re: [PATCH] drm/i915: Lock signaler timeline while navigating

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 17/09/2019 08:43, Chris Wilson wrote:
As we need to take a walk back along the signaler timeline to find the
fence before upon which we want to wait, we need to lock that timeline
to prevent it being modified as we walk. Similarly, we also need to
acquire a reference to the earlier fence while it still exists!

Though we lack the correct locking today, we are saved by the
overarching struct_mutex -- but that protection is being removed.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_request.c | 30 +++++++++++++++++++++++------
  1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index f12358150097..452ad7a8ff0c 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -767,16 +767,34 @@ i915_request_create(struct intel_context *ce)
  static int
  i915_request_await_start(struct i915_request *rq, struct i915_request *signal)
  {
-	if (list_is_first(&signal->link, &signal->timeline->requests))
+	struct intel_timeline *tl = signal->timeline;
+	struct dma_fence *fence;
+	int err;
+
+	if (list_is_first(&signal->link, &tl->requests))
  		return 0;
- signal = list_prev_entry(signal, link);
-	if (intel_timeline_sync_is_later(rq->timeline, &signal->fence))
+	if (mutex_lock_interruptible_nested(&tl->mutex, SINGLE_DEPTH_NESTING))

We are getting more and more these nested ones and it will become unmanageable to remember which ones, why and on what paths. Perhaps we need a comment next to the member in the structure definition?

+		return -EINTR;
+
+	if (list_is_first(&signal->link, &tl->requests)) {
+		fence = NULL;
+	} else {
+		signal = list_prev_entry(signal, link);
+		fence = dma_fence_get_rcu(&signal->fence);
+	}
+	mutex_unlock(&tl->mutex);
+	if (!fence)

Can it be no fence when it was obtained under the mutex?

  		return 0;
- return i915_sw_fence_await_dma_fence(&rq->submit,
-					     &signal->fence, 0,
-					     I915_FENCE_GFP);
+	err = 0;
+	if (!intel_timeline_sync_is_later(rq->timeline, fence))
+		err = i915_sw_fence_await_dma_fence(&rq->submit,
+						    fence, 0,
+						    I915_FENCE_GFP);
+	dma_fence_put(fence);
+
+	return err;
  }
static intel_engine_mask_t


Regards,

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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux