Refcounting dma_resv and using that for drm_exec support in TTM

Refcounting dma_resv and using that for drm_exec support in TTM

[Christian König]

Hi everybody,

The idea of ref-counting dma_resv or ww_mutex came up multiple times from
different people, but so far at least I have abandoned that as to
complicated to implement considering how widely used that object is.

Thanks to AI I gave the task to refcount dma_resv to Claude Sonet 4 just
to check how horrible it would look like.

Well turns out that this is actually a cleanup we should most likely aim
for and I'm really wondering why we haven't done it like this in the
first place.

Not only resolves it a bunch of issues with dma_resv instances shared by
multiple GEM objects (we just recently had a bunch of patches for that on
the mailing list), but also allows TTM to implement it's delayed delete
handling without any zombie resurrection or similar hacks.

The patch set is totally work in progress and only survives a smoke test
with amdgpu, but I still wanted to check if the idea is valuable and
should be looked in further.

Please comment and/or tear apart :)

Cheers,
Christian.

[PATCH 01/12] dma-buf: Add reference counting to dma_resv

[Christian König]

Introduce reference counting for dma_resv objects to better manage their
lifecycle. This replaces the previous approach where dma_buf would either
embed a dma_resv or allocate it inline using a size trick.

Add three new functions:
- dma_resv_alloc(): allocates and initializes a dma_resv with refcount
- dma_resv_get(): acquires a reference to a dma_resv
- dma_resv_put(): releases a reference, freeing when count reaches zero

Update all callers to use dma_resv_put() instead of dma_resv_fini(),
which now becomes an internal cleanup function. The dma_buf export path
now explicitly allocates the dma_resv when needed rather than using
pointer arithmetic tricks.

This provides clearer ownership semantics and makes the code more
maintainable by removing the embedded allocation hack.

Signed-off-by: Christian König <christian.koenig@amd.com>
Assisted-by: Claude:Sonnet 4
---
 drivers/dma-buf/dma-buf.c                  | 24 +++----
 drivers/dma-buf/dma-resv.c                 | 80 +++++++++++++++++++---
 drivers/dma-buf/st-dma-resv.c              | 10 +--
 drivers/gpu/drm/drm_gem.c                  |  2 +-
 drivers/gpu/drm/drm_mode_config.c          |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_object.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c       |  4 +-
 drivers/gpu/drm/i915/gt/intel_gtt.c        |  2 +-
 drivers/gpu/drm/nouveau/nouveau_bo.c       |  2 +-
 drivers/gpu/drm/ttm/tests/ttm_bo_test.c    |  2 +-
 drivers/gpu/drm/ttm/ttm_bo_util.c          |  2 +-
 include/linux/dma-resv.h                   | 22 +++++-
 12 files changed, 117 insertions(+), 37 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index d504c636dc294..53e428fdaf6bd 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -182,8 +182,7 @@ static void dma_buf_release(struct dentry *dentry)
 
 	dmabuf->ops->release(dmabuf);
 
-	if (dmabuf->resv == (struct dma_resv *)&dmabuf[1])
-		dma_resv_fini(dmabuf->resv);
+	dma_resv_put(dmabuf->resv);
 
 	WARN_ON(!list_empty(&dmabuf->attachments));
 	module_put(dmabuf->owner);
@@ -707,10 +706,9 @@ static struct file *dma_buf_getfile(size_t size, int flags)
  */
 struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 {
-	struct dma_buf *dmabuf;
 	struct dma_resv *resv = exp_info->resv;
+	struct dma_buf *dmabuf;
 	struct file *file;
-	size_t alloc_size = sizeof(struct dma_buf);
 	int ret;
 
 	if (WARN_ON(!exp_info->priv || !exp_info->ops
@@ -731,12 +729,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 		goto err_module;
 	}
 
-	if (!exp_info->resv)
-		alloc_size += sizeof(struct dma_resv);
-	else
-		/* prevent &dma_buf[1] == dma_buf->resv */
-		alloc_size += 1;
-	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
+	dmabuf = kzalloc_obj(*dmabuf);
 	if (!dmabuf) {
 		ret = -ENOMEM;
 		goto err_file;
@@ -754,10 +747,13 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 	INIT_LIST_HEAD(&dmabuf->attachments);
 
 	if (!resv) {
-		dmabuf->resv = (struct dma_resv *)&dmabuf[1];
-		dma_resv_init(dmabuf->resv);
+		dmabuf->resv = dma_resv_alloc();
+		if (!dmabuf->resv) {
+			ret = -ENOMEM;
+			goto err_dmabuf;
+		}
 	} else {
-		dmabuf->resv = resv;
+		dmabuf->resv = dma_resv_get(resv);
 	}
 
 	file->private_data = dmabuf;
@@ -770,6 +766,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
 
 	return dmabuf;
 
+err_dmabuf:
+	kfree(dmabuf);
 err_file:
 	fput(file);
 err_module:
diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
index 39a92d9f24136..1251b18e1b540 100644
--- a/drivers/dma-buf/dma-resv.c
+++ b/drivers/dma-buf/dma-resv.c
@@ -36,6 +36,7 @@
 #include <linux/dma-resv.h>
 #include <linux/dma-fence-array.h>
 #include <linux/export.h>
+#include <linux/kref.h>
 #include <linux/mm.h>
 #include <linux/sched/mm.h>
 #include <linux/mmu_notifier.h>
@@ -137,26 +138,87 @@ static void dma_resv_list_free(struct dma_resv_list *list)
  */
 void dma_resv_init(struct dma_resv *obj)
 {
+	kref_init(&obj->refcount);
+	obj->allocated = false;
 	ww_mutex_init(&obj->lock, &reservation_ww_class);
 
 	RCU_INIT_POINTER(obj->fences, NULL);
 }
 EXPORT_SYMBOL(dma_resv_init);
 
-/**
- * dma_resv_fini - destroys a reservation object
- * @obj: the reservation object
+/*
+ * dma_resv_release - release function for kref
+ * @kref: the kref inside the dma_resv object
+ *
+ * This is called when the last reference to a dma_resv object is released.
+ * Cleans up the object and frees it if it was allocated by dma_resv_alloc().
  */
-void dma_resv_fini(struct dma_resv *obj)
+static void dma_resv_release(struct kref *kref)
 {
-	/*
-	 * This object should be dead and all references must have
-	 * been released to it, so no need to be protected with rcu.
-	 */
+	struct dma_resv *obj = container_of(kref, struct dma_resv, refcount);
+
 	dma_resv_list_free(rcu_dereference_protected(obj->fences, true));
 	ww_mutex_destroy(&obj->lock);
+	if (obj->allocated)
+		kfree(obj);
+}
+
+/**
+ * dma_resv_alloc - allocate and initialize a reservation object
+ *
+ * Allocates a new dma_resv object, initializes it, and returns it with a
+ * reference count of 1. The object must be freed with dma_resv_put() when
+ * no longer needed.
+ *
+ * Returns:
+ * A pointer to the allocated dma_resv object, or NULL on allocation failure.
+ */
+struct dma_resv *dma_resv_alloc(void)
+{
+	struct dma_resv *obj;
+
+	obj = kzalloc_obj(*obj);
+	if (!obj)
+		return NULL;
+
+	dma_resv_init(obj);
+	obj->allocated = true;
+
+	return obj;
+}
+EXPORT_SYMBOL(dma_resv_alloc);
+
+/**
+ * dma_resv_get - acquire a reference to a reservation object
+ * @obj: the reservation object
+ *
+ * Increments the reference count on the dma_resv object.
+ *
+ * Returns:
+ * The dma_resv object pointer for convenience.
+ */
+struct dma_resv *dma_resv_get(struct dma_resv *obj)
+{
+	if (obj)
+		kref_get(&obj->refcount);
+	return obj;
+}
+EXPORT_SYMBOL(dma_resv_get);
+
+/**
+ * dma_resv_put - release a reference to a reservation object
+ * @obj: the reservation object
+ *
+ * Decrements the reference count on the dma_resv object. When the reference
+ * count reaches zero, the object is cleaned up with dma_resv_fini() and freed
+ * if it was allocated by dma_resv_alloc().
+ */
+void dma_resv_put(struct dma_resv *obj)
+{
+	if (obj)
+		kref_put(&obj->refcount, dma_resv_release);
 }
-EXPORT_SYMBOL(dma_resv_fini);
+EXPORT_SYMBOL(dma_resv_put);
 
 /* Dereference the fences while ensuring RCU rules */
 static inline struct dma_resv_list *dma_resv_fences_list(struct dma_resv *obj)
diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c
index 0b96136bbd546..2c43d2d2b0169 100644
--- a/drivers/dma-buf/st-dma-resv.c
+++ b/drivers/dma-buf/st-dma-resv.c
@@ -59,7 +59,7 @@ static void test_sanitycheck(struct kunit *test)
 		KUNIT_FAIL(test, "Resv locking failed\n");
 	else
 		dma_resv_unlock(&resv);
-	dma_resv_fini(&resv);
+	dma_resv_put(&resv);
 }
 
 static void test_signaling(struct kunit *test)
@@ -101,7 +101,7 @@ static void test_signaling(struct kunit *test)
 err_unlock:
 	dma_resv_unlock(&resv);
 err_free:
-	dma_resv_fini(&resv);
+	dma_resv_put(&resv);
 	dma_fence_put(f);
 }
 
@@ -160,7 +160,7 @@ static void test_for_each(struct kunit *test)
 err_unlock:
 	dma_resv_unlock(&resv);
 err_free:
-	dma_resv_fini(&resv);
+	dma_resv_put(&resv);
 	dma_fence_put(f);
 }
 
@@ -231,7 +231,7 @@ static void test_for_each_unlocked(struct kunit *test)
 	dma_resv_iter_end(&cursor);
 	dma_fence_signal(f);
 err_free:
-	dma_resv_fini(&resv);
+	dma_resv_put(&resv);
 	dma_fence_put(f);
 }
 
@@ -282,7 +282,7 @@ static void test_get_fences(struct kunit *test)
 		dma_fence_put(fences[i]);
 	kfree(fences);
 err_resv:
-	dma_resv_fini(&resv);
+	dma_resv_put(&resv);
 	dma_fence_put(f);
 }
 
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 018df97d590dc..48176a11d5520 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -249,7 +249,7 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
 {
 	WARN_ON(obj->dma_buf);
 
-	dma_resv_fini(&obj->_resv);
+	dma_resv_put(&obj->_resv);
 	mutex_destroy(&obj->gpuva.lock);
 }
 EXPORT_SYMBOL(drm_gem_private_object_fini);
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
index f432f485a914c..b2d98eab380bf 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -681,7 +681,7 @@ int drmm_mode_config_init(struct drm_device *dev)
 
 		drm_modeset_drop_locks(&modeset_ctx);
 		drm_modeset_acquire_fini(&modeset_ctx);
-		dma_resv_fini(&resv);
+		dma_resv_put(&resv);
 	}
 
 	return drmm_add_action_or_reset(dev, drm_mode_config_init_release,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index 5172d39826548..384c74794fdc1 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -144,7 +144,7 @@ void __i915_gem_object_fini(struct drm_i915_gem_object *obj)
 {
 	mutex_destroy(&obj->mm.get_page.lock);
 	mutex_destroy(&obj->mm.get_dma_page.lock);
-	dma_resv_fini(&obj->base._resv);
+	dma_resv_put(&obj->base._resv);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 64ca5bbc53c68..3cc8df7b8fad1 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -1140,7 +1140,7 @@ void i915_ggtt_driver_late_release(struct drm_i915_private *i915)
 	struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
 
 	GEM_WARN_ON(kref_read(&ggtt->vm.resv_ref) != 1);
-	dma_resv_fini(&ggtt->vm._resv);
+	dma_resv_put(&ggtt->vm._resv);
 }
 
 static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
@@ -1524,7 +1524,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt)
 		ret = intel_ggtt_gmch_probe(ggtt);
 
 	if (ret) {
-		dma_resv_fini(&ggtt->vm._resv);
+		dma_resv_put(&ggtt->vm._resv);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index afbc5c7693089..7b1bdb121c88d 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -225,7 +225,7 @@ void i915_vm_resv_release(struct kref *kref)
 	struct i915_address_space *vm =
 		container_of(kref, typeof(*vm), resv_ref);
 
-	dma_resv_fini(&vm->_resv);
+	dma_resv_put(&vm->_resv);
 	mutex_destroy(&vm->mutex);
 
 	kfree(vm);
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 0e8de6d4b36f7..67c9d32f4f27b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -160,7 +160,7 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo)
 
 		drm_gem_object_release(&bo->base);
 	} else {
-		dma_resv_fini(&bo->base._resv);
+		dma_resv_put(&bo->base._resv);
 	}
 
 	kfree(nvbo);
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
index f3103307b5df9..49b0b48c6c2ac 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
@@ -376,7 +376,7 @@ static void ttm_bo_unreserve_bulk(struct kunit *test)
 	ttm_resource_free(bo1, &res1);
 	ttm_resource_free(bo2, &res2);
 
-	dma_resv_fini(resv);
+	dma_resv_put(resv);
 }
 
 static void ttm_bo_fini_basic(struct kunit *test)
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 3e3c201a02226..029c218f9fb47 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -207,7 +207,7 @@ static void ttm_transfered_destroy(struct ttm_buffer_object *bo)
 	struct ttm_transfer_obj *fbo;
 
 	fbo = container_of(bo, struct ttm_transfer_obj, base);
-	dma_resv_fini(&fbo->base.base._resv);
+	dma_resv_put(&fbo->base.base._resv);
 	ttm_bo_put(fbo->bo);
 	kfree(fbo);
 }
diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
index c5ab6fd9ebe8c..4d12519df34e1 100644
--- a/include/linux/dma-resv.h
+++ b/include/linux/dma-resv.h
@@ -44,6 +44,7 @@
 #include <linux/slab.h>
 #include <linux/seqlock.h>
 #include <linux/rcupdate.h>
+#include <linux/kref.h>
 
 extern struct ww_class reservation_ww_class;
 
@@ -153,6 +154,23 @@ static inline enum dma_resv_usage dma_resv_usage_rw(bool write)
  * drm_gem_object with the same scheme.
  */
 struct dma_resv {
+	/**
+	 * @refcount:
+	 *
+	 * Reference count for this reservation object. The object is freed
+	 * when the reference count reaches zero via dma_resv_put().
+	 */
+	struct kref refcount;
+
+	/**
+	 * @allocated:
+	 *
+	 * True if this object was allocated by dma_resv_alloc(), false if
+	 * embedded in another structure. Used to determine whether to free
+	 * the object memory in the release function.
+	 */
+	bool allocated;
+
 	/**
 	 * @lock:
 	 *
@@ -465,7 +483,9 @@ static inline void dma_resv_unlock(struct dma_resv *obj)
 }
 
 void dma_resv_init(struct dma_resv *obj);
-void dma_resv_fini(struct dma_resv *obj);
+struct dma_resv *dma_resv_alloc(void);
+struct dma_resv *dma_resv_get(struct dma_resv *obj);
+void dma_resv_put(struct dma_resv *obj);
 int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences);
 void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence,
 			enum dma_resv_usage usage);
-- 
2.43.0

Re: [PATCH 01/12] dma-buf: Add reference counting to dma_resv

[Matthew Brost]

On Fri, Jul 10, 2026 at 08:52:41PM +0200, Christian König wrote:
> Introduce reference counting for dma_resv objects to better manage their
> lifecycle. This replaces the previous approach where dma_buf would either
> embed a dma_resv or allocate it inline using a size trick.
> 
> Add three new functions:
> - dma_resv_alloc(): allocates and initializes a dma_resv with refcount
> - dma_resv_get(): acquires a reference to a dma_resv
> - dma_resv_put(): releases a reference, freeing when count reaches zero
> 
> Update all callers to use dma_resv_put() instead of dma_resv_fini(),
> which now becomes an internal cleanup function. The dma_buf export path
> now explicitly allocates the dma_resv when needed rather than using
> pointer arithmetic tricks.
> 
> This provides clearer ownership semantics and makes the code more
> maintainable by removing the embedded allocation hack.
> 

This looks a lot better to me. In particular, I agree with the last
sentence in the commit message.

I'm confident enough in this to say:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

However, given that this is a community-wide change, I'd wait for at
least one other person to give it their blessing as well.

Matt

> Signed-off-by: Christian König <christian.koenig@amd.com>
> Assisted-by: Claude:Sonnet 4
> ---
>  drivers/dma-buf/dma-buf.c                  | 24 +++----
>  drivers/dma-buf/dma-resv.c                 | 80 +++++++++++++++++++---
>  drivers/dma-buf/st-dma-resv.c              | 10 +--
>  drivers/gpu/drm/drm_gem.c                  |  2 +-
>  drivers/gpu/drm/drm_mode_config.c          |  2 +-
>  drivers/gpu/drm/i915/gem/i915_gem_object.c |  2 +-
>  drivers/gpu/drm/i915/gt/intel_ggtt.c       |  4 +-
>  drivers/gpu/drm/i915/gt/intel_gtt.c        |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_bo.c       |  2 +-
>  drivers/gpu/drm/ttm/tests/ttm_bo_test.c    |  2 +-
>  drivers/gpu/drm/ttm/ttm_bo_util.c          |  2 +-
>  include/linux/dma-resv.h                   | 22 +++++-
>  12 files changed, 117 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index d504c636dc294..53e428fdaf6bd 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -182,8 +182,7 @@ static void dma_buf_release(struct dentry *dentry)
>  
>  	dmabuf->ops->release(dmabuf);
>  
> -	if (dmabuf->resv == (struct dma_resv *)&dmabuf[1])
> -		dma_resv_fini(dmabuf->resv);
> +	dma_resv_put(dmabuf->resv);
>  
>  	WARN_ON(!list_empty(&dmabuf->attachments));
>  	module_put(dmabuf->owner);
> @@ -707,10 +706,9 @@ static struct file *dma_buf_getfile(size_t size, int flags)
>   */
>  struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  {
> -	struct dma_buf *dmabuf;
>  	struct dma_resv *resv = exp_info->resv;
> +	struct dma_buf *dmabuf;
>  	struct file *file;
> -	size_t alloc_size = sizeof(struct dma_buf);
>  	int ret;
>  
>  	if (WARN_ON(!exp_info->priv || !exp_info->ops
> @@ -731,12 +729,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  		goto err_module;
>  	}
>  
> -	if (!exp_info->resv)
> -		alloc_size += sizeof(struct dma_resv);
> -	else
> -		/* prevent &dma_buf[1] == dma_buf->resv */
> -		alloc_size += 1;
> -	dmabuf = kzalloc(alloc_size, GFP_KERNEL);
> +	dmabuf = kzalloc_obj(*dmabuf);
>  	if (!dmabuf) {
>  		ret = -ENOMEM;
>  		goto err_file;
> @@ -754,10 +747,13 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  	INIT_LIST_HEAD(&dmabuf->attachments);
>  
>  	if (!resv) {
> -		dmabuf->resv = (struct dma_resv *)&dmabuf[1];
> -		dma_resv_init(dmabuf->resv);
> +		dmabuf->resv = dma_resv_alloc();
> +		if (!dmabuf->resv) {
> +			ret = -ENOMEM;
> +			goto err_dmabuf;
> +		}
>  	} else {
> -		dmabuf->resv = resv;
> +		dmabuf->resv = dma_resv_get(resv);
>  	}
>  
>  	file->private_data = dmabuf;
> @@ -770,6 +766,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info)
>  
>  	return dmabuf;
>  
> +err_dmabuf:
> +	kfree(dmabuf);
>  err_file:
>  	fput(file);
>  err_module:
> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> index 39a92d9f24136..1251b18e1b540 100644
> --- a/drivers/dma-buf/dma-resv.c
> +++ b/drivers/dma-buf/dma-resv.c
> @@ -36,6 +36,7 @@
>  #include <linux/dma-resv.h>
>  #include <linux/dma-fence-array.h>
>  #include <linux/export.h>
> +#include <linux/kref.h>
>  #include <linux/mm.h>
>  #include <linux/sched/mm.h>
>  #include <linux/mmu_notifier.h>
> @@ -137,26 +138,87 @@ static void dma_resv_list_free(struct dma_resv_list *list)
>   */
>  void dma_resv_init(struct dma_resv *obj)
>  {
> +	kref_init(&obj->refcount);
> +	obj->allocated = false;
>  	ww_mutex_init(&obj->lock, &reservation_ww_class);
>  
>  	RCU_INIT_POINTER(obj->fences, NULL);
>  }
>  EXPORT_SYMBOL(dma_resv_init);
>  
> -/**
> - * dma_resv_fini - destroys a reservation object
> - * @obj: the reservation object
> +/*
> + * dma_resv_release - release function for kref
> + * @kref: the kref inside the dma_resv object
> + *
> + * This is called when the last reference to a dma_resv object is released.
> + * Cleans up the object and frees it if it was allocated by dma_resv_alloc().
>   */
> -void dma_resv_fini(struct dma_resv *obj)
> +static void dma_resv_release(struct kref *kref)
>  {
> -	/*
> -	 * This object should be dead and all references must have
> -	 * been released to it, so no need to be protected with rcu.
> -	 */
> +	struct dma_resv *obj = container_of(kref, struct dma_resv, refcount);
> +
>  	dma_resv_list_free(rcu_dereference_protected(obj->fences, true));
>  	ww_mutex_destroy(&obj->lock);
> +	if (obj->allocated)
> +		kfree(obj);
> +}
> +
> +/**
> + * dma_resv_alloc - allocate and initialize a reservation object
> + *
> + * Allocates a new dma_resv object, initializes it, and returns it with a
> + * reference count of 1. The object must be freed with dma_resv_put() when
> + * no longer needed.
> + *
> + * Returns:
> + * A pointer to the allocated dma_resv object, or NULL on allocation failure.
> + */
> +struct dma_resv *dma_resv_alloc(void)
> +{
> +	struct dma_resv *obj;
> +
> +	obj = kzalloc_obj(*obj);
> +	if (!obj)
> +		return NULL;
> +
> +	dma_resv_init(obj);
> +	obj->allocated = true;
> +
> +	return obj;
> +}
> +EXPORT_SYMBOL(dma_resv_alloc);
> +
> +/**
> + * dma_resv_get - acquire a reference to a reservation object
> + * @obj: the reservation object
> + *
> + * Increments the reference count on the dma_resv object.
> + *
> + * Returns:
> + * The dma_resv object pointer for convenience.
> + */
> +struct dma_resv *dma_resv_get(struct dma_resv *obj)
> +{
> +	if (obj)
> +		kref_get(&obj->refcount);
> +	return obj;
> +}
> +EXPORT_SYMBOL(dma_resv_get);
> +
> +/**
> + * dma_resv_put - release a reference to a reservation object
> + * @obj: the reservation object
> + *
> + * Decrements the reference count on the dma_resv object. When the reference
> + * count reaches zero, the object is cleaned up with dma_resv_fini() and freed
> + * if it was allocated by dma_resv_alloc().
> + */
> +void dma_resv_put(struct dma_resv *obj)
> +{
> +	if (obj)
> +		kref_put(&obj->refcount, dma_resv_release);
>  }
> -EXPORT_SYMBOL(dma_resv_fini);
> +EXPORT_SYMBOL(dma_resv_put);
>  
>  /* Dereference the fences while ensuring RCU rules */
>  static inline struct dma_resv_list *dma_resv_fences_list(struct dma_resv *obj)
> diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c
> index 0b96136bbd546..2c43d2d2b0169 100644
> --- a/drivers/dma-buf/st-dma-resv.c
> +++ b/drivers/dma-buf/st-dma-resv.c
> @@ -59,7 +59,7 @@ static void test_sanitycheck(struct kunit *test)
>  		KUNIT_FAIL(test, "Resv locking failed\n");
>  	else
>  		dma_resv_unlock(&resv);
> -	dma_resv_fini(&resv);
> +	dma_resv_put(&resv);
>  }
>  
>  static void test_signaling(struct kunit *test)
> @@ -101,7 +101,7 @@ static void test_signaling(struct kunit *test)
>  err_unlock:
>  	dma_resv_unlock(&resv);
>  err_free:
> -	dma_resv_fini(&resv);
> +	dma_resv_put(&resv);
>  	dma_fence_put(f);
>  }
>  
> @@ -160,7 +160,7 @@ static void test_for_each(struct kunit *test)
>  err_unlock:
>  	dma_resv_unlock(&resv);
>  err_free:
> -	dma_resv_fini(&resv);
> +	dma_resv_put(&resv);
>  	dma_fence_put(f);
>  }
>  
> @@ -231,7 +231,7 @@ static void test_for_each_unlocked(struct kunit *test)
>  	dma_resv_iter_end(&cursor);
>  	dma_fence_signal(f);
>  err_free:
> -	dma_resv_fini(&resv);
> +	dma_resv_put(&resv);
>  	dma_fence_put(f);
>  }
>  
> @@ -282,7 +282,7 @@ static void test_get_fences(struct kunit *test)
>  		dma_fence_put(fences[i]);
>  	kfree(fences);
>  err_resv:
> -	dma_resv_fini(&resv);
> +	dma_resv_put(&resv);
>  	dma_fence_put(f);
>  }
>  
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 018df97d590dc..48176a11d5520 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -249,7 +249,7 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
>  {
>  	WARN_ON(obj->dma_buf);
>  
> -	dma_resv_fini(&obj->_resv);
> +	dma_resv_put(&obj->_resv);
>  	mutex_destroy(&obj->gpuva.lock);
>  }
>  EXPORT_SYMBOL(drm_gem_private_object_fini);
> diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c
> index f432f485a914c..b2d98eab380bf 100644
> --- a/drivers/gpu/drm/drm_mode_config.c
> +++ b/drivers/gpu/drm/drm_mode_config.c
> @@ -681,7 +681,7 @@ int drmm_mode_config_init(struct drm_device *dev)
>  
>  		drm_modeset_drop_locks(&modeset_ctx);
>  		drm_modeset_acquire_fini(&modeset_ctx);
> -		dma_resv_fini(&resv);
> +		dma_resv_put(&resv);
>  	}
>  
>  	return drmm_add_action_or_reset(dev, drm_mode_config_init_release,
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> index 5172d39826548..384c74794fdc1 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
> @@ -144,7 +144,7 @@ void __i915_gem_object_fini(struct drm_i915_gem_object *obj)
>  {
>  	mutex_destroy(&obj->mm.get_page.lock);
>  	mutex_destroy(&obj->mm.get_dma_page.lock);
> -	dma_resv_fini(&obj->base._resv);
> +	dma_resv_put(&obj->base._resv);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index 64ca5bbc53c68..3cc8df7b8fad1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -1140,7 +1140,7 @@ void i915_ggtt_driver_late_release(struct drm_i915_private *i915)
>  	struct i915_ggtt *ggtt = to_gt(i915)->ggtt;
>  
>  	GEM_WARN_ON(kref_read(&ggtt->vm.resv_ref) != 1);
> -	dma_resv_fini(&ggtt->vm._resv);
> +	dma_resv_put(&ggtt->vm._resv);
>  }
>  
>  static unsigned int gen6_get_total_gtt_size(u16 snb_gmch_ctl)
> @@ -1524,7 +1524,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt)
>  		ret = intel_ggtt_gmch_probe(ggtt);
>  
>  	if (ret) {
> -		dma_resv_fini(&ggtt->vm._resv);
> +		dma_resv_put(&ggtt->vm._resv);
>  		return ret;
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index afbc5c7693089..7b1bdb121c88d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -225,7 +225,7 @@ void i915_vm_resv_release(struct kref *kref)
>  	struct i915_address_space *vm =
>  		container_of(kref, typeof(*vm), resv_ref);
>  
> -	dma_resv_fini(&vm->_resv);
> +	dma_resv_put(&vm->_resv);
>  	mutex_destroy(&vm->mutex);
>  
>  	kfree(vm);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 0e8de6d4b36f7..67c9d32f4f27b 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -160,7 +160,7 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo)
>  
>  		drm_gem_object_release(&bo->base);
>  	} else {
> -		dma_resv_fini(&bo->base._resv);
> +		dma_resv_put(&bo->base._resv);
>  	}
>  
>  	kfree(nvbo);
> diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> index f3103307b5df9..49b0b48c6c2ac 100644
> --- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> +++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
> @@ -376,7 +376,7 @@ static void ttm_bo_unreserve_bulk(struct kunit *test)
>  	ttm_resource_free(bo1, &res1);
>  	ttm_resource_free(bo2, &res2);
>  
> -	dma_resv_fini(resv);
> +	dma_resv_put(resv);
>  }
>  
>  static void ttm_bo_fini_basic(struct kunit *test)
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
> index 3e3c201a02226..029c218f9fb47 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> @@ -207,7 +207,7 @@ static void ttm_transfered_destroy(struct ttm_buffer_object *bo)
>  	struct ttm_transfer_obj *fbo;
>  
>  	fbo = container_of(bo, struct ttm_transfer_obj, base);
> -	dma_resv_fini(&fbo->base.base._resv);
> +	dma_resv_put(&fbo->base.base._resv);
>  	ttm_bo_put(fbo->bo);
>  	kfree(fbo);
>  }
> diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h
> index c5ab6fd9ebe8c..4d12519df34e1 100644
> --- a/include/linux/dma-resv.h
> +++ b/include/linux/dma-resv.h
> @@ -44,6 +44,7 @@
>  #include <linux/slab.h>
>  #include <linux/seqlock.h>
>  #include <linux/rcupdate.h>
> +#include <linux/kref.h>
>  
>  extern struct ww_class reservation_ww_class;
>  
> @@ -153,6 +154,23 @@ static inline enum dma_resv_usage dma_resv_usage_rw(bool write)
>   * drm_gem_object with the same scheme.
>   */
>  struct dma_resv {
> +	/**
> +	 * @refcount:
> +	 *
> +	 * Reference count for this reservation object. The object is freed
> +	 * when the reference count reaches zero via dma_resv_put().
> +	 */
> +	struct kref refcount;
> +
> +	/**
> +	 * @allocated:
> +	 *
> +	 * True if this object was allocated by dma_resv_alloc(), false if
> +	 * embedded in another structure. Used to determine whether to free
> +	 * the object memory in the release function.
> +	 */
> +	bool allocated;
> +
>  	/**
>  	 * @lock:
>  	 *
> @@ -465,7 +483,9 @@ static inline void dma_resv_unlock(struct dma_resv *obj)
>  }
>  
>  void dma_resv_init(struct dma_resv *obj);
> -void dma_resv_fini(struct dma_resv *obj);
> +struct dma_resv *dma_resv_alloc(void);
> +struct dma_resv *dma_resv_get(struct dma_resv *obj);
> +void dma_resv_put(struct dma_resv *obj);
>  int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences);
>  void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence,
>  			enum dma_resv_usage usage);
> -- 
> 2.43.0
> 

Re: [PATCH 01/12] dma-buf: Add reference counting to dma_resv

[Danilo Krummrich]

On Sat Jul 11, 2026 at 3:27 AM CEST, Matthew Brost wrote:
> On Fri, Jul 10, 2026 at 08:52:41PM +0200, Christian König wrote:
>> This provides clearer ownership semantics and makes the code more
>> maintainable by removing the embedded allocation hack.
>> 
>
> This looks a lot better to me. In particular, I agree with the last
> sentence in the commit message.

I have to disagree with this, it is the opposite. As long as the struct
dma_resv::allocated fields and the corresponding semantics exists, this does
result into less clear ownership semantics.

When the dma_resv is embedded in another object the reference count becomes
meaningless. If the object embedding the dma_resv is freed it doesn't matter
whether I have a reference count, it would a UAF regardless.

It is misleading (and hence error prone) to have an API where one can obtain a
reference count of an object where the underlying memory can be freed regardless
of the obtained reference count.

A refernece count represents a shared ownership model, which is undermined if
the underlying memory is not owned by the reference count.

That said, I don't mind the reference count, but we can't mix up exclusive
ownership (embedding a structure) and shared ownership (reference count).

>> +static void dma_resv_release(struct kref *kref)
>>  {
>> -	/*
>> -	 * This object should be dead and all references must have
>> -	 * been released to it, so no need to be protected with rcu.
>> -	 */
>> +	struct dma_resv *obj = container_of(kref, struct dma_resv, refcount);
>> +
>>  	dma_resv_list_free(rcu_dereference_protected(obj->fences, true));
>>  	ww_mutex_destroy(&obj->lock);
>> +	if (obj->allocated)
>> +		kfree(obj);
>> +}

[PATCH 02/12] dma-buf/tests: Convert st-dma-resv tests to use dma_resv_alloc

[Christian König]

Update all test cases in st-dma-resv.c to use the new dma_resv_alloc()
API instead of stack-allocating dma_resv objects. This demonstrates the
proper usage of the new allocation and reference counting interface.

All five test functions now:
- Use dma_resv_alloc() to allocate objects
- Check for allocation failure with KUNIT_ASSERT_NOT_NULL
- Clean up with dma_resv_put() which handles freeing

This change also provides better test coverage for the reference
counting implementation.

Signed-off-by: Christian König <christian.koenig@amd.com>
Assisted-by: Claude:Sonnet 4
---
 drivers/dma-buf/st-dma-resv.c | 90 +++++++++++++++++++----------------
 1 file changed, 50 insertions(+), 40 deletions(-)

diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c
index 2c43d2d2b0169..b843d90417822 100644
--- a/drivers/dma-buf/st-dma-resv.c
+++ b/drivers/dma-buf/st-dma-resv.c
@@ -41,7 +41,7 @@ static struct dma_fence *alloc_fence(void)
 
 static void test_sanitycheck(struct kunit *test)
 {
-	struct dma_resv resv;
+	struct dma_resv *resv;
 	struct dma_fence *f;
 	int r;
 
@@ -53,20 +53,22 @@ static void test_sanitycheck(struct kunit *test)
 	dma_fence_signal(f);
 	dma_fence_put(f);
 
-	dma_resv_init(&resv);
-	r = dma_resv_lock(&resv, NULL);
+	resv = dma_resv_alloc();
+	KUNIT_ASSERT_NOT_NULL(test, resv);
+
+	r = dma_resv_lock(resv, NULL);
 	if (r)
 		KUNIT_FAIL(test, "Resv locking failed\n");
 	else
-		dma_resv_unlock(&resv);
-	dma_resv_put(&resv);
+		dma_resv_unlock(resv);
+	dma_resv_put(resv);
 }
 
 static void test_signaling(struct kunit *test)
 {
 	const struct dma_resv_usage_param *param = test->param_value;
 	enum dma_resv_usage usage = param->usage;
-	struct dma_resv resv;
+	struct dma_resv *resv;
 	struct dma_fence *f;
 	int r;
 
@@ -75,33 +77,35 @@ static void test_signaling(struct kunit *test)
 
 	dma_fence_enable_signaling(f);
 
-	dma_resv_init(&resv);
-	r = dma_resv_lock(&resv, NULL);
+	resv = dma_resv_alloc();
+	KUNIT_ASSERT_NOT_NULL(test, resv);
+
+	r = dma_resv_lock(resv, NULL);
 	if (r) {
 		KUNIT_FAIL(test, "Resv locking failed");
 		goto err_free;
 	}
 
-	r = dma_resv_reserve_fences(&resv, 1);
+	r = dma_resv_reserve_fences(resv, 1);
 	if (r) {
 		KUNIT_FAIL(test, "Resv shared slot allocation failed");
 		goto err_unlock;
 	}
 
-	dma_resv_add_fence(&resv, f, usage);
-	if (dma_resv_test_signaled(&resv, usage)) {
+	dma_resv_add_fence(resv, f, usage);
+	if (dma_resv_test_signaled(resv, usage)) {
 		KUNIT_FAIL(test, "Resv unexpectedly signaled");
 		goto err_unlock;
 	}
 	dma_fence_signal(f);
-	if (!dma_resv_test_signaled(&resv, usage)) {
+	if (!dma_resv_test_signaled(resv, usage)) {
 		KUNIT_FAIL(test, "Resv not reporting signaled");
 		goto err_unlock;
 	}
 err_unlock:
-	dma_resv_unlock(&resv);
+	dma_resv_unlock(resv);
 err_free:
-	dma_resv_put(&resv);
+	dma_resv_put(resv);
 	dma_fence_put(f);
 }
 
@@ -111,7 +115,7 @@ static void test_for_each(struct kunit *test)
 	enum dma_resv_usage usage = param->usage;
 	struct dma_resv_iter cursor;
 	struct dma_fence *f, *fence;
-	struct dma_resv resv;
+	struct dma_resv *resv;
 	int r;
 
 	f = alloc_fence();
@@ -119,23 +123,25 @@ static void test_for_each(struct kunit *test)
 
 	dma_fence_enable_signaling(f);
 
-	dma_resv_init(&resv);
-	r = dma_resv_lock(&resv, NULL);
+	resv = dma_resv_alloc();
+	KUNIT_ASSERT_NOT_NULL(test, resv);
+
+	r = dma_resv_lock(resv, NULL);
 	if (r) {
 		KUNIT_FAIL(test, "Resv locking failed");
 		goto err_free;
 	}
 
-	r = dma_resv_reserve_fences(&resv, 1);
+	r = dma_resv_reserve_fences(resv, 1);
 	if (r) {
 		KUNIT_FAIL(test, "Resv shared slot allocation failed");
 		goto err_unlock;
 	}
 
-	dma_resv_add_fence(&resv, f, usage);
+	dma_resv_add_fence(resv, f, usage);
 
 	r = -ENOENT;
-	dma_resv_for_each_fence(&cursor, &resv, usage, fence) {
+	dma_resv_for_each_fence(&cursor, resv, usage, fence) {
 		if (!r) {
 			KUNIT_FAIL(test, "More than one fence found");
 			goto err_unlock;
@@ -158,9 +164,9 @@ static void test_for_each(struct kunit *test)
 	}
 	dma_fence_signal(f);
 err_unlock:
-	dma_resv_unlock(&resv);
+	dma_resv_unlock(resv);
 err_free:
-	dma_resv_put(&resv);
+	dma_resv_put(resv);
 	dma_fence_put(f);
 }
 
@@ -170,7 +176,7 @@ static void test_for_each_unlocked(struct kunit *test)
 	enum dma_resv_usage usage = param->usage;
 	struct dma_resv_iter cursor;
 	struct dma_fence *f, *fence;
-	struct dma_resv resv;
+	struct dma_resv *resv;
 	int r;
 
 	f = alloc_fence();
@@ -178,25 +184,27 @@ static void test_for_each_unlocked(struct kunit *test)
 
 	dma_fence_enable_signaling(f);
 
-	dma_resv_init(&resv);
-	r = dma_resv_lock(&resv, NULL);
+	resv = dma_resv_alloc();
+	KUNIT_ASSERT_NOT_NULL(test, resv);
+
+	r = dma_resv_lock(resv, NULL);
 	if (r) {
 		KUNIT_FAIL(test, "Resv locking failed");
 		goto err_free;
 	}
 
-	r = dma_resv_reserve_fences(&resv, 1);
+	r = dma_resv_reserve_fences(resv, 1);
 	if (r) {
 		KUNIT_FAIL(test, "Resv shared slot allocation failed");
-		dma_resv_unlock(&resv);
+		dma_resv_unlock(resv);
 		goto err_free;
 	}
 
-	dma_resv_add_fence(&resv, f, usage);
-	dma_resv_unlock(&resv);
+	dma_resv_add_fence(resv, f, usage);
+	dma_resv_unlock(resv);
 
 	r = -ENOENT;
-	dma_resv_iter_begin(&cursor, &resv, usage);
+	dma_resv_iter_begin(&cursor, resv, usage);
 	dma_resv_for_each_fence_unlocked(&cursor, fence) {
 		if (!r) {
 			KUNIT_FAIL(test, "More than one fence found");
@@ -231,7 +239,7 @@ static void test_for_each_unlocked(struct kunit *test)
 	dma_resv_iter_end(&cursor);
 	dma_fence_signal(f);
 err_free:
-	dma_resv_put(&resv);
+	dma_resv_put(resv);
 	dma_fence_put(f);
 }
 
@@ -240,7 +248,7 @@ static void test_get_fences(struct kunit *test)
 	const struct dma_resv_usage_param *param = test->param_value;
 	enum dma_resv_usage usage = param->usage;
 	struct dma_fence *f, **fences = NULL;
-	struct dma_resv resv;
+	struct dma_resv *resv;
 	int r, i;
 
 	f = alloc_fence();
@@ -248,24 +256,26 @@ static void test_get_fences(struct kunit *test)
 
 	dma_fence_enable_signaling(f);
 
-	dma_resv_init(&resv);
-	r = dma_resv_lock(&resv, NULL);
+	resv = dma_resv_alloc();
+	KUNIT_ASSERT_NOT_NULL(test, resv);
+
+	r = dma_resv_lock(resv, NULL);
 	if (r) {
 		KUNIT_FAIL(test, "Resv locking failed");
 		goto err_resv;
 	}
 
-	r = dma_resv_reserve_fences(&resv, 1);
+	r = dma_resv_reserve_fences(resv, 1);
 	if (r) {
 		KUNIT_FAIL(test, "Resv shared slot allocation failed");
-		dma_resv_unlock(&resv);
+		dma_resv_unlock(resv);
 		goto err_resv;
 	}
 
-	dma_resv_add_fence(&resv, f, usage);
-	dma_resv_unlock(&resv);
+	dma_resv_add_fence(resv, f, usage);
+	dma_resv_unlock(resv);
 
-	r = dma_resv_get_fences(&resv, usage, &i, &fences);
+	r = dma_resv_get_fences(resv, usage, &i, &fences);
 	if (r) {
 		KUNIT_FAIL(test, "get_fences failed");
 		goto err_free;
@@ -282,7 +292,7 @@ static void test_get_fences(struct kunit *test)
 		dma_fence_put(fences[i]);
 	kfree(fences);
 err_resv:
-	dma_resv_put(&resv);
+	dma_resv_put(resv);
 	dma_fence_put(f);
 }
 
-- 
2.43.0

[PATCH 03/12] drm/gem: Add helper for drm_gem_object resv assignment

[Christian König]

Introduce drm_gem_object_set_resv() to safely assign reservation objects
to GEM objects with proper reference counting. This helper replaces all
direct assignments to drm_gem_object.resv throughout the DRM subsystem.

The function:
- Acquires a reference to the new resv via dma_resv_get()
- Assigns it to obj->resv
- Releases the old resv reference via dma_resv_put()

This ensures proper reference counting when sharing reservation objects
between GEM objects or when importing dma-bufs, preventing reference
leaks and use-after-free bugs.

Update all drivers to use the new helper:
- drm_prime: dma-buf import path
- drm_gem_shmem_helper: shmem prime import
- i915: dmabuf import
- msm: MSM_BO_NO_SHARE case
- panthor: exclusive_vm case
- virtio: dma-buf import
- xe: dummy object creation
- ttm: external resv assignment
- ttm/tests: test cases

Signed-off-by: Christian König <christian.koenig@amd.com>
Assisted-by: Claude:Sonnet 4
---
 drivers/gpu/drm/drm_gem.c                     | 31 ++++++++++++++++++-
 drivers/gpu/drm/drm_gem_shmem_helper.c        |  2 +-
 drivers/gpu/drm/drm_prime.c                   |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c    |  2 +-
 drivers/gpu/drm/msm/msm_gem.c                 |  2 +-
 drivers/gpu/drm/panthor/panthor_gem.c         |  2 +-
 drivers/gpu/drm/ttm/tests/ttm_bo_test.c       |  2 +-
 drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  2 +-
 drivers/gpu/drm/ttm/ttm_bo.c                  |  7 ++---
 drivers/gpu/drm/virtio/virtgpu_prime.c        |  2 +-
 drivers/gpu/drm/xe/xe_dma_buf.c               |  2 +-
 include/drm/drm_gem.h                         |  2 ++
 12 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 48176a11d5520..bbcbd25f014f0 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -230,7 +230,7 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	mutex_init(&obj->gpuva.lock);
 	dma_resv_init(&obj->_resv);
 	if (!obj->resv)
-		obj->resv = &obj->_resv;
+		obj->resv = dma_resv_get(&obj->_resv);
 
 	drm_gem_gpuva_init(obj);
 
@@ -249,11 +249,40 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
 {
 	WARN_ON(obj->dma_buf);
 
+	dma_resv_put(obj->resv);
 	dma_resv_put(&obj->_resv);
 	mutex_destroy(&obj->gpuva.lock);
 }
 EXPORT_SYMBOL(drm_gem_private_object_fini);
 
+/**
+ * drm_gem_object_set_resv - Set the reservation object for a GEM object
+ * @obj: GEM object
+ * @resv: reservation object to assign
+ *
+ * This function safely assigns a new reservation object to a GEM object.
+ * It releases the old reservation object reference (if any) and acquires
+ * a reference to the new one.
+ *
+ * This should be used when changing the reservation object of an already
+ * initialized GEM object, for example when importing a dma-buf or sharing
+ * a reservation object with another object.
+ *
+ * Returns:
+ * The new reservation object pointer for convenience.
+ */
+struct dma_resv *drm_gem_object_set_resv(struct drm_gem_object *obj,
+					  struct dma_resv *resv)
+{
+	struct dma_resv *old_resv = obj->resv;
+
+	obj->resv = dma_resv_get(resv);
+	dma_resv_put(old_resv);
+
+	return obj->resv;
+}
+EXPORT_SYMBOL(drm_gem_object_set_resv);
+
 static void drm_gem_object_handle_get(struct drm_gem_object *obj)
 {
 	struct drm_device *dev = obj->dev;
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 22ec52e2ffb87..76a8d7f252e00 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -977,7 +977,7 @@ struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
 	drm_dbg_prime(dev, "size = %zu\n", size);
 
 	shmem->base.import_attach = attach;
-	shmem->base.resv = dma_buf->resv;
+	drm_gem_object_set_resv(&shmem->base, dma_buf->resv);
 
 	return &shmem->base;
 
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 9b44c78cd77fc..6cfeca347faff 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -1006,7 +1006,7 @@ struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev,
 	}
 
 	obj->import_attach = attach;
-	obj->resv = dma_buf->resv;
+	drm_gem_object_set_resv(obj, dma_buf->resv);
 
 	return obj;
 
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
index b43d34c7d6419..c824606497d2e 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_dmabuf.c
@@ -320,7 +320,7 @@ struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,
 	i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops, &lock_class,
 			     I915_BO_ALLOC_USER);
 	obj->base.import_attach = attach;
-	obj->base.resv = dma_buf->resv;
+	drm_gem_object_set_resv(&obj->base, dma_buf->resv);
 
 	/* We use GTT as shorthand for a coherent domain, one that is
 	 * neither in the GPU cache nor in the CPU cache, where all
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index efd3d3c9a4490..81ebb1e1094c3 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -1151,7 +1151,7 @@ int msm_gem_new_handle(struct drm_device *dev, struct drm_file *file,
 
 		drm_gem_object_get(r_obj);
 
-		obj->resv = r_obj->resv;
+		drm_gem_object_set_resv(obj, r_obj->resv);
 	}
 
 	ret = drm_gem_handle_create(file, obj, handle);
diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
index 9855df7381947..2de24f1b60184 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.c
+++ b/drivers/gpu/drm/panthor/panthor_gem.c
@@ -1024,7 +1024,7 @@ panthor_gem_create(struct drm_device *dev, size_t size, uint32_t flags,
 	if (exclusive_vm) {
 		bo->exclusive_vm_root_gem = panthor_vm_root_gem(exclusive_vm);
 		drm_gem_object_get(bo->exclusive_vm_root_gem);
-		bo->base.resv = bo->exclusive_vm_root_gem->resv;
+		drm_gem_object_set_resv(&bo->base, bo->exclusive_vm_root_gem->resv);
 	}
 
 	panthor_gem_debugfs_set_usage_flags(bo, usage_flags);
diff --git a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
index 49b0b48c6c2ac..3aff1c427cd1f 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_bo_test.c
@@ -461,7 +461,7 @@ static void ttm_bo_fini_shared_resv(struct kunit *test)
 
 	bo = ttm_bo_kunit_init(test, test->priv, BO_SIZE, NULL);
 	bo->type = ttm_bo_type_device;
-	bo->base.resv = external_resv;
+	drm_gem_object_set_resv(&bo->base, external_resv);
 
 	ttm_bo_fini(bo);
 }
diff --git a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
index 5cfe8f3f80d75..1146cc9ae5224 100644
--- a/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
+++ b/drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c
@@ -181,7 +181,7 @@ struct ttm_buffer_object *ttm_bo_kunit_init(struct kunit *test,
 	bo->base = gem_obj;
 
 	if (obj)
-		bo->base.resv = obj;
+		drm_gem_object_set_resv(&bo->base, obj);
 
 	err = drm_gem_object_init(devs->drm, &bo->base, size);
 	KUNIT_ASSERT_EQ(test, err, 0);
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 3980f376e3ba4..8cf266da2bc61 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -209,7 +209,7 @@ static int ttm_bo_individualize_resv(struct ttm_buffer_object *bo)
 		 * the resv object while holding the lru_lock.
 		 */
 		spin_lock(&bo->bdev->lru_lock);
-		bo->base.resv = &bo->base._resv;
+		drm_gem_object_set_resv(&bo->base, &bo->base._resv);
 		spin_unlock(&bo->bdev->lru_lock);
 	}
 
@@ -942,10 +942,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo,
 	bo->pin_count = 0;
 	bo->sg = sg;
 	bo->bulk_move = NULL;
-	if (resv)
-		bo->base.resv = resv;
-	else
-		bo->base.resv = &bo->base._resv;
+	drm_gem_object_set_resv(&bo->base, resv ?: &bo->base._resv);
 	atomic_inc(&ttm_glob.bo_count);
 
 	/*
diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 216c77cd0d21b..79964b4362439 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -361,7 +361,7 @@ struct drm_gem_object *virtgpu_gem_prime_import(struct drm_device *dev,
 	INIT_LIST_HEAD(&bo->restore_node);
 
 	obj = &bo->base.base;
-	obj->resv = buf->resv;
+	drm_gem_object_set_resv(obj, buf->resv);
 	obj->funcs = &virtgpu_gem_dma_buf_funcs;
 	drm_gem_private_object_init(dev, obj, buf->size);
 
diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c
index 8a920e58245cd..33bebf85ac622 100644
--- a/drivers/gpu/drm/xe/xe_dma_buf.c
+++ b/drivers/gpu/drm/xe/xe_dma_buf.c
@@ -293,7 +293,7 @@ xe_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf)
 	if (!dummy_obj)
 		return ERR_PTR(-ENOMEM);
 
-	dummy_obj->resv = resv;
+	drm_gem_object_set_resv(dummy_obj, resv);
 	xe_validation_guard(&ctx, &xe->val, &exec, (struct xe_val_flags) {}, ret) {
 		ret = drm_exec_lock_obj(&exec, dummy_obj);
 		drm_exec_retry_on_contention(&exec);
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 885244e375d32..09f5ec28218a0 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -529,6 +529,8 @@ int drm_gem_object_init(struct drm_device *dev,
 void drm_gem_private_object_init(struct drm_device *dev,
 				 struct drm_gem_object *obj, size_t size);
 void drm_gem_private_object_fini(struct drm_gem_object *obj);
+struct dma_resv *drm_gem_object_set_resv(struct drm_gem_object *obj,
+					  struct dma_resv *resv);
 void drm_gem_vm_open(struct vm_area_struct *vma);
 void drm_gem_vm_close(struct vm_area_struct *vma);
 int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
-- 
2.43.0

[PATCH 04/12] drm/ttm: Switch LRU cursor to track dma_resv instead of buffer objects

[Christian König]

Refactor the LRU cursor to hold references to dma_resv directly while
locking it rather than the buffer object.

This avoid the need to grab a reference to the BO and so allows handling
of BOs with zero reference count.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c  | 124 +++++++++++++++--------------
 drivers/gpu/drm/ttm/ttm_resource.c |  18 +++++
 include/drm/ttm/ttm_bo.h           |  10 +--
 include/drm/ttm/ttm_resource.h     |   2 +
 4 files changed, 87 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 029c218f9fb47..1f7361604b552 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -819,19 +819,17 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
 }
 
 static bool ttm_lru_walk_trylock(struct ttm_bo_lru_cursor *curs,
-				 struct ttm_buffer_object *bo)
+				 struct dma_resv *resv)
 {
 	struct ttm_operation_ctx *ctx = curs->arg->ctx;
 
-	curs->needs_unlock = false;
-
-	if (dma_resv_trylock(bo->base.resv)) {
-		curs->needs_unlock = true;
+	if (dma_resv_trylock(resv)) {
+		curs->resv = dma_resv_get(resv);
 		return true;
 	}
 
-	if (bo->base.resv == ctx->resv && ctx->allow_res_evict) {
-		dma_resv_assert_held(bo->base.resv);
+	if (resv == ctx->resv && ctx->allow_res_evict) {
+		dma_resv_assert_held(resv);
 		return true;
 	}
 
@@ -839,18 +837,18 @@ static bool ttm_lru_walk_trylock(struct ttm_bo_lru_cursor *curs,
 }
 
 static int ttm_lru_walk_ticketlock(struct ttm_bo_lru_cursor *curs,
-				   struct ttm_buffer_object *bo)
+				   struct dma_resv *resv)
 {
 	struct ttm_lru_walk_arg *arg = curs->arg;
 	int ret;
 
 	if (arg->ctx->interruptible)
-		ret = dma_resv_lock_interruptible(bo->base.resv, arg->ticket);
+		ret = dma_resv_lock_interruptible(resv, arg->ticket);
 	else
-		ret = dma_resv_lock(bo->base.resv, arg->ticket);
+		ret = dma_resv_lock(resv, arg->ticket);
 
 	if (!ret) {
-		curs->needs_unlock = true;
+		curs->resv = dma_resv_get(resv);
 		/*
 		 * Only a single ticketlock per loop. Ticketlocks are prone
 		 * to return -EDEADLK causing the eviction to fail, so
@@ -920,14 +918,16 @@ s64 ttm_lru_walk_for_evict(struct ttm_lru_walk *walk, struct ttm_device *bdev,
 }
 EXPORT_SYMBOL(ttm_lru_walk_for_evict);
 
-static void ttm_bo_lru_cursor_cleanup_bo(struct ttm_bo_lru_cursor *curs)
+static void ttm_bo_lru_cursor_cleanup(struct ttm_bo_lru_cursor *curs)
 {
-	struct ttm_buffer_object *bo = curs->bo;
+	if (curs->resv) {
+		dma_resv_unlock(curs->resv);
+		dma_resv_put(curs->resv);
+		curs->resv = NULL;
+	}
 
-	if (bo) {
-		if (curs->needs_unlock)
-			dma_resv_unlock(bo->base.resv);
-		ttm_bo_put(bo);
+	if (curs->bo) {
+		drm_gem_object_put(&curs->bo->base);
 		curs->bo = NULL;
 	}
 }
@@ -941,7 +941,7 @@ void ttm_bo_lru_cursor_fini(struct ttm_bo_lru_cursor *curs)
 {
 	spinlock_t *lru_lock = &curs->res_curs.man->bdev->lru_lock;
 
-	ttm_bo_lru_cursor_cleanup_bo(curs);
+	ttm_bo_lru_cursor_cleanup(curs);
 	spin_lock(lru_lock);
 	ttm_resource_cursor_fini(&curs->res_curs);
 	spin_unlock(lru_lock);
@@ -972,21 +972,18 @@ ttm_bo_lru_cursor_init(struct ttm_bo_lru_cursor *curs,
 EXPORT_SYMBOL(ttm_bo_lru_cursor_init);
 
 static struct ttm_buffer_object *
-__ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
+__ttm_bo_lru_cursor_iter(struct ttm_bo_lru_cursor *curs, bool first)
 {
 	spinlock_t *lru_lock = &curs->res_curs.man->bdev->lru_lock;
-	struct ttm_resource *res = NULL;
-	struct ttm_buffer_object *bo;
 	struct ttm_lru_walk_arg *arg = curs->arg;
-	bool first = !curs->bo;
-
-	ttm_bo_lru_cursor_cleanup_bo(curs);
+	int ret;
 
-	spin_lock(lru_lock);
 	for (;;) {
-		int mem_type, ret = 0;
-		bool bo_locked = false;
+		struct ttm_resource *res;
+
+		ttm_bo_lru_cursor_cleanup(curs);
 
+		spin_lock(lru_lock);
 		if (first) {
 			res = ttm_resource_manager_first(&curs->res_curs);
 			first = false;
@@ -996,43 +993,48 @@ __ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
 		if (!res)
 			break;
 
-		bo = res->bo;
-		if (ttm_lru_walk_trylock(curs, bo))
-			bo_locked = true;
-		else if (!arg->ticket || arg->ctx->no_wait_gpu || arg->trylock_only)
-			continue;
-
-		if (!ttm_bo_get_unless_zero(bo)) {
-			if (curs->needs_unlock)
-				dma_resv_unlock(bo->base.resv);
-			continue;
+		if (!ttm_lru_walk_trylock(curs, res->bo->base.resv)) {
+			struct dma_resv *resv;
+
+			if (!arg->ticket || arg->ctx->no_wait_gpu ||
+			    arg->trylock_only) {
+				spin_unlock(lru_lock);
+				continue;
+			}
+
+			resv = dma_resv_get(res->bo->base.resv);
+			spin_unlock(lru_lock);
+
+			ret = ttm_lru_walk_ticketlock(curs, resv);
+			if (ret && ret != -EALREADY)
+				return ERR_PTR(ret);
+
+			/*
+			 * We need to double check that we still have the same
+			 * dma_resv object.
+			 */
+			spin_lock(lru_lock);
+			res = ttm_resource_manager_current(&curs->res_curs);
+			if (ret || !res || res->bo->base.resv != resv) {
+				spin_unlock(lru_lock);
+				dma_resv_put(resv);
+				continue;
+			}
+			dma_resv_put(resv);
 		}
-
-		mem_type = res->mem_type;
 		spin_unlock(lru_lock);
-		if (!bo_locked)
-			ret = ttm_lru_walk_ticketlock(curs, bo);
+
+		/* Grab a GEM reference to the BO if it isn't already deleted */
+		if (kref_get_unless_zero(&res->bo->base.refcount))
+			curs->bo = res->bo;
 
 		/*
-		 * Note that in between the release of the lru lock and the
-		 * ticketlock, the bo may have switched resource,
-		 * and also memory type, since the resource may have been
-		 * freed and allocated again with a different memory type.
-		 * In that case, just skip it.
+		 * The BO is now locked so it can't be released any more until
+		 * we drop both the lock and the eventual GEM reference.
 		 */
-		curs->bo = bo;
-		if (!ret && bo->resource && bo->resource->mem_type == mem_type)
-			return bo;
-
-		ttm_bo_lru_cursor_cleanup_bo(curs);
-		if (ret && ret != -EALREADY)
-			return ERR_PTR(ret);
-
-		spin_lock(lru_lock);
+		return res->bo;
 	}
-
-	spin_unlock(lru_lock);
-	return res ? bo : NULL;
+	return NULL;
 }
 
 /**
@@ -1046,7 +1048,7 @@ __ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
  */
 struct ttm_buffer_object *ttm_bo_lru_cursor_next(struct ttm_bo_lru_cursor *curs)
 {
-	return __ttm_bo_lru_cursor_next(curs);
+	return __ttm_bo_lru_cursor_iter(curs, false);
 }
 EXPORT_SYMBOL(ttm_bo_lru_cursor_next);
 
@@ -1060,8 +1062,8 @@ EXPORT_SYMBOL(ttm_bo_lru_cursor_next);
  */
 struct ttm_buffer_object *ttm_bo_lru_cursor_first(struct ttm_bo_lru_cursor *curs)
 {
-	ttm_bo_lru_cursor_cleanup_bo(curs);
-	return __ttm_bo_lru_cursor_next(curs);
+	ttm_bo_lru_cursor_cleanup(curs);
+	return __ttm_bo_lru_cursor_iter(curs, true);
 }
 EXPORT_SYMBOL(ttm_bo_lru_cursor_first);
 
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 154d6739256f8..4a765b25472c3 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -714,6 +714,24 @@ ttm_resource_manager_next(struct ttm_resource_cursor *cursor)
 	return NULL;
 }
 
+/* TODO */
+struct ttm_resource *
+ttm_resource_manager_current(struct ttm_resource_cursor *cursor)
+{
+	struct ttm_resource_manager *man = cursor->man;
+	struct ttm_lru_item *lru;
+
+	lockdep_assert_held(&man->bdev->lru_lock);
+
+	lru = &cursor->hitch;
+	list_for_each_entry_continue_reverse(lru, &man->lru[cursor->priority],
+					     link) {
+		if (ttm_lru_item_is_res(lru))
+			return ttm_lru_item_to_res(lru);
+	}
+	return NULL;
+}
+
 /**
  * ttm_lru_first_res_or_null() - Return the first resource on an lru list
  * @head: The list head of the lru list.
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 8310bc3d55f90..30e835414e721 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -488,15 +488,13 @@ struct ttm_bo_lru_cursor {
 	/** @res_curs: Embedded struct ttm_resource_cursor. */
 	struct ttm_resource_cursor res_curs;
 	/**
-	 * @bo: Buffer object pointer if a buffer object is refcounted,
-	 * NULL otherwise.
+	 * @resv: reference to the locked dma_resv
 	 */
-	struct ttm_buffer_object *bo;
+	struct dma_resv *resv;
 	/**
-	 * @needs_unlock: Valid iff @bo != NULL. The bo resv needs
-	 * unlock before the next iteration or after loop exit.
+	 * @bo: TTM BO with GEM reference, NULL for deleted BOs
 	 */
-	bool needs_unlock;
+	struct ttm_buffer_object *bo;
 	/** @arg: Pointer to common BO LRU walk arguments. */
 	struct ttm_lru_walk_arg *arg;
 };
diff --git a/include/drm/ttm/ttm_resource.h b/include/drm/ttm/ttm_resource.h
index a5d386583fb6e..e8e9c8b81ce4b 100644
--- a/include/drm/ttm/ttm_resource.h
+++ b/include/drm/ttm/ttm_resource.h
@@ -488,6 +488,8 @@ struct ttm_resource *
 ttm_resource_manager_first(struct ttm_resource_cursor *cursor);
 struct ttm_resource *
 ttm_resource_manager_next(struct ttm_resource_cursor *cursor);
+struct ttm_resource *
+ttm_resource_manager_current(struct ttm_resource_cursor *cursor);
 
 struct ttm_resource *
 ttm_lru_first_res_or_null(struct list_head *head);
-- 
2.43.0

[PATCH 05/12] drm/ttm: switch to ttm_bo_lru_for_each_reserved_guarded for swapout

[Christian König]

Instead of the walker wrapper use the underlying foreach. Saves us quite
a bunch of complexity and loc.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c     | 64 ++++++--------------------------
 drivers/gpu/drm/ttm/ttm_device.c | 19 ++++++++--
 include/drm/ttm/ttm_bo.h         |  5 +--
 3 files changed, 28 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 8cf266da2bc61..db72fb2fde9ff 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1088,25 +1088,18 @@ int ttm_bo_wait_ctx(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx)
 EXPORT_SYMBOL(ttm_bo_wait_ctx);
 
 /**
- * struct ttm_bo_swapout_walk - Parameters for the swapout walk
+ * ttm_bo_swapout() - Swap out buffer objects on the LRU list to shmem.
+ * @bo: The buffer to swap out.
+ * @ctx: The ttm_operation_ctx governing the swapout operation.
+ * @gfp_flags: The gfp flags used for shmem page allocations.
+ *
+ * Return: The number of bytes actually swapped out, or negative error code
+ * on error.
  */
-struct ttm_bo_swapout_walk {
-	/** @walk: The walk base parameters. */
-	struct ttm_lru_walk walk;
-	/** @gfp_flags: The gfp flags to use for ttm_tt_swapout() */
-	gfp_t gfp_flags;
-	/** @hit_low: Whether we should attempt to swap BO's with low watermark threshold */
-	/** @evict_low: If we cannot swap a bo when @try_low is false (first pass) */
-	bool hit_low, evict_low;
-};
-
-static s64
-ttm_bo_swapout_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *bo)
+s64 ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
+		   gfp_t gfp_flags)
 {
-	struct ttm_place place = { .mem_type = bo->resource->mem_type };
-	struct ttm_bo_swapout_walk *swapout_walk =
-		container_of(walk, typeof(*swapout_walk), walk);
-	struct ttm_operation_ctx *ctx = walk->arg.ctx;
+	struct ttm_place place = {.mem_type = bo->resource->mem_type};
 	struct ttm_device *bdev = bo->bdev;
 	struct ttm_tt *tt = bo->ttm;
 	s64 ret;
@@ -1174,7 +1167,7 @@ ttm_bo_swapout_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *bo)
 		bdev->funcs->swap_notify(bo);
 
 	if (ttm_tt_is_populated(tt)) {
-		ret = ttm_tt_swapout(bdev, tt, swapout_walk->gfp_flags);
+		ret = ttm_tt_swapout(bdev, tt, gfp_flags);
 		if (!ret) {
 			spin_lock(&bdev->lru_lock);
 			ttm_resource_del_bulk_move_unevictable(bo->resource, bo);
@@ -1191,41 +1184,6 @@ ttm_bo_swapout_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *bo)
 	return ret;
 }
 
-const struct ttm_lru_walk_ops ttm_swap_ops = {
-	.process_bo = ttm_bo_swapout_cb,
-};
-
-/**
- * ttm_bo_swapout() - Swap out buffer objects on the LRU list to shmem.
- * @bdev: The ttm device.
- * @ctx: The ttm_operation_ctx governing the swapout operation.
- * @man: The resource manager whose resources / buffer objects are
- * goint to be swapped out.
- * @gfp_flags: The gfp flags used for shmem page allocations.
- * @target: The desired number of pages to swap out.
- *
- * Return: The number of pages actually swapped out, or negative error code
- * on error.
- */
-s64 ttm_bo_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
-		   struct ttm_resource_manager *man, gfp_t gfp_flags,
-		   s64 target)
-{
-	struct ttm_bo_swapout_walk swapout_walk = {
-		.walk = {
-			.ops = &ttm_swap_ops,
-			.arg = {
-				.ctx = ctx,
-				.trylock_only = true,
-			},
-		},
-		.gfp_flags = gfp_flags,
-	};
-
-	return ttm_lru_walk_for_evict(&swapout_walk.walk, bdev, man, target);
-}
-EXPORT_SYMBOL_FOR_TESTS_ONLY(ttm_bo_swapout);
-
 void ttm_bo_tt_destroy(struct ttm_buffer_object *bo)
 {
 	if (bo->ttm == NULL)
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index d3bfb9a696a74..e4188e2ee7ab1 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -171,6 +171,12 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
 		       gfp_t gfp_flags)
 {
 	struct ttm_resource_manager *man;
+	struct ttm_bo_lru_cursor cursor;
+	struct ttm_buffer_object *bo;
+	struct ttm_lru_walk_arg arg = {
+		.ctx = ctx,
+		.trylock_only = true
+	};
 	unsigned i;
 	s64 lret;
 
@@ -179,10 +185,15 @@ int ttm_device_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
 		if (!man || !man->use_tt)
 			continue;
 
-		lret = ttm_bo_swapout(bdev, ctx, man, gfp_flags, 1);
-		/* Can be both positive (num_pages) and negative (error) */
-		if (lret)
-			return lret;
+		ttm_bo_lru_for_each_reserved_guarded(&cursor, man, &arg, bo) {
+			lret = ttm_bo_swapout(bo, ctx, gfp_flags);
+				continue;
+			/* Can be both positive (num_pages) and negative (error) */
+			if (lret && lret != -EBUSY && lret != -EALREADY)
+				return lret;
+		}
+		if (IS_ERR(bo))
+			return PTR_ERR(bo);
 	}
 	return 0;
 }
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 30e835414e721..33dfa61ba882a 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -423,9 +423,8 @@ void *ttm_bo_kmap_try_from_panic(struct ttm_buffer_object *bo, unsigned long pag
 int ttm_bo_vmap(struct ttm_buffer_object *bo, struct iosys_map *map);
 void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct iosys_map *map);
 int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
-s64 ttm_bo_swapout(struct ttm_device *bdev, struct ttm_operation_ctx *ctx,
-		   struct ttm_resource_manager *man, gfp_t gfp_flags,
-		   s64 target);
+s64 ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
+		   gfp_t gfp_flags);
 void ttm_bo_pin(struct ttm_buffer_object *bo);
 void ttm_bo_unpin(struct ttm_buffer_object *bo);
 int ttm_bo_evict_first(struct ttm_device *bdev,
-- 
2.43.0

[PATCH 06/12] drm/ttm: move delete handling into ttm_bo_evict

[Christian König]

Both callers do the same thing, so we can trivially unify that.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index db72fb2fde9ff..e28e11c06ef48 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -363,6 +363,13 @@ static int ttm_bo_evict(struct ttm_buffer_object *bo,
 	struct ttm_place hop;
 	int ret = 0;
 
+	if (bo->deleted) {
+		ret = ttm_bo_wait_ctx(bo, ctx);
+		if (!ret)
+			ttm_bo_cleanup_memtype_use(bo);
+		return ret;
+	}
+
 	memset(&hop, 0, sizeof(hop));
 
 	dma_resv_assert_held(bo->base.resv);
@@ -470,13 +477,7 @@ int ttm_bo_evict_first(struct ttm_device *bdev, struct ttm_resource_manager *man
 	if (!bo->resource || bo->resource->mem_type != mem_type)
 		goto out_bo_moved;
 
-	if (bo->deleted) {
-		ret = ttm_bo_wait_ctx(bo, ctx);
-		if (!ret)
-			ttm_bo_cleanup_memtype_use(bo);
-	} else {
-		ret = ttm_bo_evict(bo, ctx);
-	}
+	ret = ttm_bo_evict(bo, ctx);
 out_bo_moved:
 	dma_resv_unlock(bo->base.resv);
 out_no_lock:
@@ -524,14 +525,7 @@ static s64 ttm_bo_evict_cb(struct ttm_lru_walk *walk, struct ttm_buffer_object *
 	if (bo->pin_count || !bo->bdev->funcs->eviction_valuable(bo, evict_walk->place))
 		return 0;
 
-	if (bo->deleted) {
-		lret = ttm_bo_wait_ctx(bo, walk->arg.ctx);
-		if (!lret)
-			ttm_bo_cleanup_memtype_use(bo);
-	} else {
-		lret = ttm_bo_evict(bo, walk->arg.ctx);
-	}
-
+	lret = ttm_bo_evict(bo, walk->arg.ctx);
 	if (lret)
 		goto out;
 
-- 
2.43.0

[PATCH 07/12] drm/ttm: use ttm_bo_lru_for_each_reserved_guarded in evict_all

[Christian König]

Use the for_each loop to evict all BOs of an resource manager as well.

Greately simplifying the handling and finally allows us to
remove ttm_bo_evict_first().

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo.c       | 51 +-----------------------------
 drivers/gpu/drm/ttm/ttm_resource.c | 22 +++++++++----
 include/drm/ttm/ttm_bo.h           |  1 +
 3 files changed, 17 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index e28e11c06ef48..6af247dba53e5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -355,8 +355,7 @@ static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo,
 	return 0;
 }
 
-static int ttm_bo_evict(struct ttm_buffer_object *bo,
-			struct ttm_operation_ctx *ctx)
+int ttm_bo_evict(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx)
 {
 	struct ttm_resource *evict_mem;
 	struct ttm_placement placement;
@@ -441,54 +440,6 @@ bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 }
 EXPORT_SYMBOL(ttm_bo_eviction_valuable);
 
-/**
- * ttm_bo_evict_first() - Evict the first bo on the manager's LRU list.
- * @bdev: The ttm device.
- * @man: The manager whose bo to evict.
- * @ctx: The TTM operation ctx governing the eviction.
- *
- * Return: 0 if successful or the resource disappeared. Negative error code on error.
- */
-int ttm_bo_evict_first(struct ttm_device *bdev, struct ttm_resource_manager *man,
-		       struct ttm_operation_ctx *ctx)
-{
-	struct ttm_resource_cursor cursor;
-	struct ttm_buffer_object *bo;
-	struct ttm_resource *res;
-	unsigned int mem_type;
-	int ret = 0;
-
-	spin_lock(&bdev->lru_lock);
-	ttm_resource_cursor_init(&cursor, man);
-	res = ttm_resource_manager_first(&cursor);
-	ttm_resource_cursor_fini(&cursor);
-	if (!res) {
-		ret = -ENOENT;
-		goto out_no_ref;
-	}
-	bo = res->bo;
-	if (!ttm_bo_get_unless_zero(bo))
-		goto out_no_ref;
-	mem_type = res->mem_type;
-	spin_unlock(&bdev->lru_lock);
-	ret = ttm_bo_reserve(bo, ctx->interruptible, ctx->no_wait_gpu, NULL);
-	if (ret)
-		goto out_no_lock;
-	if (!bo->resource || bo->resource->mem_type != mem_type)
-		goto out_bo_moved;
-
-	ret = ttm_bo_evict(bo, ctx);
-out_bo_moved:
-	dma_resv_unlock(bo->base.resv);
-out_no_lock:
-	ttm_bo_put(bo);
-	return ret;
-
-out_no_ref:
-	spin_unlock(&bdev->lru_lock);
-	return ret;
-}
-
 /**
  * struct ttm_bo_evict_walk - Parameters for the evict walk.
  */
diff --git a/drivers/gpu/drm/ttm/ttm_resource.c b/drivers/gpu/drm/ttm/ttm_resource.c
index 4a765b25472c3..e686fc2849d99 100644
--- a/drivers/gpu/drm/ttm/ttm_resource.c
+++ b/drivers/gpu/drm/ttm/ttm_resource.c
@@ -561,17 +561,25 @@ EXPORT_SYMBOL(ttm_resource_manager_init);
 int ttm_resource_manager_evict_all(struct ttm_device *bdev,
 				   struct ttm_resource_manager *man)
 {
-	struct ttm_operation_ctx ctx = { };
+	struct ttm_bo_lru_cursor cursor;
+	struct ttm_operation_ctx ctx = {
+		.interruptible = false,
+		.no_wait_gpu = false,
+	};
+	struct ttm_lru_walk_arg arg = {
+		.ctx = &ctx,
+		.trylock_only = true
+	};
+	struct ttm_buffer_object *bo;
 	struct dma_fence *fence;
 	int ret, i;
 
-	do {
-		ret = ttm_bo_evict_first(bdev, man, &ctx);
+	ttm_bo_lru_for_each_reserved_guarded(&cursor, man, &arg, bo) {
+		ret = ttm_bo_evict(bo, &ctx);
+		if (ret)
+			return ret;
 		cond_resched();
-	} while (!ret);
-
-	if (ret && ret != -ENOENT)
-		return ret;
+	}
 
 	ret = 0;
 
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 33dfa61ba882a..026ab044b0202 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -404,6 +404,7 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
 void ttm_bo_fini(struct ttm_buffer_object *bo);
 void ttm_bo_set_bulk_move(struct ttm_buffer_object *bo,
 			  struct ttm_lru_bulk_move *bulk);
+int ttm_bo_evict(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx);
 bool ttm_bo_eviction_valuable(struct ttm_buffer_object *bo,
 			      const struct ttm_place *place);
 int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo,
-- 
2.43.0

[PATCH 08/12] drm/ttm: use dma_resv reference in ttm_device_clear_lru_dma_mappings

[Christian König]

Instead of trying to grab a BO reference.

Also lock the dma_resv object, that TT unpopulate is done without
holding the lock looks extremely suspicious.

Only compile tested!

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_device.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index e4188e2ee7ab1..be6fc42772f9d 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -293,19 +293,29 @@ static void ttm_device_clear_lru_dma_mappings(struct ttm_device *bdev,
 
 	spin_lock(&bdev->lru_lock);
 	while ((res = ttm_lru_first_res_or_null(list))) {
-		struct ttm_buffer_object *bo = res->bo;
+		struct dma_resv *resv;
 
 		/* Take ref against racing releases once lru_lock is unlocked */
-		if (!ttm_bo_get_unless_zero(bo))
-			continue;
+		resv = dma_resv_get(res->bo->base.resv);
+		spin_unlock(&bdev->lru_lock);
+
+		dma_resv_lock(resv, NULL);
 
-		list_del_init(&bo->resource->lru.link);
+		/* Double check that res and bo is still valid */
+		spin_lock(&bdev->lru_lock);
+		if (res != ttm_lru_first_res_or_null(list)) {
+			dma_resv_unlock(resv);
+			dma_resv_put(resv);
+			continue;
+		}
+		list_del_init(&res->lru.link);
 		spin_unlock(&bdev->lru_lock);
 
-		if (bo->ttm)
-			ttm_tt_unpopulate(bo->bdev, bo->ttm);
+		if (res->bo->ttm)
+			ttm_tt_unpopulate(res->bo->bdev, res->bo->ttm);
 
-		ttm_bo_put(bo);
+		dma_resv_unlock(resv);
+		dma_resv_put(resv);
 		spin_lock(&bdev->lru_lock);
 	}
 	spin_unlock(&bdev->lru_lock);
-- 
2.43.0

[PATCH 09/12] drm/ttm: nuke buffer refcounting

[Christian König]

TTMs buffer reference is finally useles.

WIP, add more commit message

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_ttm.c |   2 +-
 drivers/gpu/drm/ttm/ttm_bo.c            | 122 ++++++++++--------------
 drivers/gpu/drm/ttm/ttm_bo_internal.h   |  60 ------------
 drivers/gpu/drm/ttm/ttm_bo_util.c       |  11 +--
 drivers/gpu/drm/ttm/ttm_device.c        |   1 -
 drivers/gpu/drm/xe/xe_bo.c              |   2 +-
 include/drm/ttm/ttm_bo.h                |  22 +++--
 7 files changed, 69 insertions(+), 151 deletions(-)
 delete mode 100644 drivers/gpu/drm/ttm/ttm_bo_internal.h

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index df3fcc2b1248e..caccac6c7bce7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -939,7 +939,7 @@ void i915_ttm_adjust_lru(struct drm_i915_gem_object *obj)
 	 * Don't manipulate the TTM LRUs while in TTM bo destruction.
 	 * We're called through i915_ttm_delete_mem_notify().
 	 */
-	if (!kref_read(&bo->kref))
+	if (ttm_bo_is_deleted(bo))
 		return;
 
 	/*
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 6af247dba53e5..3dac10f199322 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -50,7 +50,6 @@
 #include <linux/dma-resv.h>
 
 #include "ttm_module.h"
-#include "ttm_bo_internal.h"
 
 static void ttm_bo_mem_space_debug(struct ttm_buffer_object *bo,
 					struct ttm_placement *placement)
@@ -243,92 +242,76 @@ static void ttm_bo_delayed_delete(struct work_struct *work)
 	dma_resv_lock(bo->base.resv, NULL);
 	ttm_bo_cleanup_memtype_use(bo);
 	dma_resv_unlock(bo->base.resv);
-	ttm_bo_put(bo);
+
+	atomic_dec(&ttm_glob.bo_count);
+	bo->destroy(bo);
 }
 
-static void ttm_bo_release(struct kref *kref)
+void ttm_bo_fini(struct ttm_buffer_object *bo)
 {
-	struct ttm_buffer_object *bo =
-	    container_of(kref, struct ttm_buffer_object, kref);
 	struct ttm_device *bdev = bo->bdev;
 	int ret;
 
 	WARN_ON_ONCE(bo->pin_count);
 	WARN_ON_ONCE(bo->bulk_move);
 
-	if (!bo->deleted) {
-		ret = ttm_bo_individualize_resv(bo);
-		if (ret) {
-			/* Last resort, if we fail to allocate memory for the
-			 * fences block for the BO to become idle
-			 */
-			dma_resv_wait_timeout(bo->base.resv,
-					      DMA_RESV_USAGE_BOOKKEEP, false,
-					      30 * HZ);
-		}
-
-		if (bdev->funcs->release_notify)
-			bdev->funcs->release_notify(bo);
+	ret = ttm_bo_individualize_resv(bo);
+	if (ret) {
+		/* Last resort, if we fail to allocate memory for the
+		 * fences block for the BO to become idle
+		 */
+		dma_resv_wait_timeout(bo->base.resv,
+				      DMA_RESV_USAGE_BOOKKEEP, false,
+				      30 * HZ);
+	}
 
-		drm_vma_offset_remove(bdev->vma_manager, &bo->base.vma_node);
-		ttm_mem_io_free(bdev, bo->resource);
+	if (bdev->funcs->release_notify)
+		bdev->funcs->release_notify(bo);
 
-		if (!dma_resv_test_signaled(&bo->base._resv,
-					    DMA_RESV_USAGE_BOOKKEEP) ||
-		    (want_init_on_free() && (bo->ttm != NULL)) ||
-		    bo->type == ttm_bo_type_sg ||
-		    !dma_resv_trylock(bo->base.resv)) {
-			/* The BO is not idle, resurrect it for delayed destroy */
-			ttm_bo_flush_all_fences(bo);
-			bo->deleted = true;
+	drm_vma_offset_remove(bdev->vma_manager, &bo->base.vma_node);
+	ttm_mem_io_free(bdev, bo->resource);
 
-			spin_lock(&bdev->lru_lock);
+	if (!dma_resv_test_signaled(&bo->base._resv,
+				    DMA_RESV_USAGE_BOOKKEEP) ||
+	    (want_init_on_free() && (bo->ttm != NULL)) ||
+	    bo->type == ttm_bo_type_sg ||
+	    !dma_resv_trylock(bo->base.resv)) {
 
-			/*
-			 * Make pinned bos immediately available to
-			 * shrinkers, now that they are queued for
-			 * destruction.
-			 *
-			 * FIXME: QXL is triggering this. Can be removed when the
-			 * driver is fixed.
-			 */
-			if (bo->pin_count) {
-				bo->pin_count = 0;
-				ttm_resource_move_to_lru_tail(bo->resource);
-			}
+		/* The BO is not idle, queue it for delayed destroy */
+		ttm_bo_flush_all_fences(bo);
 
-			kref_init(&bo->kref);
+		/*
+		 * Make pinned bos immediately available to
+		 * shrinkers, now that they are queued for
+		 * destruction.
+		 *
+		 * FIXME: QXL is triggering this. Can be removed when the
+		 * driver is fixed.
+		 */
+		if (bo->pin_count) {
+			spin_lock(&bdev->lru_lock);
+			bo->pin_count = 0;
+			ttm_resource_move_to_lru_tail(bo->resource);
 			spin_unlock(&bdev->lru_lock);
-
-			INIT_WORK(&bo->delayed_delete, ttm_bo_delayed_delete);
-
-			/* Schedule the worker on the closest NUMA node. This
-			 * improves performance since system memory might be
-			 * cleared on free and that is best done on a CPU core
-			 * close to it.
-			 */
-			queue_work_node(bdev->pool.nid, bdev->wq, &bo->delayed_delete);
-			return;
 		}
 
-		ttm_bo_cleanup_memtype_use(bo);
-		dma_resv_unlock(bo->base.resv);
+		INIT_WORK(&bo->delayed_delete, ttm_bo_delayed_delete);
+
+		/* Schedule the worker on the closest NUMA node. This
+		 * improves performance since system memory might be
+		 * cleared on free and that is best done on a CPU core
+		 * close to it.
+		 */
+		queue_work_node(bdev->pool.nid, bdev->wq, &bo->delayed_delete);
+		return;
 	}
 
+	ttm_bo_cleanup_memtype_use(bo);
+	dma_resv_unlock(bo->base.resv);
+
 	atomic_dec(&ttm_glob.bo_count);
 	bo->destroy(bo);
 }
-
-/* TODO: remove! */
-void ttm_bo_put(struct ttm_buffer_object *bo)
-{
-	kref_put(&bo->kref, ttm_bo_release);
-}
-
-void ttm_bo_fini(struct ttm_buffer_object *bo)
-{
-	ttm_bo_put(bo);
-}
 EXPORT_SYMBOL(ttm_bo_fini);
 
 static int ttm_bo_bounce_temp_buffer(struct ttm_buffer_object *bo,
@@ -362,7 +345,7 @@ int ttm_bo_evict(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx)
 	struct ttm_place hop;
 	int ret = 0;
 
-	if (bo->deleted) {
+	if (ttm_bo_is_deleted(bo)) {
 		ret = ttm_bo_wait_ctx(bo, ctx);
 		if (!ret)
 			ttm_bo_cleanup_memtype_use(bo);
@@ -569,7 +552,6 @@ static int ttm_bo_evict_alloc(struct ttm_device *bdev,
 void ttm_bo_pin(struct ttm_buffer_object *bo)
 {
 	dma_resv_assert_held(bo->base.resv);
-	WARN_ON_ONCE(!kref_read(&bo->kref));
 	spin_lock(&bo->bdev->lru_lock);
 	if (bo->resource)
 		ttm_resource_del_bulk_move(bo->resource, bo);
@@ -588,7 +570,6 @@ EXPORT_SYMBOL(ttm_bo_pin);
 void ttm_bo_unpin(struct ttm_buffer_object *bo)
 {
 	dma_resv_assert_held(bo->base.resv);
-	WARN_ON_ONCE(!kref_read(&bo->kref));
 	if (WARN_ON_ONCE(!bo->pin_count))
 		return;
 
@@ -879,7 +860,6 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo,
 {
 	int ret;
 
-	kref_init(&bo->kref);
 	bo->bdev = bdev;
 	bo->type = type;
 	bo->page_alignment = alignment;
@@ -920,7 +900,7 @@ int ttm_bo_init_reserved(struct ttm_device *bdev, struct ttm_buffer_object *bo,
 		dma_resv_unlock(bo->base.resv);
 
 err_put:
-	ttm_bo_put(bo);
+	ttm_bo_fini(bo);
 	return ret;
 }
 EXPORT_SYMBOL(ttm_bo_init_reserved);
@@ -1066,7 +1046,7 @@ s64 ttm_bo_swapout(struct ttm_buffer_object *bo, struct ttm_operation_ctx *ctx,
 		goto out;
 	}
 
-	if (bo->deleted) {
+	if (ttm_bo_is_deleted(bo)) {
 		pgoff_t num_pages = tt->num_pages;
 
 		ret = ttm_bo_wait_ctx(bo, ctx);
diff --git a/drivers/gpu/drm/ttm/ttm_bo_internal.h b/drivers/gpu/drm/ttm/ttm_bo_internal.h
deleted file mode 100644
index e0d48eac74b03..0000000000000
--- a/drivers/gpu/drm/ttm/ttm_bo_internal.h
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright 2018 Advanced Micro Devices, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- *
- *
- */
-
-#ifndef _TTM_BO_INTERNAL_H_
-#define _TTM_BO_INTERNAL_H_
-
-#include <drm/ttm/ttm_bo.h>
-
-/**
- * ttm_bo_get - reference a struct ttm_buffer_object
- *
- * @bo: The buffer object.
- */
-static inline void ttm_bo_get(struct ttm_buffer_object *bo)
-{
-	kref_get(&bo->kref);
-}
-
-/**
- * ttm_bo_get_unless_zero - reference a struct ttm_buffer_object unless
- * its refcount has already reached zero.
- * @bo: The buffer object.
- *
- * Used to reference a TTM buffer object in lookups where the object is removed
- * from the lookup structure during the destructor and for RCU lookups.
- *
- * Returns: @bo if the referencing was successful, NULL otherwise.
- */
-static inline __must_check struct ttm_buffer_object *
-ttm_bo_get_unless_zero(struct ttm_buffer_object *bo)
-{
-	if (!kref_get_unless_zero(&bo->kref))
-		return NULL;
-	return bo;
-}
-
-void ttm_bo_put(struct ttm_buffer_object *bo);
-
-#endif
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 1f7361604b552..1bdd69643c313 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -39,8 +39,6 @@
 
 #include <drm/drm_cache.h>
 
-#include "ttm_bo_internal.h"
-
 struct ttm_transfer_obj {
 	struct ttm_buffer_object base;
 	struct ttm_buffer_object *bo;
@@ -208,7 +206,7 @@ static void ttm_transfered_destroy(struct ttm_buffer_object *bo)
 
 	fbo = container_of(bo, struct ttm_transfer_obj, base);
 	dma_resv_put(&fbo->base.base._resv);
-	ttm_bo_put(fbo->bo);
+	drm_gem_object_put(&fbo->bo->base);
 	kfree(fbo);
 }
 
@@ -247,7 +245,6 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
 	atomic_inc(&ttm_glob.bo_count);
 	drm_vma_node_reset(&fbo->base.base.vma_node);
 
-	kref_init(&fbo->base.kref);
 	fbo->base.destroy = &ttm_transfered_destroy;
 	fbo->base.pin_count = 0;
 	if (bo->type != ttm_bo_type_sg)
@@ -273,7 +270,7 @@ static int ttm_buffer_object_transfer(struct ttm_buffer_object *bo,
 		fbo->base.bulk_move = NULL;
 	}
 
-	ttm_bo_get(bo);
+	drm_gem_object_get(&bo->base);
 	fbo->bo = bo;
 
 	ttm_bo_move_to_lru_tail_unlocked(&fbo->base);
@@ -632,7 +629,7 @@ static int ttm_bo_move_to_ghost(struct ttm_buffer_object *bo,
 		bo->ttm = NULL;
 
 	dma_resv_unlock(&ghost_obj->base._resv);
-	ttm_bo_put(ghost_obj);
+	ttm_bo_fini(ghost_obj);
 	return 0;
 }
 
@@ -809,7 +806,7 @@ int ttm_bo_pipeline_gutting(struct ttm_buffer_object *bo)
 	}
 
 	dma_resv_unlock(&ghost->base._resv);
-	ttm_bo_put(ghost);
+	ttm_bo_fini(ghost);
 	bo->ttm = ttm;
 	return 0;
 
diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
index be6fc42772f9d..d0a0f414bed0c 100644
--- a/drivers/gpu/drm/ttm/ttm_device.c
+++ b/drivers/gpu/drm/ttm/ttm_device.c
@@ -38,7 +38,6 @@
 #include <drm/ttm/ttm_placement.h>
 
 #include "ttm_module.h"
-#include "ttm_bo_internal.h"
 
 /*
  * ttm_global_mutex - protecting the global state
diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index 85e6d9a0f575b..c62c7bd3b4605 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -1651,7 +1651,7 @@ static bool xe_ttm_bo_lock_in_destructor(struct ttm_buffer_object *ttm_bo)
 	struct xe_device *xe = ttm_to_xe_device(ttm_bo->bdev);
 	bool locked;
 
-	xe_assert(xe, !kref_read(&ttm_bo->kref));
+	xe_assert(xe, ttm_bo_is_deleted(ttm_bo));
 
 	/*
 	 * We can typically only race with TTM trylocking under the
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index 026ab044b0202..e1221e3be7bda 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -33,7 +33,6 @@
 
 #include <drm/drm_gem.h>
 
-#include <linux/kref.h>
 #include <linux/list.h>
 
 #include "ttm_device.h"
@@ -78,11 +77,8 @@ enum ttm_bo_type {
  * @type: The bo type.
  * @page_alignment: Page alignment.
  * @destroy: Destruction function. If NULL, kfree is used.
- * @kref: Reference count of this buffer object. When this refcount reaches
- * zero, the object is destroyed or put on the delayed delete list.
  * @resource: structure describing current placement.
  * @ttm: TTM structure holding system pages.
- * @deleted: True if the object is only a zombie and already deleted.
  * @bulk_move: The bulk move object.
  * @priority: Priority for LRU, BOs with lower priority are evicted first.
  * @pin_count: Pin count.
@@ -109,17 +105,11 @@ struct ttm_buffer_object {
 	uint32_t page_alignment;
 	void (*destroy) (struct ttm_buffer_object *);
 
-	/*
-	* Members not needing protection.
-	*/
-	struct kref kref;
-
 	/*
 	 * Members protected by the bo::resv::reserved lock.
 	 */
 	struct ttm_resource *resource;
 	struct ttm_tt *ttm;
-	bool deleted;
 	struct ttm_lru_bulk_move *bulk_move;
 	unsigned priority;
 	unsigned pin_count;
@@ -378,6 +368,18 @@ static inline void ttm_bo_unreserve(struct ttm_buffer_object *bo)
 	dma_resv_unlock(bo->base.resv);
 }
 
+/**
+ * ttm_bo_is_deleted - test if buffer is already deleted
+ * @bo: the struct ttm_buffer_object to test
+ *
+ * Returns true if the buffer is already deleted and only waiting for
+ * destruction.
+ */
+static inline bool ttm_bo_is_deleted(struct ttm_buffer_object *bo)
+{
+	return !kref_read(&bo->base.refcount);
+}
+
 /**
  * ttm_kmap_obj_virtual
  *
-- 
2.43.0

Re: [PATCH 09/12] drm/ttm: nuke buffer refcounting

[Danilo Krummrich]

On Fri Jul 10, 2026 at 8:52 PM CEST, =?UTF-8?q?Christian=20K=C3=B6nig?= wrote:
> +/**
> + * ttm_bo_is_deleted - test if buffer is already deleted
> + * @bo: the struct ttm_buffer_object to test
> + *
> + * Returns true if the buffer is already deleted and only waiting for
> + * destruction.
> + */
> +static inline bool ttm_bo_is_deleted(struct ttm_buffer_object *bo)
> +{
> +	return !kref_read(&bo->base.refcount);
> +}

This is an anti-pattern, branch decisions cannot rely on an atomic reference
count unless additional invariants are upheld that otherwise guarantee that the
read values does not instantly become meaningless.

Something like this should at least document in which context and under which
conditions it is valid to rely on the returned value.

[PATCH 10/12] drm/exec: add drm_exec_lock_resv function

[Christian König]

Restructure the drm_exec object to work with dma_resv references instead
of GEM object references.

Add the new function dma_exec_lock_resv() to lock individual dma_resv
objects and so allow higher level implementations to handle contention
purely on dma_resv objects.

WIP! Don't commit like that!

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/drm_exec.c | 75 ++++++++++++++++++++++----------------
 drivers/gpu/drm/drm_gem.c  |  2 +
 include/drm/drm_exec.h     |  9 +++--
 3 files changed, 50 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
index fa923852fae45..382bf7bcd5ff3 100644
--- a/drivers/gpu/drm/drm_exec.c
+++ b/drivers/gpu/drm/drm_exec.c
@@ -58,8 +58,11 @@ static void drm_exec_unlock_all(struct drm_exec *exec)
 		drm_gem_object_put(obj);
 	}
 
-	drm_gem_object_put(exec->prelocked);
-	exec->prelocked = NULL;
+	if (exec->prelocked) {
+		dma_resv_unlock(exec->prelocked);
+		dma_resv_put(exec->prelocked);
+		exec->prelocked = NULL;
+	}
 }
 
 /**
@@ -101,7 +104,7 @@ void drm_exec_fini(struct drm_exec *exec)
 	drm_exec_unlock_all(exec);
 	kvfree(exec->objects);
 	if (exec->contended != DRM_EXEC_DUMMY) {
-		drm_gem_object_put(exec->contended);
+		dma_resv_put(exec->contended);
 		ww_acquire_fini(&exec->ticket);
 	}
 }
@@ -158,50 +161,41 @@ static int drm_exec_obj_locked(struct drm_exec *exec,
 /* Make sure the contended object is locked first */
 static int drm_exec_lock_contended(struct drm_exec *exec)
 {
-	struct drm_gem_object *obj = exec->contended;
+	struct dma_resv *resv = exec->contended;
 	int ret;
 
-	if (likely(!obj))
+	if (likely(!resv))
 		return 0;
 
 	/* Always cleanup the contention so that error handling can kick in */
 	exec->contended = NULL;
 	if (exec->flags & DRM_EXEC_INTERRUPTIBLE_WAIT) {
-		ret = dma_resv_lock_slow_interruptible(obj->resv,
-						       &exec->ticket);
+		ret = dma_resv_lock_slow_interruptible(resv, &exec->ticket);
 		if (unlikely(ret))
 			goto error_dropref;
 	} else {
-		dma_resv_lock_slow(obj->resv, &exec->ticket);
+		dma_resv_lock_slow(resv, &exec->ticket);
 	}
 
-	ret = drm_exec_obj_locked(exec, obj);
-	if (unlikely(ret))
-		goto error_unlock;
-
-	exec->prelocked = obj;
+	exec->prelocked = resv;
 	return 0;
 
-error_unlock:
-	dma_resv_unlock(obj->resv);
-
 error_dropref:
-	drm_gem_object_put(obj);
+	dma_resv_put(resv);
 	return ret;
 }
 
 /**
- * drm_exec_lock_obj - lock a GEM object for use
+ * drm_exec_lock_resv - lock a dma_resv object
  * @exec: the drm_exec object with the state
- * @obj: the GEM object to lock
+ * @resv: the dma_resv object to lock
  *
- * Lock a GEM object for use and grab a reference to it.
+ * Lock a dma_resv object for use or grab a reference to it on contention.
  *
  * Returns: -EDEADLK if a contention is detected, -EALREADY when object is
- * already locked (can be suppressed by setting the DRM_EXEC_IGNORE_DUPLICATES
- * flag), -ENOMEM when memory allocation failed and zero for success.
+ * already locked, -ENOMEM when memory allocation failed and zero for success.
  */
-int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj)
+int drm_exec_lock_resv(struct drm_exec *exec, struct dma_resv *resv)
 {
 	int ret;
 
@@ -209,22 +203,39 @@ int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj)
 	if (unlikely(ret))
 		return ret;
 
-	if (exec->prelocked == obj) {
-		drm_gem_object_put(exec->prelocked);
+	if (exec->prelocked == resv) {
+		dma_resv_put(exec->prelocked);
 		exec->prelocked = NULL;
 		return 0;
 	}
 
 	if (exec->flags & DRM_EXEC_INTERRUPTIBLE_WAIT)
-		ret = dma_resv_lock_interruptible(obj->resv, &exec->ticket);
+		ret = dma_resv_lock_interruptible(resv, &exec->ticket);
 	else
-		ret = dma_resv_lock(obj->resv, &exec->ticket);
+		ret = dma_resv_lock(resv, &exec->ticket);
 
-	if (unlikely(ret == -EDEADLK)) {
-		drm_gem_object_get(obj);
-		exec->contended = obj;
-		return -EDEADLK;
-	}
+	if (unlikely(ret == -EDEADLK))
+		exec->contended = dma_resv_get(resv);
+	return ret;
+}
+EXPORT_SYMBOL(drm_exec_lock_resv);
+
+/**
+ * drm_exec_lock_obj - lock a GEM object for use
+ * @exec: the drm_exec object with the state
+ * @obj: the GEM object to lock
+ *
+ * Lock a GEM object for use and grab a reference to it.
+ *
+ * Returns: -EDEADLK if a contention is detected, -EALREADY when object is
+ * already locked (can be suppressed by setting the DRM_EXEC_IGNORE_DUPLICATES
+ * flag), -ENOMEM when memory allocation failed and zero for success.
+ */
+int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj)
+{
+	int ret;
+
+	ret = drm_exec_lock_resv(exec, obj->resv);
 
 	if (unlikely(ret == -EALREADY) &&
 	    exec->flags & DRM_EXEC_IGNORE_DUPLICATES)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index bbcbd25f014f0..f5cf9ad596a67 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -229,6 +229,8 @@ void drm_gem_private_object_init(struct drm_device *dev,
 	obj->size = size;
 	mutex_init(&obj->gpuva.lock);
 	dma_resv_init(&obj->_resv);
+
+	/* TODO: This needs to go away for drm_exec to work correctly!!! */
 	if (!obj->resv)
 		obj->resv = dma_resv_get(&obj->_resv);
 
diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
index 8725ba92ff916..9daedb676d7b1 100644
--- a/include/drm/drm_exec.h
+++ b/include/drm/drm_exec.h
@@ -47,14 +47,14 @@ struct drm_exec {
 	struct drm_gem_object	**objects;
 
 	/**
-	 * @contended: contended GEM object we backed off for
+	 * @contended: contended dma_resv object we backed off for
 	 */
-	struct drm_gem_object	*contended;
+	struct dma_resv		*contended;
 
 	/**
-	 * @prelocked: already locked GEM object due to contention
+	 * @prelocked: already locked dma_resv object due to contention
 	 */
-	struct drm_gem_object *prelocked;
+	struct dma_resv		*prelocked;
 };
 
 /**
@@ -175,6 +175,7 @@ static inline struct ww_acquire_ctx *drm_exec_ticket(struct drm_exec *exec)
 void drm_exec_init(struct drm_exec *exec, u32 flags, unsigned nr);
 void drm_exec_fini(struct drm_exec *exec);
 bool drm_exec_cleanup(struct drm_exec *exec);
+int drm_exec_lock_resv(struct drm_exec *exec, struct dma_resv *resv);
 int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object *obj);
 void drm_exec_unlock_obj(struct drm_exec *exec, struct drm_gem_object *obj);
 int drm_exec_prepare_obj(struct drm_exec *exec, struct drm_gem_object *obj,
-- 
2.43.0

[PATCH 11/12] drm/ttm: support using drm_exec during eviction v4

[Christian König]

Allow specifying a drm_exec object in TTMs operation context which is
used to lock objects during eviction.

This allows to handle deadlocks much more gracefully and with that
avoid returning -ENOMEM on heavily contended domains.

v2: rebased on top of Thomas work
v3: rebased again
v4: adjust to dma_resv changes

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 12 ++++++++----
 include/drm/ttm/ttm_bo.h          |  5 +++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 1bdd69643c313..570640ae79d84 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -38,6 +38,7 @@
 #include <drm/ttm/ttm_tt.h>
 
 #include <drm/drm_cache.h>
+#include <drm/drm_exec.h>
 
 struct ttm_transfer_obj {
 	struct ttm_buffer_object base;
@@ -839,7 +840,9 @@ static int ttm_lru_walk_ticketlock(struct ttm_bo_lru_cursor *curs,
 	struct ttm_lru_walk_arg *arg = curs->arg;
 	int ret;
 
-	if (arg->ctx->interruptible)
+	if (arg->ctx->exec)
+		ret = drm_exec_lock_resv(arg->ctx->exec, resv);
+	else if (arg->ctx->interruptible)
 		ret = dma_resv_lock_interruptible(resv, arg->ticket);
 	else
 		ret = dma_resv_lock(resv, arg->ticket);
@@ -853,7 +856,8 @@ static int ttm_lru_walk_ticketlock(struct ttm_bo_lru_cursor *curs,
 		 * trylocking for this walk.
 		 */
 		arg->ticket = NULL;
-	} else if (ret == -EDEADLK) {
+
+	} else if (!arg->ctx->exec && ret == -EDEADLK) {
 		/* Caller needs to exit the ww transaction. */
 		ret = -ENOSPC;
 	}
@@ -993,8 +997,8 @@ __ttm_bo_lru_cursor_iter(struct ttm_bo_lru_cursor *curs, bool first)
 		if (!ttm_lru_walk_trylock(curs, res->bo->base.resv)) {
 			struct dma_resv *resv;
 
-			if (!arg->ticket || arg->ctx->no_wait_gpu ||
-			    arg->trylock_only) {
+			if ((!arg->ticket || arg->ctx->no_wait_gpu ||
+			     arg->trylock_only) && !arg->ctx->exec) {
 				spin_unlock(lru_lock);
 				continue;
 			}
diff --git a/include/drm/ttm/ttm_bo.h b/include/drm/ttm/ttm_bo.h
index e1221e3be7bda..0ffa84a5caa65 100644
--- a/include/drm/ttm/ttm_bo.h
+++ b/include/drm/ttm/ttm_bo.h
@@ -186,6 +186,11 @@ struct ttm_operation_ctx {
 	 * @bytes_moved: Statistics on how many bytes have been moved.
 	 */
 	uint64_t bytes_moved;
+	/*
+	 * @exec: optional drm_exec object to use for locking and contention
+	 * handling
+	 */
+	struct drm_exec *exec;
 };
 
 struct ttm_lru_walk;
-- 
2.43.0

[PATCH 12/12] drm/amdgpu: use drm_exec during BO validation

[Christian König]

This allows to detect deadlocks happening because of resource
constraints.

Especially submissions which want to use all of GDS doesn't result in
sporadic -ENOMEM any more.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 82 ++++++++++++++------------
 1 file changed, 44 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index c2e6495a28bc5..17223e295ff25 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -793,7 +793,7 @@ static int amdgpu_cs_bo_validate(void *param, struct amdgpu_bo *bo)
 	struct ttm_operation_ctx ctx = {
 		.interruptible = true,
 		.no_wait_gpu = false,
-		.resv = bo->tbo.base.resv
+		.exec = &p->exec,
 	};
 	uint32_t domain;
 	int r;
@@ -845,7 +845,10 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 				union drm_amdgpu_cs *cs)
 {
 	struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
-	struct ttm_operation_ctx ctx = { true, false };
+	struct ttm_operation_ctx ctx = {
+		.interruptible =true,
+		.exec = &p->exec
+	};
 	struct amdgpu_vm *vm = &fpriv->vm;
 	struct amdgpu_bo_list_entry *e;
 	struct drm_gem_object *obj;
@@ -899,6 +902,11 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 		e->user_invalidated = userpage_invalidated;
 	}
 
+	amdgpu_cs_get_threshold_for_moves(p->adev, &p->bytes_moved_threshold,
+					  &p->bytes_moved_vis_threshold);
+	p->bytes_moved = 0;
+	p->bytes_moved_vis = 0;
+
 	drm_exec_until_all_locked(&p->exec) {
 		r = amdgpu_vm_lock_pd(&fpriv->vm, &p->exec, 1 + p->gang_size);
 		drm_exec_retry_on_contention(&p->exec);
@@ -922,47 +930,48 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 			if (unlikely(r))
 				goto out_free_user_pages;
 		}
-	}
 
-	amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
-		struct mm_struct *usermm;
+		amdgpu_bo_list_for_each_userptr_entry(e, p->bo_list) {
+			struct mm_struct *usermm;
 
-		usermm = amdgpu_ttm_tt_get_usermm(e->bo->tbo.ttm);
-		if (usermm && usermm != current->mm) {
-			r = -EPERM;
-			goto out_free_user_pages;
-		}
-
-		if (amdgpu_ttm_tt_is_userptr(e->bo->tbo.ttm) &&
-		    e->user_invalidated) {
-			amdgpu_bo_placement_from_domain(e->bo,
-							AMDGPU_GEM_DOMAIN_CPU);
-			r = ttm_bo_validate(&e->bo->tbo, &e->bo->placement,
-					    &ctx);
-			if (r)
+			usermm = amdgpu_ttm_tt_get_usermm(e->bo->tbo.ttm);
+			if (usermm && usermm != current->mm) {
+				r = -EPERM;
 				goto out_free_user_pages;
+			}
 
-			amdgpu_ttm_tt_set_user_pages(e->bo->tbo.ttm,
-						     e->range);
+			if (amdgpu_ttm_tt_is_userptr(e->bo->tbo.ttm) &&
+			    e->user_invalidated) {
+				amdgpu_bo_placement_from_domain(e->bo,
+								AMDGPU_GEM_DOMAIN_CPU);
+				r = ttm_bo_validate(&e->bo->tbo, &e->bo->placement,
+						    &ctx);
+				drm_exec_retry_on_contention(&p->exec);
+				if (r)
+					goto out_free_user_pages;
+
+				amdgpu_ttm_tt_set_user_pages(e->bo->tbo.ttm,
+							     e->range);
+			}
 		}
-	}
 
-	amdgpu_cs_get_threshold_for_moves(p->adev, &p->bytes_moved_threshold,
-					  &p->bytes_moved_vis_threshold);
-	p->bytes_moved = 0;
-	p->bytes_moved_vis = 0;
+		r = amdgpu_vm_validate(p->adev, &fpriv->vm, NULL,
+				       amdgpu_cs_bo_validate, p);
+		drm_exec_retry_on_contention(&p->exec);
+		if (r) {
+			drm_err(adev_to_drm(p->adev), "amdgpu_vm_validate() failed.\n");
+			goto out_free_user_pages;
+		}
 
-	r = amdgpu_vm_validate(p->adev, &fpriv->vm, NULL,
-			       amdgpu_cs_bo_validate, p);
-	if (r) {
-		drm_err(adev_to_drm(p->adev), "amdgpu_vm_validate() failed.\n");
-		goto out_free_user_pages;
-	}
+		drm_exec_for_each_locked_object(&p->exec, obj) {
+			r = amdgpu_cs_bo_validate(p, gem_to_amdgpu_bo(obj));
+			drm_exec_retry_on_contention(&p->exec);
+			if (unlikely(r))
+				goto out_free_user_pages;
 
-	drm_exec_for_each_locked_object(&p->exec, obj) {
-		r = amdgpu_cs_bo_validate(p, gem_to_amdgpu_bo(obj));
-		if (unlikely(r))
-			goto out_free_user_pages;
+			amdgpu_cs_report_moved_bytes(p->adev, p->bytes_moved,
+						     p->bytes_moved_vis);
+		}
 	}
 
 	if (p->uf_bo) {
@@ -973,9 +982,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
 		p->gang_leader->uf_addr += amdgpu_bo_gpu_offset(p->uf_bo);
 	}
 
-	amdgpu_cs_report_moved_bytes(p->adev, p->bytes_moved,
-				     p->bytes_moved_vis);
-
 	for (i = 0; i < p->gang_size; ++i)
 		amdgpu_job_set_resources(p->jobs[i], p->bo_list->gds_obj,
 					 p->bo_list->gws_obj,
-- 
2.43.0