[PATCH] vmaref

Chris Wilson chris at chris-wilson.co.uk
Fri May 22 18:38:57 UTC 2020


---
 drivers/gpu/drm/i915/gem/i915_gem_object.c    | 24 -------------------
 .../gpu/drm/i915/gem/i915_gem_object_blt.c    | 17 +++++++++----
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  4 ++--
 drivers/gpu/drm/i915/gt/gen6_ppgtt.c          |  2 +-
 drivers/gpu/drm/i915/gt/intel_gtt.c           | 10 +-------
 drivers/gpu/drm/i915/i915_gem.c               |  6 +++--
 drivers/gpu/drm/i915/i915_perf.c              |  1 +
 drivers/gpu/drm/i915/i915_vma.c               | 23 +++++++-----------
 drivers/gpu/drm/i915/i915_vma.h               | 22 ++++-------------
 9 files changed, 36 insertions(+), 73 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 99356c00c19e..6b81823c353b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -168,31 +168,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 
 		trace_i915_gem_object_destroy(obj);
 
-		if (!list_empty(&obj->vma.list)) {
-			struct i915_vma *vma;
-
-			/*
-			 * Note that the vma keeps an object reference while
-			 * it is active, so it *should* not sleep while we
-			 * destroy it. Our debug code errs insits it *might*.
-			 * For the moment, play along.
-			 */
-			spin_lock(&obj->vma.lock);
-			while ((vma = list_first_entry_or_null(&obj->vma.list,
-							       struct i915_vma,
-							       obj_link))) {
-				GEM_BUG_ON(vma->obj != obj);
-				spin_unlock(&obj->vma.lock);
-
-				__i915_vma_put(vma);
-
-				spin_lock(&obj->vma.lock);
-			}
-			spin_unlock(&obj->vma.lock);
-		}
-
 		i915_gem_object_release_mmap(obj);
-
 		rbtree_postorder_for_each_entry_safe(mmo, mn,
 						     &obj->mmo.offsets,
 						     offset) {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_blt.c b/drivers/gpu/drm/i915/gem/i915_gem_object_blt.c
index f457d7130491..7b2ab0586122 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_blt.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_blt.c
@@ -123,6 +123,7 @@ void intel_emit_vma_release(struct intel_context *ce, struct i915_vma *vma)
 {
 	i915_vma_unpin(vma);
 	intel_gt_buffer_pool_put(vma->private);
+	i915_vma_put(vma);
 	intel_engine_pm_put(ce->engine);
 }
 
@@ -141,7 +142,7 @@ int i915_gem_object_fill_blt(struct drm_i915_gem_object *obj,
 
 	err = i915_vma_pin(vma, 0, 0, PIN_USER);
 	if (unlikely(err))
-		return err;
+		goto out_vma;
 
 	if (obj->cache_dirty & ~obj->cache_coherent) {
 		i915_gem_object_lock(obj);
@@ -195,6 +196,8 @@ int i915_gem_object_fill_blt(struct drm_i915_gem_object *obj,
 	intel_emit_vma_release(ce, batch);
 out_unpin:
 	i915_vma_unpin(vma);
+out_vma:
+	i915_vma_put(vma);
 	return err;
 }
 
@@ -305,11 +308,13 @@ struct i915_vma *intel_emit_vma_copy_blt(struct intel_context *ce,
 
 	err = i915_vma_pin(batch, 0, 0, PIN_USER);
 	if (unlikely(err))
-		goto out_put;
+		goto out_batch;
 
 	batch->private = pool;
 	return batch;
 
+out_batch:
+	i915_vma_put(batch);
 out_put:
 	intel_gt_buffer_pool_put(pool);
 out_pm:
@@ -344,7 +349,7 @@ int i915_gem_object_copy_blt(struct drm_i915_gem_object *src,
 
 	err = i915_vma_pin(vma[0], 0, 0, PIN_USER);
 	if (unlikely(err))
-		return err;
+		goto out_src;
 
 	vma[1] = i915_vma_instance(dst, vm, NULL);
 	if (IS_ERR(vma[1]))
@@ -352,7 +357,7 @@ int i915_gem_object_copy_blt(struct drm_i915_gem_object *src,
 
 	err = i915_vma_pin(vma[1], 0, 0, PIN_USER);
 	if (unlikely(err))
-		goto out_unpin_src;
+		goto out_dst;
 
 	batch = intel_emit_vma_copy_blt(ce, vma[0], vma[1]);
 	if (IS_ERR(batch)) {
@@ -408,8 +413,12 @@ int i915_gem_object_copy_blt(struct drm_i915_gem_object *src,
 	intel_emit_vma_release(ce, batch);
 out_unpin_dst:
 	i915_vma_unpin(vma[1]);
+out_dst:
+	i915_vma_put(vma[1]);
 out_unpin_src:
 	i915_vma_unpin(vma[0]);
+out_src:
+	i915_vma_put(vma[0]);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9c7402ce5bf9..ae8f979d5f8e 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -164,7 +164,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
 	kunmap(p);
 
 out:
-	__i915_vma_put(vma);
+	i915_vma_put(vma);
 	return err;
 }
 
@@ -258,7 +258,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj,
 		if (err)
 			return err;
 
-		__i915_vma_put(vma);
+		i915_vma_put(vma);
 
 		if (igt_timeout(end_time,
 				"%s: timed out after tiling=%d stride=%d\n",
diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
index f4fec7eb4064..34cfb79bc0ad 100644
--- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
+++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c
@@ -276,7 +276,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 {
 	struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 
-	__i915_vma_put(ppgtt->vma);
+	i915_vma_put(ppgtt->vma);
 
 	gen6_ppgtt_free_pd(ppgtt);
 	free_scratch(vm);
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 2a72cce63fd9..78454bdec582 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -175,17 +175,9 @@ void __i915_vm_close(struct i915_address_space *vm)
 		return;
 
 	list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link) {
-		struct drm_i915_gem_object *obj = vma->obj;
-
-		/* Keep the obj (and hence the vma) alive as _we_ destroy it */
-		if (!kref_get_unless_zero(&obj->base.refcount))
-			continue;
-
 		atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
 		WARN_ON(__i915_vma_unbind(vma));
-		__i915_vma_put(vma);
-
-		i915_gem_object_put(obj);
+		i915_vma_put(vma);
 	}
 	GEM_BUG_ON(!list_empty(&vm->bound_list));
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0cbcb9f54e7d..431a511e39d4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -129,6 +129,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 	 */
 	wakeref = intel_runtime_pm_get(rpm);
 
+	i915_gem_object_get(obj);
 try_again:
 	ret = 0;
 	spin_lock(&obj->vma.lock);
@@ -151,7 +152,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			break;
 
 		/* Prevent vma being freed by i915_vma_parked as we unbind */
-		vma = __i915_vma_get(vma);
+		vma = i915_vma_get(vma);
 		spin_unlock(&obj->vma.lock);
 
 		if (vma) {
@@ -160,7 +161,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			    !i915_vma_is_active(vma))
 				ret = i915_vma_unbind(vma);
 
-			__i915_vma_put(vma);
+			i915_vma_put(vma);
 		}
 
 		i915_vm_close(vm);
@@ -174,6 +175,7 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 		goto try_again;
 	}
 
+	i915_gem_object_put(obj);
 	intel_runtime_pm_put(rpm, wakeref);
 
 	return ret;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index f35712d04ba4..7f0bb6f2c9ce 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1879,6 +1879,7 @@ alloc_oa_config_buffer(struct i915_perf_stream *stream,
 
 	oa_bo->oa_config = i915_oa_config_get(oa_config);
 	llist_add(&oa_bo->node, &stream->oa_config_bos);
+	i915_gem_object_put(obj);
 
 	return oa_bo;
 
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 22198b758459..d82051f8fec1 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -90,7 +90,8 @@ static inline struct i915_vma *active_to_vma(struct i915_active *ref)
 
 static int __i915_vma_active(struct i915_active *ref)
 {
-	return i915_vma_tryget(active_to_vma(ref)) ? 0 : -ENOENT;
+	i915_vma_get(active_to_vma(ref));
+	return 0;
 }
 
 __i915_active_call
@@ -118,7 +119,7 @@ vma_create(struct drm_i915_gem_object *obj,
 	mutex_init(&vma->pages_mutex);
 	vma->vm = i915_vm_get(vm);
 	vma->ops = &vm->vma_ops;
-	vma->obj = obj;
+	vma->obj = i915_gem_object_get(obj);
 	vma->resv = obj->base.resv;
 	vma->size = obj->base.size;
 	vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
@@ -282,6 +283,8 @@ i915_vma_instance(struct drm_i915_gem_object *obj,
 
 	spin_lock(&obj->vma.lock);
 	vma = vma_lookup(obj, vm, view);
+	if (vma)
+		vma = i915_vma_get(vma);
 	spin_unlock(&obj->vma.lock);
 
 	/* vma_create() will resolve the race if another creates the vma */
@@ -1089,6 +1092,8 @@ void i915_vma_release(struct kref *ref)
 		list_del(&vma->obj_link);
 		rb_erase(&vma->obj_node, &obj->vma.tree);
 		spin_unlock(&obj->vma.lock);
+
+		i915_gem_object_put(obj);
 	}
 
 	__i915_vma_remove_closed(vma);
@@ -1105,32 +1110,22 @@ void i915_vma_parked(struct intel_gt *gt)
 
 	spin_lock_irq(&gt->closed_lock);
 	list_for_each_entry_safe(vma, next, &gt->closed_vma, closed_link) {
-		struct drm_i915_gem_object *obj = vma->obj;
 		struct i915_address_space *vm = vma->vm;
 
-		/* XXX All to avoid keeping a reference on i915_vma itself */
-
-		if (!kref_get_unless_zero(&obj->base.refcount))
+		if (!i915_vm_tryopen(vm))
 			continue;
 
-		if (!i915_vm_tryopen(vm)) {
-			i915_gem_object_put(obj);
-			continue;
-		}
-
 		list_move(&vma->closed_link, &closed);
 	}
 	spin_unlock_irq(&gt->closed_lock);
 
 	/* As the GT is held idle, no vma can be reopened as we destroy them */
 	list_for_each_entry_safe(vma, next, &closed, closed_link) {
-		struct drm_i915_gem_object *obj = vma->obj;
 		struct i915_address_space *vm = vma->vm;
 
 		INIT_LIST_HEAD(&vma->closed_link);
-		__i915_vma_put(vma);
+		i915_vma_put(vma);
 
-		i915_gem_object_put(obj);
 		i915_vm_close(vm);
 	}
 }
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 8ad1daabcd58..b38570ffeeee 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -124,15 +124,17 @@ static inline u32 i915_ggtt_pin_bias(struct i915_vma *vma)
 	return i915_vm_to_ggtt(vma->vm)->pin_bias;
 }
 
+void i915_vma_release(struct kref *ref);
+
 static inline struct i915_vma *i915_vma_get(struct i915_vma *vma)
 {
-	i915_gem_object_get(vma->obj);
+	kref_get(&vma->ref);
 	return vma;
 }
 
 static inline struct i915_vma *i915_vma_tryget(struct i915_vma *vma)
 {
-	if (likely(kref_get_unless_zero(&vma->obj->base.refcount)))
+	if (kref_get_unless_zero(&vma->ref))
 		return vma;
 
 	return NULL;
@@ -140,7 +142,7 @@ static inline struct i915_vma *i915_vma_tryget(struct i915_vma *vma)
 
 static inline void i915_vma_put(struct i915_vma *vma)
 {
-	i915_gem_object_put(vma->obj);
+	kref_put(&vma->ref, i915_vma_release);
 }
 
 static __always_inline ptrdiff_t ptrdiff(const void *a, const void *b)
@@ -209,20 +211,6 @@ void i915_vma_unlink_ctx(struct i915_vma *vma);
 void i915_vma_close(struct i915_vma *vma);
 void i915_vma_reopen(struct i915_vma *vma);
 
-static inline struct i915_vma *__i915_vma_get(struct i915_vma *vma)
-{
-	if (kref_get_unless_zero(&vma->ref))
-		return vma;
-
-	return NULL;
-}
-
-void i915_vma_release(struct kref *ref);
-static inline void __i915_vma_put(struct i915_vma *vma)
-{
-	kref_put(&vma->ref, i915_vma_release);
-}
-
 #define assert_vma_held(vma) dma_resv_assert_held((vma)->resv)
 
 static inline void i915_vma_lock(struct i915_vma *vma)
-- 
2.20.1



More information about the Intel-gfx-trybot mailing list