From 65baf0ef046b0297a1214932d48a6b71d3d79b4c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 18 Mar 2019 09:51:46 +0000 Subject: [PATCH] drm/i915: Hold a ref to the ring while retiring As the final request on a ring may hold the reference to this ring (via retiring the last pinned context), we may find ourselves chasing a dangling pointer on completion of the list. A quick solution is to hold a reference to the ring itself as we retire along it so that we only free it after we stop dereferencing it. Signed-off-by: Chris Wilson Reviewed-by: Tvrtko Ursulin Link: https://patchwork.freedesktop.org/patch/msgid/20190318095204.9913-4-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/i915_request.c | 6 +++++- drivers/gpu/drm/i915/intel_engine_types.h | 2 ++ drivers/gpu/drm/i915/intel_lrc.c | 4 ++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++---- drivers/gpu/drm/i915/intel_ringbuffer.h | 13 ++++++++++++- drivers/gpu/drm/i915/selftests/mock_engine.c | 1 + 6 files changed, 27 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 9533a85cb0b3..0a3d94517d0a 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1332,8 +1332,12 @@ void i915_retire_requests(struct drm_i915_private *i915) if (!i915->gt.active_requests) return; - list_for_each_entry_safe(ring, tmp, &i915->gt.active_rings, active_link) + list_for_each_entry_safe(ring, tmp, + &i915->gt.active_rings, active_link) { + intel_ring_get(ring); /* last rq holds reference! */ ring_retire_requests(ring); + intel_ring_put(ring); + } } #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) diff --git a/drivers/gpu/drm/i915/intel_engine_types.h b/drivers/gpu/drm/i915/intel_engine_types.h index b0aa1f0d4e47..88ed7ba8886f 100644 --- a/drivers/gpu/drm/i915/intel_engine_types.h +++ b/drivers/gpu/drm/i915/intel_engine_types.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -51,6 +52,7 @@ struct intel_engine_hangcheck { }; struct intel_ring { + struct kref ref; struct i915_vma *vma; void *vaddr; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 29042060b42c..8d1cb81ba0f5 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1230,7 +1230,7 @@ static void execlists_submit_request(struct i915_request *request) static void __execlists_context_fini(struct intel_context *ce) { - intel_ring_free(ce->ring); + intel_ring_put(ce->ring); GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj)); i915_gem_object_put(ce->state->obj); @@ -2867,7 +2867,7 @@ static int execlists_context_deferred_alloc(struct intel_context *ce, return 0; error_ring_free: - intel_ring_free(ring); + intel_ring_put(ring); error_deref_obj: i915_gem_object_put(ctx_obj); return ret; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 366be3d67e15..5137f0140664 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1307,6 +1307,7 @@ intel_engine_create_ring(struct intel_engine_cs *engine, if (!ring) return ERR_PTR(-ENOMEM); + kref_init(&ring->ref); INIT_LIST_HEAD(&ring->request_list); ring->timeline = i915_timeline_get(timeline); @@ -1331,9 +1332,9 @@ intel_engine_create_ring(struct intel_engine_cs *engine, return ring; } -void -intel_ring_free(struct intel_ring *ring) +void intel_ring_free(struct kref *ref) { + struct intel_ring *ring = container_of(ref, typeof(*ring), ref); struct drm_i915_gem_object *obj = ring->vma->obj; i915_vma_close(ring->vma); @@ -1587,7 +1588,7 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine) err_unpin: intel_ring_unpin(ring); err_ring: - intel_ring_free(ring); + intel_ring_put(ring); err: intel_engine_cleanup_common(engine); return err; @@ -1601,7 +1602,7 @@ void intel_engine_cleanup(struct intel_engine_cs *engine) (I915_READ_MODE(engine) & MODE_IDLE) == 0); intel_ring_unpin(engine->buffer); - intel_ring_free(engine->buffer); + intel_ring_put(engine->buffer); if (engine->cleanup) engine->cleanup(engine); diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index f9593e2e070d..a02c92dac5da 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -231,7 +231,18 @@ int intel_ring_pin(struct intel_ring *ring); void intel_ring_reset(struct intel_ring *ring, u32 tail); unsigned int intel_ring_update_space(struct intel_ring *ring); void intel_ring_unpin(struct intel_ring *ring); -void intel_ring_free(struct intel_ring *ring); +void intel_ring_free(struct kref *ref); + +static inline struct intel_ring *intel_ring_get(struct intel_ring *ring) +{ + kref_get(&ring->ref); + return ring; +} + +static inline void intel_ring_put(struct intel_ring *ring) +{ + kref_put(&ring->ref, intel_ring_free); +} void intel_engine_stop(struct intel_engine_cs *engine); void intel_engine_cleanup(struct intel_engine_cs *engine); diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c index f6d120e05ee4..881450c694e9 100644 --- a/drivers/gpu/drm/i915/selftests/mock_engine.c +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c @@ -57,6 +57,7 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine) return NULL; } + kref_init(&ring->base.ref); ring->base.size = sz; ring->base.effective_size = sz; ring->base.vaddr = (void *)(ring + 1); -- 2.11.0