All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/39] drm/i915: Hold a ref to the ring while retiring
@ 2019-03-13 14:43 Chris Wilson
  2019-03-13 14:43 ` [PATCH 02/39] drm/i915: Lock the gem_context->active_list while dropping the link Chris Wilson
                   ` (41 more replies)
  0 siblings, 42 replies; 69+ messages in thread
From: Chris Wilson @ 2019-03-13 14:43 UTC (permalink / raw)
  To: intel-gfx

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 <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 <linux/hashtable.h>
 #include <linux/irq_work.h>
+#include <linux/kref.h>
 #include <linux/list.h>
 #include <linux/types.h>
 
@@ -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 dc3de09c7586..00fa4a3bc9a3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1236,7 +1236,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);
@@ -2869,7 +2869,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 f26f5cc1584c..ebac752e092d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1311,6 +1311,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);
 
@@ -1335,9 +1336,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);
@@ -1591,7 +1592,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;
@@ -1605,7 +1606,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 e612bdca9fd9..a57489fcb302 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.20.1

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

^ permalink raw reply related	[flat|nested] 69+ messages in thread

end of thread, other threads:[~2019-03-15  9:45 UTC | newest]

Thread overview: 69+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-13 14:43 [PATCH 01/39] drm/i915: Hold a ref to the ring while retiring Chris Wilson
2019-03-13 14:43 ` [PATCH 02/39] drm/i915: Lock the gem_context->active_list while dropping the link Chris Wilson
2019-03-13 14:43 ` [PATCH 03/39] drm/i915: Hold a reference to the active HW context Chris Wilson
2019-03-13 14:43 ` [PATCH 04/39] drm/i915: Stop needlessly acquiring wakeref for debugfs/drop_caches_set Chris Wilson
2019-03-13 14:43 ` [PATCH 05/39] drm/i915/selftests: Provide stub reset functions Chris Wilson
2019-03-13 14:43 ` [PATCH 06/39] drm/i915: Switch to use HWS indices rather than addresses Chris Wilson
2019-03-13 14:43 ` [PATCH 07/39] drm/i915: Introduce the i915_user_extension_method Chris Wilson
2019-03-14 14:52   ` Tvrtko Ursulin
2019-03-14 14:59     ` Chris Wilson
2019-03-14 16:10       ` Tvrtko Ursulin
2019-03-13 14:43 ` [PATCH 08/39] drm/i915: Create/destroy VM (ppGTT) for use with contexts Chris Wilson
2019-03-13 20:11   ` Rodrigo Vivi
2019-03-13 20:47     ` Chris Wilson
2019-03-13 21:15       ` Rodrigo Vivi
2019-03-14 16:07   ` Tvrtko Ursulin
2019-03-14 16:46     ` Chris Wilson
2019-03-13 14:43 ` [PATCH 09/39] drm/i915: Extend CONTEXT_CREATE to set parameters upon construction Chris Wilson
2019-03-13 14:43 ` [PATCH 10/39] drm/i915: Allow contexts to share a single timeline across all engines Chris Wilson
2019-03-14 16:09   ` Tvrtko Ursulin
2019-03-13 14:43 ` [PATCH 11/39] drm/i915: Allow userspace to clone contexts on creation Chris Wilson
2019-03-14 16:18   ` Tvrtko Ursulin
2019-03-14 16:54     ` Chris Wilson
2019-03-14 17:49       ` Tvrtko Ursulin
2019-03-14 17:55         ` Chris Wilson
2019-03-14 16:56     ` Chris Wilson
2019-03-13 14:43 ` [PATCH 12/39] drm/i915: Allow a context to define its set of engines Chris Wilson
2019-03-14 16:47   ` Tvrtko Ursulin
2019-03-14 17:15     ` Chris Wilson
2019-03-14 17:58       ` Tvrtko Ursulin
2019-03-14 18:09         ` Chris Wilson
2019-03-13 14:43 ` [PATCH 13/39] drm/i915: Extend I915_CONTEXT_PARAM_SSEU to support local ctx->engine[] Chris Wilson
2019-03-14 16:49   ` Tvrtko Ursulin
2019-03-14 17:04     ` Chris Wilson
2019-03-14 17:19       ` Tvrtko Ursulin
2019-03-13 14:43 ` [PATCH 14/39] drm/i915: Load balancing across a virtual engine Chris Wilson
2019-03-13 14:43 ` [PATCH 15/39] drm/i915: Extend execution fence to support a callback Chris Wilson
2019-03-14 16:50   ` Tvrtko Ursulin
2019-03-13 14:43 ` [PATCH 16/39] drm/i915/execlists: Virtual engine bonding Chris Wilson
2019-03-14 17:26   ` Tvrtko Ursulin
2019-03-15  9:45     ` Chris Wilson
2019-03-13 14:43 ` [PATCH 17/39] drm/i915: Allow specification of parallel execbuf Chris Wilson
2019-03-14 17:27   ` Tvrtko Ursulin
2019-03-13 14:43 ` [PATCH 18/39] drm/i915/execlists: Skip direct submission if only lite-restore Chris Wilson
2019-03-13 14:43 ` [PATCH 19/39] drm/i915: Split GEM object type definition to its own header Chris Wilson
2019-03-13 14:43 ` [PATCH 20/39] drm/i915: Pull GEM ioctls interface to its own file Chris Wilson
2019-03-13 14:43 ` [PATCH 21/39] drm/i915: Move object->pages API to i915_gem_object.[ch] Chris Wilson
2019-03-13 14:43 ` [PATCH 22/39] drm/i915: Move shmem object setup to its own file Chris Wilson
2019-03-13 14:43 ` [PATCH 23/39] drm/i915: Move phys objects " Chris Wilson
2019-03-13 14:43 ` [PATCH 24/39] drm/i915: Move mmap and friends " Chris Wilson
2019-03-13 14:43 ` [PATCH 25/39] drm/i915: Move GEM domain management " Chris Wilson
2019-03-13 14:43 ` [PATCH 26/39] drm/i915: Move more GEM objects under gem/ Chris Wilson
2019-03-13 14:43 ` [PATCH 27/39] drm/i915: Pull scatterlist utils out of i915_gem.h Chris Wilson
2019-03-13 14:43 ` [PATCH 28/39] drm/i915: Move GEM object domain management from struct_mutex to local Chris Wilson
2019-03-13 14:43 ` [PATCH 29/39] drm/i915: Move GEM object waiting to its own file Chris Wilson
2019-03-13 14:43 ` [PATCH 30/39] drm/i915: Move GEM object busy checking " Chris Wilson
2019-03-13 14:43 ` [PATCH 31/39] drm/i915: Move GEM client throttling " Chris Wilson
2019-03-13 14:43 ` [PATCH 32/39] drm/i915: Drop the deferred active reference Chris Wilson
2019-03-13 14:43 ` [PATCH 33/39] drm/i915: Move object close under its own lock Chris Wilson
2019-03-13 14:43 ` [PATCH 34/39] drm/i915: Rename intel_context.active to .inflight Chris Wilson
2019-03-13 14:43 ` [PATCH 35/39] drm/i915: Keep contexts pinned until after the next kernel context switch Chris Wilson
2019-03-13 14:46   ` Chris Wilson
2019-03-13 14:43 ` [PATCH 36/39] drm/i915: Stop retiring along engine Chris Wilson
2019-03-13 14:43 ` [PATCH 37/39] drm/i915: Replace engine->timeline with a plain list Chris Wilson
2019-03-13 14:44 ` [PATCH 38/39] drm/i915/execlists: Preempt-to-busy Chris Wilson
2019-03-13 14:44 ` [PATCH 39/39] drm/i915: Remove logical HW ID Chris Wilson
2019-03-13 23:55 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/39] drm/i915: Hold a ref to the ring while retiring Patchwork
2019-03-14  0:12 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-03-14  0:22 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-03-14 18:26 ` ✗ Fi.CI.BAT: failure for series starting with [01/39] drm/i915: Hold a ref to the ring while retiring (rev2) Patchwork

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.