[PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Thomas Zimmermann]

Revert the use of drm_gem_object.dma_buf back to .import_attach->dmabuf
in the affected places. Also revert any fixes on top. Separates references
to imported and exported DMA bufs within a GEM object; as before.

Using the dma_buf as the one authoritative field for the DMA buf turns
out to be fragile. The GEM object's dma_buf pointer can be NULL if
userspace releases the GEM handle too early. Sima mentioned that the fix
in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for
framebuffers") is conceptionally broken. Linus still notices boot-up
hangs that might be related.

Reverting the whole thing is the only sensible action here.

Tested on virtio; and amdgpu, simpledrm plus udl for dma-buf sharing.

Thomas Zimmermann (9):
  Revert "drm/framebuffer: Acquire internal references on GEM handles"
  Revert "drm/gem: Acquire references on GEM handles for framebuffers"
  Revert "drm/virtio: Use dma_buf from GEM object instance"
  Revert "drm/vmwgfx: Use dma_buf from GEM object instance"
  Revert "drm/etnaviv: Use dma_buf from GEM object instance"
  Revert "drm/prime: Use dma_buf from GEM object instance"
  Revert "drm/gem-framebuffer: Use dma_buf from GEM object instance"
  Revert "drm/gem-shmem: Use dma_buf from GEM object instance"
  Revert "drm/gem-dma: Use dma_buf from GEM object instance"

 drivers/gpu/drm/drm_framebuffer.c            | 31 +---------
 drivers/gpu/drm/drm_gem.c                    | 64 +++-----------------
 drivers/gpu/drm/drm_gem_dma_helper.c         |  2 +-
 drivers/gpu/drm/drm_gem_framebuffer_helper.c |  8 ++-
 drivers/gpu/drm/drm_gem_shmem_helper.c       |  4 +-
 drivers/gpu/drm/drm_internal.h               |  2 -
 drivers/gpu/drm/drm_prime.c                  |  8 ++-
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c  |  4 +-
 drivers/gpu/drm/virtio/virtgpu_prime.c       |  5 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_gem.c          |  6 +-
 include/drm/drm_framebuffer.h                |  7 ---
 11 files changed, 35 insertions(+), 106 deletions(-)

-- 
2.50.0

[PATCH 1/9] Revert "drm/framebuffer: Acquire internal references on GEM handles"

[Thomas Zimmermann]

This reverts commit f6bfc9afc7510cb5e6fbe0a17c507917b0120280.

Not needed as we are going to revert commit 5307dce878d4 ("drm/gem:
Acquire references on GEM handles for framebuffers").

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_framebuffer.c            | 31 ++--------------
 drivers/gpu/drm/drm_gem.c                    | 38 ++++++++------------
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 +++++----
 drivers/gpu/drm/drm_internal.h               |  2 +-
 include/drm/drm_framebuffer.h                |  7 ----
 5 files changed, 26 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 63a70f285cce..b781601946db 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -862,23 +862,11 @@ EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_framebuffer_free);
 int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
 			 const struct drm_framebuffer_funcs *funcs)
 {
-	unsigned int i;
 	int ret;
-	bool exists;
 
 	if (WARN_ON_ONCE(fb->dev != dev || !fb->format))
 		return -EINVAL;
 
-	for (i = 0; i < fb->format->num_planes; i++) {
-		if (drm_WARN_ON_ONCE(dev, fb->internal_flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i)))
-			fb->internal_flags &= ~DRM_FRAMEBUFFER_HAS_HANDLE_REF(i);
-		if (fb->obj[i]) {
-			exists = drm_gem_object_handle_get_if_exists_unlocked(fb->obj[i]);
-			if (exists)
-				fb->internal_flags |= DRM_FRAMEBUFFER_HAS_HANDLE_REF(i);
-		}
-	}
-
 	INIT_LIST_HEAD(&fb->filp_head);
 
 	fb->funcs = funcs;
@@ -887,7 +875,7 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
 	ret = __drm_mode_object_add(dev, &fb->base, DRM_MODE_OBJECT_FB,
 				    false, drm_framebuffer_free);
 	if (ret)
-		goto err;
+		goto out;
 
 	mutex_lock(&dev->mode_config.fb_lock);
 	dev->mode_config.num_fb++;
@@ -895,16 +883,7 @@ int drm_framebuffer_init(struct drm_device *dev, struct drm_framebuffer *fb,
 	mutex_unlock(&dev->mode_config.fb_lock);
 
 	drm_mode_object_register(dev, &fb->base);
-
-	return 0;
-
-err:
-	for (i = 0; i < fb->format->num_planes; i++) {
-		if (fb->internal_flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i)) {
-			drm_gem_object_handle_put_unlocked(fb->obj[i]);
-			fb->internal_flags &= ~DRM_FRAMEBUFFER_HAS_HANDLE_REF(i);
-		}
-	}
+out:
 	return ret;
 }
 EXPORT_SYMBOL(drm_framebuffer_init);
@@ -981,12 +960,6 @@ EXPORT_SYMBOL(drm_framebuffer_unregister_private);
 void drm_framebuffer_cleanup(struct drm_framebuffer *fb)
 {
 	struct drm_device *dev = fb->dev;
-	unsigned int i;
-
-	for (i = 0; i < fb->format->num_planes; i++) {
-		if (fb->internal_flags & DRM_FRAMEBUFFER_HAS_HANDLE_REF(i))
-			drm_gem_object_handle_put_unlocked(fb->obj[i]);
-	}
 
 	mutex_lock(&dev->mode_config.fb_lock);
 	list_del(&fb->head);
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 6a44351e58b7..3a99e4a5d303 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -224,34 +224,23 @@ static void drm_gem_object_handle_get(struct drm_gem_object *obj)
 }
 
 /**
- * drm_gem_object_handle_get_if_exists_unlocked - acquire reference on user-space handle, if any
+ * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles
  * @obj: GEM object
  *
- * Acquires a reference on the GEM buffer object's handle. Required to keep
- * the GEM object alive. Call drm_gem_object_handle_put_if_exists_unlocked()
- * to release the reference. Does nothing if the buffer object has no handle.
- *
- * Returns:
- * True if a handle exists, or false otherwise
+ * Acquires a reference on the GEM buffer object's handle. Required
+ * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked()
+ * to release the reference.
  */
-bool drm_gem_object_handle_get_if_exists_unlocked(struct drm_gem_object *obj)
+void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj)
 {
 	struct drm_device *dev = obj->dev;
 
 	guard(mutex)(&dev->object_name_lock);
 
-	/*
-	 * First ref taken during GEM object creation, if any. Some
-	 * drivers set up internal framebuffers with GEM objects that
-	 * do not have a GEM handle. Hence, this counter can be zero.
-	 */
-	if (!obj->handle_count)
-		return false;
-
+	drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */
 	drm_gem_object_handle_get(obj);
-
-	return true;
 }
+EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked);
 
 /**
  * drm_gem_object_handle_free - release resources bound to userspace handles
@@ -284,7 +273,7 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
 }
 
 /**
- * drm_gem_object_handle_put_unlocked - releases reference on user-space handle
+ * drm_gem_object_handle_put_unlocked - releases reference on user-space handles
  * @obj: GEM object
  *
  * Releases a reference on the GEM buffer object's handle. Possibly releases
@@ -295,14 +284,14 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
 	struct drm_device *dev = obj->dev;
 	bool final = false;
 
-	if (drm_WARN_ON(dev, READ_ONCE(obj->handle_count) == 0))
+	if (WARN_ON(READ_ONCE(obj->handle_count) == 0))
 		return;
 
 	/*
-	 * Must bump handle count first as this may be the last
-	 * ref, in which case the object would disappear before
-	 * we checked for a name.
-	 */
+	* Must bump handle count first as this may be the last
+	* ref, in which case the object would disappear before we
+	* checked for a name
+	*/
 
 	mutex_lock(&dev->object_name_lock);
 	if (--obj->handle_count == 0) {
@@ -315,6 +304,7 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
 	if (final)
 		drm_gem_object_put(obj);
 }
+EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked);
 
 /*
  * Called at device or object close to release the file's
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 618ce725cd75..c60d0044d036 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -100,7 +100,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb)
 	unsigned int i;
 
 	for (i = 0; i < fb->format->num_planes; i++)
-		drm_gem_object_put(fb->obj[i]);
+		drm_gem_object_handle_put_unlocked(fb->obj[i]);
 
 	drm_framebuffer_cleanup(fb);
 	kfree(fb);
@@ -183,8 +183,10 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
 		if (!objs[i]) {
 			drm_dbg_kms(dev, "Failed to lookup GEM object\n");
 			ret = -ENOENT;
-			goto err_gem_object_put;
+			goto err_gem_object_handle_put_unlocked;
 		}
+		drm_gem_object_handle_get_unlocked(objs[i]);
+		drm_gem_object_put(objs[i]);
 
 		min_size = (height - 1) * mode_cmd->pitches[i]
 			 + drm_format_info_min_pitch(info, i, width)
@@ -194,22 +196,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
 			drm_dbg_kms(dev,
 				    "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n",
 				    objs[i]->size, min_size, i);
-			drm_gem_object_put(objs[i]);
+			drm_gem_object_handle_put_unlocked(objs[i]);
 			ret = -EINVAL;
-			goto err_gem_object_put;
+			goto err_gem_object_handle_put_unlocked;
 		}
 	}
 
 	ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
 	if (ret)
-		goto err_gem_object_put;
+		goto err_gem_object_handle_put_unlocked;
 
 	return 0;
 
-err_gem_object_put:
+err_gem_object_handle_put_unlocked:
 	while (i > 0) {
 		--i;
-		drm_gem_object_put(objs[i]);
+		drm_gem_object_handle_put_unlocked(objs[i]);
 	}
 	return ret;
 }
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index e79c3c623c9a..f921cc73f8b8 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -161,7 +161,7 @@ void drm_sysfs_lease_event(struct drm_device *dev);
 
 /* drm_gem.c */
 int drm_gem_init(struct drm_device *dev);
-bool drm_gem_object_handle_get_if_exists_unlocked(struct drm_gem_object *obj);
+void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj);
 void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj);
 int drm_gem_handle_create_tail(struct drm_file *file_priv,
 			       struct drm_gem_object *obj,
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index 38b24fc8978d..668077009fce 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -23,7 +23,6 @@
 #ifndef __DRM_FRAMEBUFFER_H__
 #define __DRM_FRAMEBUFFER_H__
 
-#include <linux/bits.h>
 #include <linux/ctype.h>
 #include <linux/list.h>
 #include <linux/sched.h>
@@ -101,8 +100,6 @@ struct drm_framebuffer_funcs {
 		     unsigned num_clips);
 };
 
-#define DRM_FRAMEBUFFER_HAS_HANDLE_REF(_i)	BIT(0u + (_i))
-
 /**
  * struct drm_framebuffer - frame buffer object
  *
@@ -191,10 +188,6 @@ struct drm_framebuffer {
 	 * DRM_MODE_FB_MODIFIERS.
 	 */
 	int flags;
-	/**
-	 * @internal_flags: Framebuffer flags like DRM_FRAMEBUFFER_HAS_HANDLE_REF.
-	 */
-	unsigned int internal_flags;
 	/**
 	 * @filp_head: Placed on &drm_file.fbs, protected by &drm_file.fbs_lock.
 	 */
-- 
2.50.0

[PATCH 2/9] Revert "drm/gem: Acquire references on GEM handles for framebuffers"

[Thomas Zimmermann]

This reverts commit 5307dce878d4126e1b375587318955bd019c3741.

We're going to revert the dma-buf handle back to separating dma_buf
and import_attach->dmabuf in struct drm_gem_object. Hence revert this
fix for it.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem.c                    | 44 ++------------------
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 ++++---
 drivers/gpu/drm/drm_internal.h               |  2 -
 3 files changed, 11 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 3a99e4a5d303..db44c40e307f 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -213,35 +213,6 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
 }
 EXPORT_SYMBOL(drm_gem_private_object_fini);
 
-static void drm_gem_object_handle_get(struct drm_gem_object *obj)
-{
-	struct drm_device *dev = obj->dev;
-
-	drm_WARN_ON(dev, !mutex_is_locked(&dev->object_name_lock));
-
-	if (obj->handle_count++ == 0)
-		drm_gem_object_get(obj);
-}
-
-/**
- * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles
- * @obj: GEM object
- *
- * Acquires a reference on the GEM buffer object's handle. Required
- * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked()
- * to release the reference.
- */
-void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj)
-{
-	struct drm_device *dev = obj->dev;
-
-	guard(mutex)(&dev->object_name_lock);
-
-	drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */
-	drm_gem_object_handle_get(obj);
-}
-EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked);
-
 /**
  * drm_gem_object_handle_free - release resources bound to userspace handles
  * @obj: GEM object to clean up.
@@ -272,14 +243,8 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
 	}
 }
 
-/**
- * drm_gem_object_handle_put_unlocked - releases reference on user-space handles
- * @obj: GEM object
- *
- * Releases a reference on the GEM buffer object's handle. Possibly releases
- * the GEM buffer object and associated dma-buf objects.
- */
-void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
+static void
+drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
 {
 	struct drm_device *dev = obj->dev;
 	bool final = false;
@@ -304,7 +269,6 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
 	if (final)
 		drm_gem_object_put(obj);
 }
-EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked);
 
 /*
  * Called at device or object close to release the file's
@@ -429,8 +393,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 	int ret;
 
 	WARN_ON(!mutex_is_locked(&dev->object_name_lock));
-
-	drm_gem_object_handle_get(obj);
+	if (obj->handle_count++ == 0)
+		drm_gem_object_get(obj);
 
 	/*
 	 * Get the user-visible handle using idr.  Preload and perform
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index c60d0044d036..618ce725cd75 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -100,7 +100,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb)
 	unsigned int i;
 
 	for (i = 0; i < fb->format->num_planes; i++)
-		drm_gem_object_handle_put_unlocked(fb->obj[i]);
+		drm_gem_object_put(fb->obj[i]);
 
 	drm_framebuffer_cleanup(fb);
 	kfree(fb);
@@ -183,10 +183,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
 		if (!objs[i]) {
 			drm_dbg_kms(dev, "Failed to lookup GEM object\n");
 			ret = -ENOENT;
-			goto err_gem_object_handle_put_unlocked;
+			goto err_gem_object_put;
 		}
-		drm_gem_object_handle_get_unlocked(objs[i]);
-		drm_gem_object_put(objs[i]);
 
 		min_size = (height - 1) * mode_cmd->pitches[i]
 			 + drm_format_info_min_pitch(info, i, width)
@@ -196,22 +194,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
 			drm_dbg_kms(dev,
 				    "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n",
 				    objs[i]->size, min_size, i);
-			drm_gem_object_handle_put_unlocked(objs[i]);
+			drm_gem_object_put(objs[i]);
 			ret = -EINVAL;
-			goto err_gem_object_handle_put_unlocked;
+			goto err_gem_object_put;
 		}
 	}
 
 	ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
 	if (ret)
-		goto err_gem_object_handle_put_unlocked;
+		goto err_gem_object_put;
 
 	return 0;
 
-err_gem_object_handle_put_unlocked:
+err_gem_object_put:
 	while (i > 0) {
 		--i;
-		drm_gem_object_handle_put_unlocked(objs[i]);
+		drm_gem_object_put(objs[i]);
 	}
 	return ret;
 }
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index f921cc73f8b8..9078504e789c 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -161,8 +161,6 @@ void drm_sysfs_lease_event(struct drm_device *dev);
 
 /* drm_gem.c */
 int drm_gem_init(struct drm_device *dev);
-void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj);
-void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj);
 int drm_gem_handle_create_tail(struct drm_file *file_priv,
 			       struct drm_gem_object *obj,
 			       u32 *handlep);
-- 
2.50.0

Re: [PATCH 2/9] Revert "drm/gem: Acquire references on GEM handles for framebuffers"

[Simona Vetter]

On Fri, Jul 11, 2025 at 11:35:17AM +0200, Thomas Zimmermann wrote:
> This reverts commit 5307dce878d4126e1b375587318955bd019c3741.
> 
> We're going to revert the dma-buf handle back to separating dma_buf
> and import_attach->dmabuf in struct drm_gem_object. Hence revert this
> fix for it.

I think we should add my reasons from the private thread here why I think
this is conceptually wrong:

handle_count is an uapi reference, and should have nothing to do with the
lifetime and consistency of the underlying gem_bo. And for imported bo the
link to the dma-buf really should be invariant, and hence
drm_gem_object_get/put() enough. The fact that this patch seems to have
helped at least in some cases indicates that our assumption that we can
replace gem_bo->import_attach.dmabuf with gem_bo->dma_buf was wrong,
because pretty obviously the latter can become NULL while the gem_bo is
still alive. Which means this was conceptually wrong and at best helped
hide a race condition somewhere.

This means that unlike the claim in the reverted commit that 1a148af06000
("drm/gem-shmem: Use dma_buf from GEM object instance") started triggering
an existing condition the much more likely explanation is that it
introduced the regression itself. And hence we need to revert this entire
chain of commits.

I'll also add all the Fixes: lines as needed when merging these to
drm-fixes, since some of the patches reverted in this series have landed
in 6.15 already.

I plan to merge them all to drm-fixes once intel-gfx-ci has approved it
all.

Thanks, Sima

> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/drm_gem.c                    | 44 ++------------------
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 ++++---
>  drivers/gpu/drm/drm_internal.h               |  2 -
>  3 files changed, 11 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 3a99e4a5d303..db44c40e307f 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -213,35 +213,6 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
>  }
>  EXPORT_SYMBOL(drm_gem_private_object_fini);
>  
> -static void drm_gem_object_handle_get(struct drm_gem_object *obj)
> -{
> -	struct drm_device *dev = obj->dev;
> -
> -	drm_WARN_ON(dev, !mutex_is_locked(&dev->object_name_lock));
> -
> -	if (obj->handle_count++ == 0)
> -		drm_gem_object_get(obj);
> -}
> -
> -/**
> - * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles
> - * @obj: GEM object
> - *
> - * Acquires a reference on the GEM buffer object's handle. Required
> - * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked()
> - * to release the reference.
> - */
> -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj)
> -{
> -	struct drm_device *dev = obj->dev;
> -
> -	guard(mutex)(&dev->object_name_lock);
> -
> -	drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */
> -	drm_gem_object_handle_get(obj);
> -}
> -EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked);
> -
>  /**
>   * drm_gem_object_handle_free - release resources bound to userspace handles
>   * @obj: GEM object to clean up.
> @@ -272,14 +243,8 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
>  	}
>  }
>  
> -/**
> - * drm_gem_object_handle_put_unlocked - releases reference on user-space handles
> - * @obj: GEM object
> - *
> - * Releases a reference on the GEM buffer object's handle. Possibly releases
> - * the GEM buffer object and associated dma-buf objects.
> - */
> -void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
> +static void
> +drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
>  {
>  	struct drm_device *dev = obj->dev;
>  	bool final = false;
> @@ -304,7 +269,6 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
>  	if (final)
>  		drm_gem_object_put(obj);
>  }
> -EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked);
>  
>  /*
>   * Called at device or object close to release the file's
> @@ -429,8 +393,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>  	int ret;
>  
>  	WARN_ON(!mutex_is_locked(&dev->object_name_lock));
> -
> -	drm_gem_object_handle_get(obj);
> +	if (obj->handle_count++ == 0)
> +		drm_gem_object_get(obj);
>  
>  	/*
>  	 * Get the user-visible handle using idr.  Preload and perform
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index c60d0044d036..618ce725cd75 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -100,7 +100,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb)
>  	unsigned int i;
>  
>  	for (i = 0; i < fb->format->num_planes; i++)
> -		drm_gem_object_handle_put_unlocked(fb->obj[i]);
> +		drm_gem_object_put(fb->obj[i]);
>  
>  	drm_framebuffer_cleanup(fb);
>  	kfree(fb);
> @@ -183,10 +183,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>  		if (!objs[i]) {
>  			drm_dbg_kms(dev, "Failed to lookup GEM object\n");
>  			ret = -ENOENT;
> -			goto err_gem_object_handle_put_unlocked;
> +			goto err_gem_object_put;
>  		}
> -		drm_gem_object_handle_get_unlocked(objs[i]);
> -		drm_gem_object_put(objs[i]);
>  
>  		min_size = (height - 1) * mode_cmd->pitches[i]
>  			 + drm_format_info_min_pitch(info, i, width)
> @@ -196,22 +194,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>  			drm_dbg_kms(dev,
>  				    "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n",
>  				    objs[i]->size, min_size, i);
> -			drm_gem_object_handle_put_unlocked(objs[i]);
> +			drm_gem_object_put(objs[i]);
>  			ret = -EINVAL;
> -			goto err_gem_object_handle_put_unlocked;
> +			goto err_gem_object_put;
>  		}
>  	}
>  
>  	ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
>  	if (ret)
> -		goto err_gem_object_handle_put_unlocked;
> +		goto err_gem_object_put;
>  
>  	return 0;
>  
> -err_gem_object_handle_put_unlocked:
> +err_gem_object_put:
>  	while (i > 0) {
>  		--i;
> -		drm_gem_object_handle_put_unlocked(objs[i]);
> +		drm_gem_object_put(objs[i]);
>  	}
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index f921cc73f8b8..9078504e789c 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -161,8 +161,6 @@ void drm_sysfs_lease_event(struct drm_device *dev);
>  
>  /* drm_gem.c */
>  int drm_gem_init(struct drm_device *dev);
> -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj);
> -void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj);
>  int drm_gem_handle_create_tail(struct drm_file *file_priv,
>  			       struct drm_gem_object *obj,
>  			       u32 *handlep);
> -- 
> 2.50.0
> 

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 2/9] Revert "drm/gem: Acquire references on GEM handles for framebuffers"

[Christian König]

On 11.07.25 12:08, Simona Vetter wrote:
> On Fri, Jul 11, 2025 at 11:35:17AM +0200, Thomas Zimmermann wrote:
>> This reverts commit 5307dce878d4126e1b375587318955bd019c3741.
>>
>> We're going to revert the dma-buf handle back to separating dma_buf
>> and import_attach->dmabuf in struct drm_gem_object. Hence revert this
>> fix for it.
> 
> I think we should add my reasons from the private thread here why I think
> this is conceptually wrong:
> 
> handle_count is an uapi reference, and should have nothing to do with the
> lifetime and consistency of the underlying gem_bo.

The problem is that we tied the lifetime of the DMA-buf reference to the handle count and I think that is not 100% clean.

The reason why that was done is to break the circle dependency GEM obj -> DMA-buf -> GEM obj, but what potentially should actually happen is that we distinct between a structure reference and an use count.

E.g. similar to what is done with mm_struct and mmgrab()/mmdrop() vs mmget()/mmput().

> And for imported bo the
> link to the dma-buf really should be invariant, and hence
> drm_gem_object_get/put() enough. The fact that this patch seems to have
> helped at least in some cases indicates that our assumption that we can
> replace gem_bo->import_attach.dmabuf with gem_bo->dma_buf was wrong,
> because pretty obviously the latter can become NULL while the gem_bo is
> still alive. Which means this was conceptually wrong and at best helped
> hide a race condition somewhere.
> 
> This means that unlike the claim in the reverted commit that 1a148af06000
> ("drm/gem-shmem: Use dma_buf from GEM object instance") started triggering
> an existing condition the much more likely explanation is that it
> introduced the regression itself. And hence we need to revert this entire
> chain of commits.

The existing condition is still a problem I think. We ran into issues with that multiple times already.

Just imagine the following scenario:
1. GEM obj is exported, DMA-buf file descriptor created
2. GEM obj is used in a FB.
3. GEM obj is closed, handle_count goes from 1->0, DMA-buf reference is dropped, but file descriptor remains open, obj->dma_buf set to NULL
4. Userspace calls DRM_IOCTL_MODE_GETFB2, handle count goes 0->1 again, but obj->dma_buf is still NULL!
5. GEM obj is exported again, second DMA-buf is created.

The first time I stumbled over that it took me a week to figure out why we can have two DMA-bufs for the same GEM obj.  Especially you can trigger the "WARN_ON(obj->dma_buf != dma_buf);" in drm_gem_prime_fd_to_handle() with this.

For my particular use case it was just a broken unit test, but it allows userspace to mess up the kernel objects quite a bit and that is really not good.

Regards,
Christian.

> 
> I'll also add all the Fixes: lines as needed when merging these to
> drm-fixes, since some of the patches reverted in this series have landed
> in 6.15 already.
> 
> I plan to merge them all to drm-fixes once intel-gfx-ci has approved it
> all.
> 
> Thanks, Sima
> 
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> ---
>>  drivers/gpu/drm/drm_gem.c                    | 44 ++------------------
>>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 ++++---
>>  drivers/gpu/drm/drm_internal.h               |  2 -
>>  3 files changed, 11 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> index 3a99e4a5d303..db44c40e307f 100644
>> --- a/drivers/gpu/drm/drm_gem.c
>> +++ b/drivers/gpu/drm/drm_gem.c
>> @@ -213,35 +213,6 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
>>  }
>>  EXPORT_SYMBOL(drm_gem_private_object_fini);
>>  
>> -static void drm_gem_object_handle_get(struct drm_gem_object *obj)
>> -{
>> -	struct drm_device *dev = obj->dev;
>> -
>> -	drm_WARN_ON(dev, !mutex_is_locked(&dev->object_name_lock));
>> -
>> -	if (obj->handle_count++ == 0)
>> -		drm_gem_object_get(obj);
>> -}
>> -
>> -/**
>> - * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles
>> - * @obj: GEM object
>> - *
>> - * Acquires a reference on the GEM buffer object's handle. Required
>> - * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked()
>> - * to release the reference.
>> - */
>> -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj)
>> -{
>> -	struct drm_device *dev = obj->dev;
>> -
>> -	guard(mutex)(&dev->object_name_lock);
>> -
>> -	drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */
>> -	drm_gem_object_handle_get(obj);
>> -}
>> -EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked);
>> -
>>  /**
>>   * drm_gem_object_handle_free - release resources bound to userspace handles
>>   * @obj: GEM object to clean up.
>> @@ -272,14 +243,8 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
>>  	}
>>  }
>>  
>> -/**
>> - * drm_gem_object_handle_put_unlocked - releases reference on user-space handles
>> - * @obj: GEM object
>> - *
>> - * Releases a reference on the GEM buffer object's handle. Possibly releases
>> - * the GEM buffer object and associated dma-buf objects.
>> - */
>> -void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
>> +static void
>> +drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
>>  {
>>  	struct drm_device *dev = obj->dev;
>>  	bool final = false;
>> @@ -304,7 +269,6 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
>>  	if (final)
>>  		drm_gem_object_put(obj);
>>  }
>> -EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked);
>>  
>>  /*
>>   * Called at device or object close to release the file's
>> @@ -429,8 +393,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>>  	int ret;
>>  
>>  	WARN_ON(!mutex_is_locked(&dev->object_name_lock));
>> -
>> -	drm_gem_object_handle_get(obj);
>> +	if (obj->handle_count++ == 0)
>> +		drm_gem_object_get(obj);
>>  
>>  	/*
>>  	 * Get the user-visible handle using idr.  Preload and perform
>> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> index c60d0044d036..618ce725cd75 100644
>> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> @@ -100,7 +100,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb)
>>  	unsigned int i;
>>  
>>  	for (i = 0; i < fb->format->num_planes; i++)
>> -		drm_gem_object_handle_put_unlocked(fb->obj[i]);
>> +		drm_gem_object_put(fb->obj[i]);
>>  
>>  	drm_framebuffer_cleanup(fb);
>>  	kfree(fb);
>> @@ -183,10 +183,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>>  		if (!objs[i]) {
>>  			drm_dbg_kms(dev, "Failed to lookup GEM object\n");
>>  			ret = -ENOENT;
>> -			goto err_gem_object_handle_put_unlocked;
>> +			goto err_gem_object_put;
>>  		}
>> -		drm_gem_object_handle_get_unlocked(objs[i]);
>> -		drm_gem_object_put(objs[i]);
>>  
>>  		min_size = (height - 1) * mode_cmd->pitches[i]
>>  			 + drm_format_info_min_pitch(info, i, width)
>> @@ -196,22 +194,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>>  			drm_dbg_kms(dev,
>>  				    "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n",
>>  				    objs[i]->size, min_size, i);
>> -			drm_gem_object_handle_put_unlocked(objs[i]);
>> +			drm_gem_object_put(objs[i]);
>>  			ret = -EINVAL;
>> -			goto err_gem_object_handle_put_unlocked;
>> +			goto err_gem_object_put;
>>  		}
>>  	}
>>  
>>  	ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
>>  	if (ret)
>> -		goto err_gem_object_handle_put_unlocked;
>> +		goto err_gem_object_put;
>>  
>>  	return 0;
>>  
>> -err_gem_object_handle_put_unlocked:
>> +err_gem_object_put:
>>  	while (i > 0) {
>>  		--i;
>> -		drm_gem_object_handle_put_unlocked(objs[i]);
>> +		drm_gem_object_put(objs[i]);
>>  	}
>>  	return ret;
>>  }
>> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
>> index f921cc73f8b8..9078504e789c 100644
>> --- a/drivers/gpu/drm/drm_internal.h
>> +++ b/drivers/gpu/drm/drm_internal.h
>> @@ -161,8 +161,6 @@ void drm_sysfs_lease_event(struct drm_device *dev);
>>  
>>  /* drm_gem.c */
>>  int drm_gem_init(struct drm_device *dev);
>> -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj);
>> -void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj);
>>  int drm_gem_handle_create_tail(struct drm_file *file_priv,
>>  			       struct drm_gem_object *obj,
>>  			       u32 *handlep);
>> -- 
>> 2.50.0
>>
> 

Re: [PATCH 2/9] Revert "drm/gem: Acquire references on GEM handles for framebuffers"

[Simona Vetter]

On Fri, Jul 11, 2025 at 01:00:03PM +0200, Christian König wrote:
> On 11.07.25 12:08, Simona Vetter wrote:
> > On Fri, Jul 11, 2025 at 11:35:17AM +0200, Thomas Zimmermann wrote:
> >> This reverts commit 5307dce878d4126e1b375587318955bd019c3741.
> >>
> >> We're going to revert the dma-buf handle back to separating dma_buf
> >> and import_attach->dmabuf in struct drm_gem_object. Hence revert this
> >> fix for it.
> > 
> > I think we should add my reasons from the private thread here why I think
> > this is conceptually wrong:
> > 
> > handle_count is an uapi reference, and should have nothing to do with the
> > lifetime and consistency of the underlying gem_bo.
> 
> The problem is that we tied the lifetime of the DMA-buf reference to the
> handle count and I think that is not 100% clean.
> 
> The reason why that was done is to break the circle dependency GEM obj
> -> DMA-buf -> GEM obj, but what potentially should actually happen is
> that we distinct between a structure reference and an use count.
> 
> E.g. similar to what is done with mm_struct and mmgrab()/mmdrop() vs
> mmget()/mmput().

Yeah, I think I'm following. The issue I see here is that I think we're
free-wheeling, dont' have enough testcases, and break existing stuff way
too much. So back to square one, start over, probably with a lot of
kerneldoc patches first and more igt and kunit tests to hit all these
issues we've (re-)discovered.

> > And for imported bo the
> > link to the dma-buf really should be invariant, and hence
> > drm_gem_object_get/put() enough. The fact that this patch seems to have
> > helped at least in some cases indicates that our assumption that we can
> > replace gem_bo->import_attach.dmabuf with gem_bo->dma_buf was wrong,
> > because pretty obviously the latter can become NULL while the gem_bo is
> > still alive. Which means this was conceptually wrong and at best helped
> > hide a race condition somewhere.
> > 
> > This means that unlike the claim in the reverted commit that 1a148af06000
> > ("drm/gem-shmem: Use dma_buf from GEM object instance") started triggering
> > an existing condition the much more likely explanation is that it
> > introduced the regression itself. And hence we need to revert this entire
> > chain of commits.
> 
> The existing condition is still a problem I think. We ran into issues with that multiple times already.
> 
> Just imagine the following scenario:
> 1. GEM obj is exported, DMA-buf file descriptor created
> 2. GEM obj is used in a FB.
> 3. GEM obj is closed, handle_count goes from 1->0, DMA-buf reference is dropped, but file descriptor remains open, obj->dma_buf set to NULL
> 4. Userspace calls DRM_IOCTL_MODE_GETFB2, handle count goes 0->1 again, but obj->dma_buf is still NULL!
> 5. GEM obj is exported again, second DMA-buf is created.
> 
> The first time I stumbled over that it took me a week to figure out why
> we can have two DMA-bufs for the same GEM obj.  Especially you can
> trigger the "WARN_ON(obj->dma_buf != dma_buf);" in
> drm_gem_prime_fd_to_handle() with this.
> 
> For my particular use case it was just a broken unit test, but it allows
> userspace to mess up the kernel objects quite a bit and that is really
> not good.

Yeah that's not good, but I think something we should sort out with adding
testcases first and figuring out fixes in -next, not in late -rc kernels.
I think the minimal fix for this corner case would be to add a flag to
gem_bo that they've had GETFB/2 called on them, and in that case disable
that WARN_ON and just quietly bail out instead. Userspace gets to keep the
pieces.

I'm not sure whether making the entire lifetime stuff even more
complicated by elevating handle_count to a more general usage count is the
right approach.

Cheers, Sima

> 
> Regards,
> Christian.
> 
> > 
> > I'll also add all the Fixes: lines as needed when merging these to
> > drm-fixes, since some of the patches reverted in this series have landed
> > in 6.15 already.
> > 
> > I plan to merge them all to drm-fixes once intel-gfx-ci has approved it
> > all.
> > 
> > Thanks, Sima
> > 
> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> >> ---
> >>  drivers/gpu/drm/drm_gem.c                    | 44 ++------------------
> >>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 16 ++++---
> >>  drivers/gpu/drm/drm_internal.h               |  2 -
> >>  3 files changed, 11 insertions(+), 51 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> >> index 3a99e4a5d303..db44c40e307f 100644
> >> --- a/drivers/gpu/drm/drm_gem.c
> >> +++ b/drivers/gpu/drm/drm_gem.c
> >> @@ -213,35 +213,6 @@ void drm_gem_private_object_fini(struct drm_gem_object *obj)
> >>  }
> >>  EXPORT_SYMBOL(drm_gem_private_object_fini);
> >>  
> >> -static void drm_gem_object_handle_get(struct drm_gem_object *obj)
> >> -{
> >> -	struct drm_device *dev = obj->dev;
> >> -
> >> -	drm_WARN_ON(dev, !mutex_is_locked(&dev->object_name_lock));
> >> -
> >> -	if (obj->handle_count++ == 0)
> >> -		drm_gem_object_get(obj);
> >> -}
> >> -
> >> -/**
> >> - * drm_gem_object_handle_get_unlocked - acquire reference on user-space handles
> >> - * @obj: GEM object
> >> - *
> >> - * Acquires a reference on the GEM buffer object's handle. Required
> >> - * to keep the GEM object alive. Call drm_gem_object_handle_put_unlocked()
> >> - * to release the reference.
> >> - */
> >> -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj)
> >> -{
> >> -	struct drm_device *dev = obj->dev;
> >> -
> >> -	guard(mutex)(&dev->object_name_lock);
> >> -
> >> -	drm_WARN_ON(dev, !obj->handle_count); /* first ref taken in create-tail helper */
> >> -	drm_gem_object_handle_get(obj);
> >> -}
> >> -EXPORT_SYMBOL(drm_gem_object_handle_get_unlocked);
> >> -
> >>  /**
> >>   * drm_gem_object_handle_free - release resources bound to userspace handles
> >>   * @obj: GEM object to clean up.
> >> @@ -272,14 +243,8 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj)
> >>  	}
> >>  }
> >>  
> >> -/**
> >> - * drm_gem_object_handle_put_unlocked - releases reference on user-space handles
> >> - * @obj: GEM object
> >> - *
> >> - * Releases a reference on the GEM buffer object's handle. Possibly releases
> >> - * the GEM buffer object and associated dma-buf objects.
> >> - */
> >> -void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
> >> +static void
> >> +drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
> >>  {
> >>  	struct drm_device *dev = obj->dev;
> >>  	bool final = false;
> >> @@ -304,7 +269,6 @@ void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj)
> >>  	if (final)
> >>  		drm_gem_object_put(obj);
> >>  }
> >> -EXPORT_SYMBOL(drm_gem_object_handle_put_unlocked);
> >>  
> >>  /*
> >>   * Called at device or object close to release the file's
> >> @@ -429,8 +393,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> >>  	int ret;
> >>  
> >>  	WARN_ON(!mutex_is_locked(&dev->object_name_lock));
> >> -
> >> -	drm_gem_object_handle_get(obj);
> >> +	if (obj->handle_count++ == 0)
> >> +		drm_gem_object_get(obj);
> >>  
> >>  	/*
> >>  	 * Get the user-visible handle using idr.  Preload and perform
> >> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> >> index c60d0044d036..618ce725cd75 100644
> >> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> >> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> >> @@ -100,7 +100,7 @@ void drm_gem_fb_destroy(struct drm_framebuffer *fb)
> >>  	unsigned int i;
> >>  
> >>  	for (i = 0; i < fb->format->num_planes; i++)
> >> -		drm_gem_object_handle_put_unlocked(fb->obj[i]);
> >> +		drm_gem_object_put(fb->obj[i]);
> >>  
> >>  	drm_framebuffer_cleanup(fb);
> >>  	kfree(fb);
> >> @@ -183,10 +183,8 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
> >>  		if (!objs[i]) {
> >>  			drm_dbg_kms(dev, "Failed to lookup GEM object\n");
> >>  			ret = -ENOENT;
> >> -			goto err_gem_object_handle_put_unlocked;
> >> +			goto err_gem_object_put;
> >>  		}
> >> -		drm_gem_object_handle_get_unlocked(objs[i]);
> >> -		drm_gem_object_put(objs[i]);
> >>  
> >>  		min_size = (height - 1) * mode_cmd->pitches[i]
> >>  			 + drm_format_info_min_pitch(info, i, width)
> >> @@ -196,22 +194,22 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
> >>  			drm_dbg_kms(dev,
> >>  				    "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n",
> >>  				    objs[i]->size, min_size, i);
> >> -			drm_gem_object_handle_put_unlocked(objs[i]);
> >> +			drm_gem_object_put(objs[i]);
> >>  			ret = -EINVAL;
> >> -			goto err_gem_object_handle_put_unlocked;
> >> +			goto err_gem_object_put;
> >>  		}
> >>  	}
> >>  
> >>  	ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
> >>  	if (ret)
> >> -		goto err_gem_object_handle_put_unlocked;
> >> +		goto err_gem_object_put;
> >>  
> >>  	return 0;
> >>  
> >> -err_gem_object_handle_put_unlocked:
> >> +err_gem_object_put:
> >>  	while (i > 0) {
> >>  		--i;
> >> -		drm_gem_object_handle_put_unlocked(objs[i]);
> >> +		drm_gem_object_put(objs[i]);
> >>  	}
> >>  	return ret;
> >>  }
> >> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> >> index f921cc73f8b8..9078504e789c 100644
> >> --- a/drivers/gpu/drm/drm_internal.h
> >> +++ b/drivers/gpu/drm/drm_internal.h
> >> @@ -161,8 +161,6 @@ void drm_sysfs_lease_event(struct drm_device *dev);
> >>  
> >>  /* drm_gem.c */
> >>  int drm_gem_init(struct drm_device *dev);
> >> -void drm_gem_object_handle_get_unlocked(struct drm_gem_object *obj);
> >> -void drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj);
> >>  int drm_gem_handle_create_tail(struct drm_file *file_priv,
> >>  			       struct drm_gem_object *obj,
> >>  			       u32 *handlep);
> >> -- 
> >> 2.50.0
> >>
> > 
> 

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 3/9] Revert "drm/virtio: Use dma_buf from GEM object instance"

[Thomas Zimmermann]

This reverts commit 415cb45895f43015515473fbc40563ca5eec9a7c.

Reverting because the fix-up commits are suspected to cause regressions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/virtio/virtgpu_prime.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
index 722cde5e2d86..97aaee26cb02 100644
--- a/drivers/gpu/drm/virtio/virtgpu_prime.c
+++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
@@ -204,15 +204,16 @@ static void virtgpu_dma_buf_free_obj(struct drm_gem_object *obj)
 {
 	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
 	struct virtio_gpu_device *vgdev = obj->dev->dev_private;
+	struct dma_buf_attachment *attach = obj->import_attach;
 
 	if (drm_gem_is_imported(obj)) {
-		struct dma_buf *dmabuf = bo->dma_buf;
+		struct dma_buf *dmabuf = attach->dmabuf;
 
 		dma_resv_lock(dmabuf->resv, NULL);
 		virtgpu_dma_buf_unmap(bo);
 		dma_resv_unlock(dmabuf->resv);
 
-		dma_buf_detach(dmabuf, obj->import_attach);
+		dma_buf_detach(dmabuf, attach);
 		dma_buf_put(dmabuf);
 	}
 
-- 
2.50.0

Re: [PATCH 3/9] Revert "drm/virtio: Use dma_buf from GEM object instance"

[Dmitry Osipenko]

On 7/11/25 12:35, Thomas Zimmermann wrote:
> This reverts commit 415cb45895f43015515473fbc40563ca5eec9a7c.
> 
> Reverting because the fix-up commits are suspected to cause regressions.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Are the fixes and stable tags missing in the patches? AFAICT, the intent
is to fix stable 6.15 kernel regression.

> ---
>  drivers/gpu/drm/virtio/virtgpu_prime.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
> index 722cde5e2d86..97aaee26cb02 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> @@ -204,15 +204,16 @@ static void virtgpu_dma_buf_free_obj(struct drm_gem_object *obj)
>  {
>  	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
>  	struct virtio_gpu_device *vgdev = obj->dev->dev_private;
> +	struct dma_buf_attachment *attach = obj->import_attach;
>  
>  	if (drm_gem_is_imported(obj)) {
> -		struct dma_buf *dmabuf = bo->dma_buf;
> +		struct dma_buf *dmabuf = attach->dmabuf;

Nit: I'd write this as `dmabuf = obj->import_attach->dmabuf` for more
consistency with drm_gem_is_imported()

>  		dma_resv_lock(dmabuf->resv, NULL);
>  		virtgpu_dma_buf_unmap(bo);
>  		dma_resv_unlock(dmabuf->resv);
>  
> -		dma_buf_detach(dmabuf, obj->import_attach);
> +		dma_buf_detach(dmabuf, attach);
>  		dma_buf_put(dmabuf);
>  	}
>  

Acked-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry

Re: [PATCH 3/9] Revert "drm/virtio: Use dma_buf from GEM object instance"

[Simona Vetter]

On Fri, Jul 11, 2025 at 02:29:04PM +0300, Dmitry Osipenko wrote:
> On 7/11/25 12:35, Thomas Zimmermann wrote:
> > This reverts commit 415cb45895f43015515473fbc40563ca5eec9a7c.
> > 
> > Reverting because the fix-up commits are suspected to cause regressions.
> > 
> > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Are the fixes and stable tags missing in the patches? AFAICT, the intent
> is to fix stable 6.15 kernel regression.

Yes, I'll add them when merging to drm-fixes.

> > ---
> >  drivers/gpu/drm/virtio/virtgpu_prime.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
> > index 722cde5e2d86..97aaee26cb02 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
> > @@ -204,15 +204,16 @@ static void virtgpu_dma_buf_free_obj(struct drm_gem_object *obj)
> >  {
> >  	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
> >  	struct virtio_gpu_device *vgdev = obj->dev->dev_private;
> > +	struct dma_buf_attachment *attach = obj->import_attach;
> >  
> >  	if (drm_gem_is_imported(obj)) {
> > -		struct dma_buf *dmabuf = bo->dma_buf;
> > +		struct dma_buf *dmabuf = attach->dmabuf;
> 
> Nit: I'd write this as `dmabuf = obj->import_attach->dmabuf` for more
> consistency with drm_gem_is_imported()

Reverts shouldn't have any additional changes, even style nits imo, so
I'll leave it as-is.

> 
> >  		dma_resv_lock(dmabuf->resv, NULL);
> >  		virtgpu_dma_buf_unmap(bo);
> >  		dma_resv_unlock(dmabuf->resv);
> >  
> > -		dma_buf_detach(dmabuf, obj->import_attach);
> > +		dma_buf_detach(dmabuf, attach);
> >  		dma_buf_put(dmabuf);
> >  	}
> >  
> 
> Acked-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Thanks, Sima
-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 3/9] Revert "drm/virtio: Use dma_buf from GEM object instance"

[Dmitry Osipenko]

On 7/11/25 14:31, Simona Vetter wrote:
> On Fri, Jul 11, 2025 at 02:29:04PM +0300, Dmitry Osipenko wrote:
>> On 7/11/25 12:35, Thomas Zimmermann wrote:
>>> This reverts commit 415cb45895f43015515473fbc40563ca5eec9a7c.
>>>
>>> Reverting because the fix-up commits are suspected to cause regressions.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
>> Are the fixes and stable tags missing in the patches? AFAICT, the intent
>> is to fix stable 6.15 kernel regression.
> 
> Yes, I'll add them when merging to drm-fixes.

On a second look, I don't see the reverted commits in 6.15, so only
6.16-rc should be affected.

>>> ---
>>>  drivers/gpu/drm/virtio/virtgpu_prime.c | 5 +++--
>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
>>> index 722cde5e2d86..97aaee26cb02 100644
>>> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
>>> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
>>> @@ -204,15 +204,16 @@ static void virtgpu_dma_buf_free_obj(struct drm_gem_object *obj)
>>>  {
>>>  	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
>>>  	struct virtio_gpu_device *vgdev = obj->dev->dev_private;
>>> +	struct dma_buf_attachment *attach = obj->import_attach;
>>>  
>>>  	if (drm_gem_is_imported(obj)) {
>>> -		struct dma_buf *dmabuf = bo->dma_buf;
>>> +		struct dma_buf *dmabuf = attach->dmabuf;
>>
>> Nit: I'd write this as `dmabuf = obj->import_attach->dmabuf` for more
>> consistency with drm_gem_is_imported()
> 
> Reverts shouldn't have any additional changes, even style nits imo, so
> I'll leave it as-is.

AFAICS, this patch is not a direct/clean revert, it reverts two commits
with manual edit. There is 44b6535d8ace on top of the reverted
415cb45895f4 and final result isn't same as a revert of 415cb45895f4.

-- 
Best regards,
Dmitry

Re: [PATCH 3/9] Revert "drm/virtio: Use dma_buf from GEM object instance"

[Thomas Zimmermann]

Hi

Am 11.07.25 um 13:49 schrieb Dmitry Osipenko:
> On 7/11/25 14:31, Simona Vetter wrote:
>> On Fri, Jul 11, 2025 at 02:29:04PM +0300, Dmitry Osipenko wrote:
>>> On 7/11/25 12:35, Thomas Zimmermann wrote:
>>>> This reverts commit 415cb45895f43015515473fbc40563ca5eec9a7c.
>>>>
>>>> Reverting because the fix-up commits are suspected to cause regressions.
>>>>
>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Are the fixes and stable tags missing in the patches? AFAICT, the intent
>>> is to fix stable 6.15 kernel regression.
>> Yes, I'll add them when merging to drm-fixes.
> On a second look, I don't see the reverted commits in 6.15, so only
> 6.16-rc should be affected.
>
>>>> ---
>>>>   drivers/gpu/drm/virtio/virtgpu_prime.c | 5 +++--
>>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/drm/virtio/virtgpu_prime.c
>>>> index 722cde5e2d86..97aaee26cb02 100644
>>>> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
>>>> @@ -204,15 +204,16 @@ static void virtgpu_dma_buf_free_obj(struct drm_gem_object *obj)
>>>>   {
>>>>   	struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
>>>>   	struct virtio_gpu_device *vgdev = obj->dev->dev_private;
>>>> +	struct dma_buf_attachment *attach = obj->import_attach;
>>>>   
>>>>   	if (drm_gem_is_imported(obj)) {
>>>> -		struct dma_buf *dmabuf = bo->dma_buf;
>>>> +		struct dma_buf *dmabuf = attach->dmabuf;
>>> Nit: I'd write this as `dmabuf = obj->import_attach->dmabuf` for more
>>> consistency with drm_gem_is_imported()
>> Reverts shouldn't have any additional changes, even style nits imo, so
>> I'll leave it as-is.
> AFAICS, this patch is not a direct/clean revert, it reverts two commits
> with manual edit. There is 44b6535d8ace on top of the reverted
> 415cb45895f4 and final result isn't same as a revert of 415cb45895f4.

It's exactly the revert mentioned in the commit message. There was a 
conflict at

-		struct dma_buf *dmabuf = bo->dma_buf;
+		struct dma_buf *dmabuf = attach->dmabuf;


which I resolved.

Best regards
Thomas



>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH 3/9] Revert "drm/virtio: Use dma_buf from GEM object instance"

[Dmitry Osipenko]

On 7/11/25 15:01, Thomas Zimmermann wrote:
> Hi
> 
> Am 11.07.25 um 13:49 schrieb Dmitry Osipenko:
>> On 7/11/25 14:31, Simona Vetter wrote:
>>> On Fri, Jul 11, 2025 at 02:29:04PM +0300, Dmitry Osipenko wrote:
>>>> On 7/11/25 12:35, Thomas Zimmermann wrote:
>>>>> This reverts commit 415cb45895f43015515473fbc40563ca5eec9a7c.
>>>>>
>>>>> Reverting because the fix-up commits are suspected to cause
>>>>> regressions.
>>>>>
>>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>>> Are the fixes and stable tags missing in the patches? AFAICT, the
>>>> intent
>>>> is to fix stable 6.15 kernel regression.
>>> Yes, I'll add them when merging to drm-fixes.
>> On a second look, I don't see the reverted commits in 6.15, so only
>> 6.16-rc should be affected.
>>
>>>>> ---
>>>>>   drivers/gpu/drm/virtio/virtgpu_prime.c | 5 +++--
>>>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_prime.c b/drivers/gpu/
>>>>> drm/virtio/virtgpu_prime.c
>>>>> index 722cde5e2d86..97aaee26cb02 100644
>>>>> --- a/drivers/gpu/drm/virtio/virtgpu_prime.c
>>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_prime.c
>>>>> @@ -204,15 +204,16 @@ static void virtgpu_dma_buf_free_obj(struct
>>>>> drm_gem_object *obj)
>>>>>   {
>>>>>       struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(obj);
>>>>>       struct virtio_gpu_device *vgdev = obj->dev->dev_private;
>>>>> +    struct dma_buf_attachment *attach = obj->import_attach;
>>>>>         if (drm_gem_is_imported(obj)) {
>>>>> -        struct dma_buf *dmabuf = bo->dma_buf;
>>>>> +        struct dma_buf *dmabuf = attach->dmabuf;
>>>> Nit: I'd write this as `dmabuf = obj->import_attach->dmabuf` for more
>>>> consistency with drm_gem_is_imported()
>>> Reverts shouldn't have any additional changes, even style nits imo, so
>>> I'll leave it as-is.
>> AFAICS, this patch is not a direct/clean revert, it reverts two commits
>> with manual edit. There is 44b6535d8ace on top of the reverted
>> 415cb45895f4 and final result isn't same as a revert of 415cb45895f4.
> 
> It's exactly the revert mentioned in the commit message. There was a
> conflict at
> 
> -        struct dma_buf *dmabuf = bo->dma_buf;
> +        struct dma_buf *dmabuf = attach->dmabuf;
> 
> 
> which I resolved.

Now I see that both commits are reverted properly by this patch, thanks.
Having two reverted commits without resolved conflicts would make it
clearer. Either way is okay to me as long as final result is correct.

-- 
Best regards,
Dmitry

[PATCH 4/9] Revert "drm/vmwgfx: Use dma_buf from GEM object instance"

[Thomas Zimmermann]

This reverts commit aec8a40228acb385d60feec59b54573d307e60f3.

Reverting because the fix-up commits are suspected to cause regressions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_gem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
index 7057d852951b..eedf1fe60be7 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_gem.c
@@ -85,10 +85,10 @@ static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
 	int ret;
 
 	if (drm_gem_is_imported(obj)) {
-		ret = dma_buf_vmap(obj->dma_buf, map);
+		ret = dma_buf_vmap(obj->import_attach->dmabuf, map);
 		if (!ret) {
 			if (drm_WARN_ON(obj->dev, map->is_iomem)) {
-				dma_buf_vunmap(obj->dma_buf, map);
+				dma_buf_vunmap(obj->import_attach->dmabuf, map);
 				return -EIO;
 			}
 		}
@@ -102,7 +102,7 @@ static int vmw_gem_vmap(struct drm_gem_object *obj, struct iosys_map *map)
 static void vmw_gem_vunmap(struct drm_gem_object *obj, struct iosys_map *map)
 {
 	if (drm_gem_is_imported(obj))
-		dma_buf_vunmap(obj->dma_buf, map);
+		dma_buf_vunmap(obj->import_attach->dmabuf, map);
 	else
 		drm_gem_ttm_vunmap(obj, map);
 }
-- 
2.50.0

[PATCH 5/9] Revert "drm/etnaviv: Use dma_buf from GEM object instance"

[Thomas Zimmermann]

This reverts commit e91eb3ae415472b28211d7fed07fa283845b311e.

Reverting because the fix-up commits are suspected to cause regressions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
index 917ad527c961..40a50c60dfff 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c
@@ -65,7 +65,7 @@ static void etnaviv_gem_prime_release(struct etnaviv_gem_object *etnaviv_obj)
 	struct iosys_map map = IOSYS_MAP_INIT_VADDR(etnaviv_obj->vaddr);
 
 	if (etnaviv_obj->vaddr)
-		dma_buf_vunmap_unlocked(etnaviv_obj->base.dma_buf, &map);
+		dma_buf_vunmap_unlocked(etnaviv_obj->base.import_attach->dmabuf, &map);
 
 	/* Don't drop the pages for imported dmabuf, as they are not
 	 * ours, just free the array we allocated:
@@ -82,7 +82,7 @@ static void *etnaviv_gem_prime_vmap_impl(struct etnaviv_gem_object *etnaviv_obj)
 
 	lockdep_assert_held(&etnaviv_obj->lock);
 
-	ret = dma_buf_vmap(etnaviv_obj->base.dma_buf, &map);
+	ret = dma_buf_vmap(etnaviv_obj->base.import_attach->dmabuf, &map);
 	if (ret)
 		return NULL;
 	return map.vaddr;
-- 
2.50.0

[PATCH 6/9] Revert "drm/prime: Use dma_buf from GEM object instance"

[Thomas Zimmermann]

This reverts commit f83a9b8c7fd0557b0c50784bfdc1bbe9140c9bf8.

Reverting because the fix-up commits are suspected to cause regressions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_prime.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index b703f83874e1..a23fc712a8b7 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -453,7 +453,13 @@ struct dma_buf *drm_gem_prime_handle_to_dmabuf(struct drm_device *dev,
 	}
 
 	mutex_lock(&dev->object_name_lock);
-	/* re-export the original imported/exported object */
+	/* re-export the original imported object */
+	if (obj->import_attach) {
+		dmabuf = obj->import_attach->dmabuf;
+		get_dma_buf(dmabuf);
+		goto out_have_obj;
+	}
+
 	if (obj->dma_buf) {
 		get_dma_buf(obj->dma_buf);
 		dmabuf = obj->dma_buf;
-- 
2.50.0

[PATCH 7/9] Revert "drm/gem-framebuffer: Use dma_buf from GEM object instance"

[Thomas Zimmermann]

This reverts commit cce16fcd7446dcff7480cd9d2b6417075ed81065.

Reverting because the fix-up commits are suspected to cause regressions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 618ce725cd75..fefb2a0f6b40 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -420,6 +420,7 @@ EXPORT_SYMBOL(drm_gem_fb_vunmap);
 static void __drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir,
 					unsigned int num_planes)
 {
+	struct dma_buf_attachment *import_attach;
 	struct drm_gem_object *obj;
 	int ret;
 
@@ -428,9 +429,10 @@ static void __drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum dma_dat
 		obj = drm_gem_fb_get_obj(fb, num_planes);
 		if (!obj)
 			continue;
+		import_attach = obj->import_attach;
 		if (!drm_gem_is_imported(obj))
 			continue;
-		ret = dma_buf_end_cpu_access(obj->dma_buf, dir);
+		ret = dma_buf_end_cpu_access(import_attach->dmabuf, dir);
 		if (ret)
 			drm_err(fb->dev, "dma_buf_end_cpu_access(%u, %d) failed: %d\n",
 				ret, num_planes, dir);
@@ -453,6 +455,7 @@ static void __drm_gem_fb_end_cpu_access(struct drm_framebuffer *fb, enum dma_dat
  */
 int drm_gem_fb_begin_cpu_access(struct drm_framebuffer *fb, enum dma_data_direction dir)
 {
+	struct dma_buf_attachment *import_attach;
 	struct drm_gem_object *obj;
 	unsigned int i;
 	int ret;
@@ -463,9 +466,10 @@ int drm_gem_fb_begin_cpu_access(struct drm_framebuffer *fb, enum dma_data_direct
 			ret = -EINVAL;
 			goto err___drm_gem_fb_end_cpu_access;
 		}
+		import_attach = obj->import_attach;
 		if (!drm_gem_is_imported(obj))
 			continue;
-		ret = dma_buf_begin_cpu_access(obj->dma_buf, dir);
+		ret = dma_buf_begin_cpu_access(import_attach->dmabuf, dir);
 		if (ret)
 			goto err___drm_gem_fb_end_cpu_access;
 	}
-- 
2.50.0

[PATCH 8/9] Revert "drm/gem-shmem: Use dma_buf from GEM object instance"

[Thomas Zimmermann]

This reverts commit 1a148af06000e545e714fe3210af3d77ff903c11.

Reverting because the fix-up commits are suspected to cause regressions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 8ac0b1fa5287..5d1349c34afd 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -351,7 +351,7 @@ int drm_gem_shmem_vmap_locked(struct drm_gem_shmem_object *shmem,
 	dma_resv_assert_held(obj->resv);
 
 	if (drm_gem_is_imported(obj)) {
-		ret = dma_buf_vmap(obj->dma_buf, map);
+		ret = dma_buf_vmap(obj->import_attach->dmabuf, map);
 	} else {
 		pgprot_t prot = PAGE_KERNEL;
 
@@ -413,7 +413,7 @@ void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
 	dma_resv_assert_held(obj->resv);
 
 	if (drm_gem_is_imported(obj)) {
-		dma_buf_vunmap(obj->dma_buf, map);
+		dma_buf_vunmap(obj->import_attach->dmabuf, map);
 	} else {
 		dma_resv_assert_held(shmem->base.resv);
 
-- 
2.50.0

[PATCH 9/9] Revert "drm/gem-dma: Use dma_buf from GEM object instance"

[Thomas Zimmermann]

This reverts commit e8afa1557f4f963c9a511bd2c6074a941c308685.

Reverting because the fix-up commits are suspected to cause regressions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_gem_dma_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c
index b7f033d4352a..4f0320df858f 100644
--- a/drivers/gpu/drm/drm_gem_dma_helper.c
+++ b/drivers/gpu/drm/drm_gem_dma_helper.c
@@ -230,7 +230,7 @@ void drm_gem_dma_free(struct drm_gem_dma_object *dma_obj)
 
 	if (drm_gem_is_imported(gem_obj)) {
 		if (dma_obj->vaddr)
-			dma_buf_vunmap_unlocked(gem_obj->dma_buf, &map);
+			dma_buf_vunmap_unlocked(gem_obj->import_attach->dmabuf, &map);
 		drm_prime_gem_destroy(gem_obj, dma_obj->sgt);
 	} else if (dma_obj->vaddr) {
 		if (dma_obj->map_noncoherent)
-- 
2.50.0

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Christian König]

On 11.07.25 11:35, Thomas Zimmermann wrote:
> Revert the use of drm_gem_object.dma_buf back to .import_attach->dmabuf
> in the affected places. Also revert any fixes on top. Separates references
> to imported and exported DMA bufs within a GEM object; as before.
> 
> Using the dma_buf as the one authoritative field for the DMA buf turns
> out to be fragile. The GEM object's dma_buf pointer can be NULL if
> userspace releases the GEM handle too early. Sima mentioned that the fix
> in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for
> framebuffers") is conceptionally broken. Linus still notices boot-up
> hangs that might be related.

Did I missed that response? What exactly is the issue?

> Reverting the whole thing is the only sensible action here.

Feel free to add Acked-by: Christian König <christian.koenig@amd.com> to the entire series.

Regards,
Christian.

> 
> Tested on virtio; and amdgpu, simpledrm plus udl for dma-buf sharing.
> 
> Thomas Zimmermann (9):
>   Revert "drm/framebuffer: Acquire internal references on GEM handles"
>   Revert "drm/gem: Acquire references on GEM handles for framebuffers"
>   Revert "drm/virtio: Use dma_buf from GEM object instance"
>   Revert "drm/vmwgfx: Use dma_buf from GEM object instance"
>   Revert "drm/etnaviv: Use dma_buf from GEM object instance"
>   Revert "drm/prime: Use dma_buf from GEM object instance"
>   Revert "drm/gem-framebuffer: Use dma_buf from GEM object instance"
>   Revert "drm/gem-shmem: Use dma_buf from GEM object instance"
>   Revert "drm/gem-dma: Use dma_buf from GEM object instance"
> 
>  drivers/gpu/drm/drm_framebuffer.c            | 31 +---------
>  drivers/gpu/drm/drm_gem.c                    | 64 +++-----------------
>  drivers/gpu/drm/drm_gem_dma_helper.c         |  2 +-
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c |  8 ++-
>  drivers/gpu/drm/drm_gem_shmem_helper.c       |  4 +-
>  drivers/gpu/drm/drm_internal.h               |  2 -
>  drivers/gpu/drm/drm_prime.c                  |  8 ++-
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c  |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_prime.c       |  5 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c          |  6 +-
>  include/drm/drm_framebuffer.h                |  7 ---
>  11 files changed, 35 insertions(+), 106 deletions(-)
> 

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Simona Vetter]

On Fri, Jul 11, 2025 at 12:32:02PM +0200, Christian König wrote:
> On 11.07.25 11:35, Thomas Zimmermann wrote:
> > Revert the use of drm_gem_object.dma_buf back to .import_attach->dmabuf
> > in the affected places. Also revert any fixes on top. Separates references
> > to imported and exported DMA bufs within a GEM object; as before.
> > 
> > Using the dma_buf as the one authoritative field for the DMA buf turns
> > out to be fragile. The GEM object's dma_buf pointer can be NULL if
> > userspace releases the GEM handle too early. Sima mentioned that the fix
> > in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for
> > framebuffers") is conceptionally broken. Linus still notices boot-up
> > hangs that might be related.
> 
> Did I missed that response? What exactly is the issue?

Sorry, private thread from Linus that he's hit the regression, applied the
fix, which was apparently not enough. Then applied the revert of "drm/gem:
Acquire references on GEM handles for framebuffers", which worked better,
but still resulted in not-before-seen issues somehow. At that point I
figured it's best we backtrack because we seem to have a history of not
really understanding this anymore collectively. Thomas also found yet
another related regression around drm_prime in -next, so this looks way
too messy to me for late -rc.

The regressions left over after the bugfix from Thomas that's in
drm-misc-fixes is some silent hangs at login, without any traces anywhere
what got stuck.

We don't yet have feedback from Linus whether the revert more approach
here helps.

> > Reverting the whole thing is the only sensible action here.
> 
> Feel free to add Acked-by: Christian König <christian.koenig@amd.com> to the entire series.

Thanks, I'll apply it to drm-fixes directly assuming intel-gfx-ci
approves.

Note that I'm not fundamentally opposed to the concepts here, at least the
usage count extensions of handle_count seems not entirely off. But it does
look very questionable to fix the patches that switched from
->import_attach.dmabuf to ->dma_buf, and it's simply too late in the -rc
for more big games.

Cheers, Sima

> 
> Regards,
> Christian.
> 
> > 
> > Tested on virtio; and amdgpu, simpledrm plus udl for dma-buf sharing.
> > 
> > Thomas Zimmermann (9):
> >   Revert "drm/framebuffer: Acquire internal references on GEM handles"
> >   Revert "drm/gem: Acquire references on GEM handles for framebuffers"
> >   Revert "drm/virtio: Use dma_buf from GEM object instance"
> >   Revert "drm/vmwgfx: Use dma_buf from GEM object instance"
> >   Revert "drm/etnaviv: Use dma_buf from GEM object instance"
> >   Revert "drm/prime: Use dma_buf from GEM object instance"
> >   Revert "drm/gem-framebuffer: Use dma_buf from GEM object instance"
> >   Revert "drm/gem-shmem: Use dma_buf from GEM object instance"
> >   Revert "drm/gem-dma: Use dma_buf from GEM object instance"
> > 
> >  drivers/gpu/drm/drm_framebuffer.c            | 31 +---------
> >  drivers/gpu/drm/drm_gem.c                    | 64 +++-----------------
> >  drivers/gpu/drm/drm_gem_dma_helper.c         |  2 +-
> >  drivers/gpu/drm/drm_gem_framebuffer_helper.c |  8 ++-
> >  drivers/gpu/drm/drm_gem_shmem_helper.c       |  4 +-
> >  drivers/gpu/drm/drm_internal.h               |  2 -
> >  drivers/gpu/drm/drm_prime.c                  |  8 ++-
> >  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c  |  4 +-
> >  drivers/gpu/drm/virtio/virtgpu_prime.c       |  5 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c          |  6 +-
> >  include/drm/drm_framebuffer.h                |  7 ---
> >  11 files changed, 35 insertions(+), 106 deletions(-)
> > 
> 

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Linus Torvalds]

On Fri, 11 Jul 2025 at 02:40, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Reverting the whole thing is the only sensible action here.

I'm assuming this is against some current drm tree, not mine, because
it doesn't apply here.

I'll try the smaller set of reverts that Simona suggested for my
testing, and will get back to you on that.

Background for others: current -git ends up having odd hangs when
Xwayland starts for me (not at boot, but when I log in). It seems to
be very timing-dependent, so presumably I'm just unlucky with my
hardware.

The timing-dependence has made my attempts to bisect it very
frustrating and non-conclusive, but reverting 5307dce878d4 ("drm/gem:
Acquire references on GEM handles for framebuffers") made things work
_better_, to the point where I thought it was fixed.

Until the hang came back.

The "hang" is not some kind of kernel hang, it seems to be user-space
getting confused by the changes. Because I can still switch back to a
text VT, and there are no messages in the kernel logs. And the system
"recovers", in that after a 30-second timeout (it feels like a minute,
but logs seem to imply it really is just half a minute), I get

   gnome-session-manager@gnome.service: start operation timed out. Terminating.
   gnome-session-manager@gnome.service: Failed with result 'timeout'.

and it restarts, and things work fine on the second try. Except when
they don't - on one reboot it went through this twice, and

Now, because it's obviously timing-sensitive, it might be something
entirely different going on and I have no guarantees, but it does seem
to be related to that commit 5307dce878d4.

And I only see this on one of my machines. Again, might be timing, but
might also be that it only shows up on certain hardware (in my case:
Radeon RX 580).

           Linus

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Thomas Zimmermann]

Hi

Am 11.07.25 um 17:48 schrieb Linus Torvalds:
> On Fri, 11 Jul 2025 at 02:40, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Reverting the whole thing is the only sensible action here.
> I'm assuming this is against some current drm tree, not mine, because
> it doesn't apply here.

Yes, it's against drm-misc-fixes, which requires an additional revert of 
the extra fix from this week.

>
> I'll try the smaller set of reverts that Simona suggested for my
> testing, and will get back to you on that.

The list of patches I sent earlier are from v6.15-rc5 and should be the 
ones you need. You will get the revert patch set as part of a DRM PR soon.

Best regards
Thomas

>
> Background for others: current -git ends up having odd hangs when
> Xwayland starts for me (not at boot, but when I log in). It seems to
> be very timing-dependent, so presumably I'm just unlucky with my
> hardware.
>
> The timing-dependence has made my attempts to bisect it very
> frustrating and non-conclusive, but reverting 5307dce878d4 ("drm/gem:
> Acquire references on GEM handles for framebuffers") made things work
> _better_, to the point where I thought it was fixed.
>
> Until the hang came back.
>
> The "hang" is not some kind of kernel hang, it seems to be user-space
> getting confused by the changes. Because I can still switch back to a
> text VT, and there are no messages in the kernel logs. And the system
> "recovers", in that after a 30-second timeout (it feels like a minute,
> but logs seem to imply it really is just half a minute), I get
>
>     gnome-session-manager@gnome.service: start operation timed out. Terminating.
>     gnome-session-manager@gnome.service: Failed with result 'timeout'.
>
> and it restarts, and things work fine on the second try. Except when
> they don't - on one reboot it went through this twice, and
>
> Now, because it's obviously timing-sensitive, it might be something
> entirely different going on and I have no guarantees, but it does seem
> to be related to that commit 5307dce878d4.
>
> And I only see this on one of my machines. Again, might be timing, but
> might also be that it only shows up on certain hardware (in my case:
> Radeon RX 580).
>
>             Linus

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Linus Torvalds]

On Fri, 11 Jul 2025 at 08:48, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Background for others: current -git ends up having odd hangs when
> Xwayland starts for me (not at boot, but when I log in). It seems to
> be very timing-dependent, so presumably I'm just unlucky with my
> hardware.

Update for this thread as well: it looks like I'm just being unlucky,
and there are two independent problems. The drm problem *may* be just
the warning issue that has been reported by others.

I'm hoping the login time timeout / hang ends up being due to a known
netlink regression, and it just happened to look like a drm issue
because it exposes itself as a hang at the first graphical login

A netlink regression *might* fit the pattern, in that it might just
cause first login dependency issues and resulting timeouts.

After what has felt like a very smooth release cycle so far, having
what looks like two entirely independent issues strikes me as
unlikely, but hey, unlikely clearly does occasionally happen.

           Linus

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Linus Torvalds]

On Fri, 11 Jul 2025 at 10:35, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I'm hoping the login time timeout / hang ends up being due to a known
> netlink regression, and it just happened to look like a drm issue
> because it exposes itself as a hang at the first graphical login
>
> A netlink regression *might* fit the pattern, in that it might just
> cause first login dependency issues and resulting timeouts.

Well, considering the random timing behavior of this bug, it's hard to
be really sure, but two boots with the alleged "fix" for the netlink
issue made the behavior worse, so it does look like my problems on
this machine were a combination of the drm refcounting issue and the
netlink thing.

I guess I'll have to do a lot more rebooting to be sure, since the
hangs and timeouts have been so random. But the netlink "fixes" did
give me a hang that was very different from the previous ones I've
seen, so I think the drm code is off the hook on this one.

                 Linus

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Simona Vetter]

On Fri, Jul 11, 2025 at 11:37:30AM -0700, Linus Torvalds wrote:
> On Fri, 11 Jul 2025 at 10:35, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > I'm hoping the login time timeout / hang ends up being due to a known
> > netlink regression, and it just happened to look like a drm issue
> > because it exposes itself as a hang at the first graphical login
> >
> > A netlink regression *might* fit the pattern, in that it might just
> > cause first login dependency issues and resulting timeouts.
> 
> Well, considering the random timing behavior of this bug, it's hard to
> be really sure, but two boots with the alleged "fix" for the netlink
> issue made the behavior worse, so it does look like my problems on
> this machine were a combination of the drm refcounting issue and the
> netlink thing.
> 
> I guess I'll have to do a lot more rebooting to be sure, since the
> hangs and timeouts have been so random. But the netlink "fixes" did
> give me a hang that was very different from the previous ones I've
> seen, so I think the drm code is off the hook on this one.

Ok sounds good, I won't include the drm reverts then. I do think the
change from ->import_attach.dmabuf to ->dma_buf is still suspect, and the
handle_count reference change for fb won't cover all the other places this
might blow up (just less likely since the most common multi-gpu use-cases
are sharing render buffers to a display driver). But better we take
another week to really think this through before rushing things.

The handle_count changes do look reasonable to me too, but for an entirely
different bug around bo import/export. And I think we'll want a testcase
for that to make sure, and evaluate options properly.

But now I'll just type the pr summary and then w/e for me.

Cheers, Sima
-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Simona Vetter]

On Fri, Jul 11, 2025 at 11:35:15AM +0200, Thomas Zimmermann wrote:
> Revert the use of drm_gem_object.dma_buf back to .import_attach->dmabuf
> in the affected places. Also revert any fixes on top. Separates references
> to imported and exported DMA bufs within a GEM object; as before.
> 
> Using the dma_buf as the one authoritative field for the DMA buf turns
> out to be fragile. The GEM object's dma_buf pointer can be NULL if
> userspace releases the GEM handle too early. Sima mentioned that the fix
> in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for
> framebuffers") is conceptionally broken. Linus still notices boot-up
> hangs that might be related.
> 
> Reverting the whole thing is the only sensible action here.
> 
> Tested on virtio; and amdgpu, simpledrm plus udl for dma-buf sharing.
> 
> Thomas Zimmermann (9):
>   Revert "drm/framebuffer: Acquire internal references on GEM handles"
>   Revert "drm/gem: Acquire references on GEM handles for framebuffers"

Ok, I think all the below we should still apply for -fixes, because
fundamentally gem_bo->dma_buf is not invariant over the lifetime of the
buffer, while gem_bo->import_attach.dmabuf is. And so we blow up.

For display drivers the handle_count reference mostly papers over the
issues, but even display drivers are allowed to keep internal references
to the underlying gem bo for longer. So there could be a bunch of really
tricky bugs lurking.

For render drivers it's even clearer, they don't have framebuffers as
objects, so there the fb handle_count references does not help.

I'm not opposed to trying to unify these fields for imported dma_buf, but
currently they're just not. Hence all the reverts.

The patches also need Fixes: and as needed, cc: stable added for merging.
With that and the above text as additional justification added:

Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>

Also we'd need to chase down any addiotional conversions that have only
landed in -next meanwhile of course.

₣or the handle_count patches I'm less sure. I don't think they're
justified for fixing the gem_bo->dma_buf NULL pointer issues, but they do
probably help with the GETFB/2 confusion Christian has pointed out.
Personally my preference is:
1. Apply the two reverts.
2. Create an igt testcase for the GETFB confusion
3. Figure out what the right fix for that is, which might or might not be
the handle_count reference of drm_fb.

But with my maintainer hat on I don't mind about the exact path, as long
as we get there somehow. If you decide to do land the reverts, they also
have my:

Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>

Cheers, Sima

>   Revert "drm/virtio: Use dma_buf from GEM object instance"
>   Revert "drm/vmwgfx: Use dma_buf from GEM object instance"
>   Revert "drm/etnaviv: Use dma_buf from GEM object instance"
>   Revert "drm/prime: Use dma_buf from GEM object instance"
>   Revert "drm/gem-framebuffer: Use dma_buf from GEM object instance"
>   Revert "drm/gem-shmem: Use dma_buf from GEM object instance"
>   Revert "drm/gem-dma: Use dma_buf from GEM object instance"
> 
>  drivers/gpu/drm/drm_framebuffer.c            | 31 +---------
>  drivers/gpu/drm/drm_gem.c                    | 64 +++-----------------
>  drivers/gpu/drm/drm_gem_dma_helper.c         |  2 +-
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c |  8 ++-
>  drivers/gpu/drm/drm_gem_shmem_helper.c       |  4 +-
>  drivers/gpu/drm/drm_internal.h               |  2 -
>  drivers/gpu/drm/drm_prime.c                  |  8 ++-
>  drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c  |  4 +-
>  drivers/gpu/drm/virtio/virtgpu_prime.c       |  5 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_gem.c          |  6 +-
>  include/drm/drm_framebuffer.h                |  7 ---
>  11 files changed, 35 insertions(+), 106 deletions(-)
> 
> -- 
> 2.50.0
> 

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Thomas Zimmermann]

Hi

Am 14.07.25 um 14:39 schrieb Simona Vetter:
> On Fri, Jul 11, 2025 at 11:35:15AM +0200, Thomas Zimmermann wrote:
>> Revert the use of drm_gem_object.dma_buf back to .import_attach->dmabuf
>> in the affected places. Also revert any fixes on top. Separates references
>> to imported and exported DMA bufs within a GEM object; as before.
>>
>> Using the dma_buf as the one authoritative field for the DMA buf turns
>> out to be fragile. The GEM object's dma_buf pointer can be NULL if
>> userspace releases the GEM handle too early. Sima mentioned that the fix
>> in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for
>> framebuffers") is conceptionally broken. Linus still notices boot-up
>> hangs that might be related.
>>
>> Reverting the whole thing is the only sensible action here.
>>
>> Tested on virtio; and amdgpu, simpledrm plus udl for dma-buf sharing.
>>
>> Thomas Zimmermann (9):
>>    Revert "drm/framebuffer: Acquire internal references on GEM handles"
>>    Revert "drm/gem: Acquire references on GEM handles for framebuffers"
> Ok, I think all the below we should still apply for -fixes, because
> fundamentally gem_bo->dma_buf is not invariant over the lifetime of the
> buffer, while gem_bo->import_attach.dmabuf is. And so we blow up.
>
> For display drivers the handle_count reference mostly papers over the
> issues, but even display drivers are allowed to keep internal references
> to the underlying gem bo for longer. So there could be a bunch of really
> tricky bugs lurking.
>
> For render drivers it's even clearer, they don't have framebuffers as
> objects, so there the fb handle_count references does not help.
>
> I'm not opposed to trying to unify these fields for imported dma_buf, but
> currently they're just not. Hence all the reverts.

Thanks for the write up.

>
> The patches also need Fixes: and as needed, cc: stable added for merging.
> With that and the above text as additional justification added:
>
> Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
>
> Also we'd need to chase down any addiotional conversions that have only
> landed in -next meanwhile of course.
>
> ₣or the handle_count patches I'm less sure. I don't think they're
> justified for fixing the gem_bo->dma_buf NULL pointer issues, but they do
> probably help with the GETFB/2 confusion Christian has pointed out.
> Personally my preference is:
> 1. Apply the two reverts.
> 2. Create an igt testcase for the GETFB confusion
> 3. Figure out what the right fix for that is, which might or might not be
> the handle_count reference of drm_fb.
>
> But with my maintainer hat on I don't mind about the exact path, as long
> as we get there somehow. If you decide to do land the reverts, they also
> have my:
>
> Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>

Let's first revert all the dma_buf switching in drm-misc and other 
trees. They should
be easy. If we revert the framebuffer-related A changes first, we might 
end up with
these intermediate errors again.

There's no hurry with the framebuffer changes. We can address them after 
upstream
picked up the dma-buf reverts.

Best regards
Thomas

>
> Cheers, Sima
>
>>    Revert "drm/virtio: Use dma_buf from GEM object instance"
>>    Revert "drm/vmwgfx: Use dma_buf from GEM object instance"
>>    Revert "drm/etnaviv: Use dma_buf from GEM object instance"
>>    Revert "drm/prime: Use dma_buf from GEM object instance"
>>    Revert "drm/gem-framebuffer: Use dma_buf from GEM object instance"
>>    Revert "drm/gem-shmem: Use dma_buf from GEM object instance"
>>    Revert "drm/gem-dma: Use dma_buf from GEM object instance"
>>
>>   drivers/gpu/drm/drm_framebuffer.c            | 31 +---------
>>   drivers/gpu/drm/drm_gem.c                    | 64 +++-----------------
>>   drivers/gpu/drm/drm_gem_dma_helper.c         |  2 +-
>>   drivers/gpu/drm/drm_gem_framebuffer_helper.c |  8 ++-
>>   drivers/gpu/drm/drm_gem_shmem_helper.c       |  4 +-
>>   drivers/gpu/drm/drm_internal.h               |  2 -
>>   drivers/gpu/drm/drm_prime.c                  |  8 ++-
>>   drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c  |  4 +-
>>   drivers/gpu/drm/virtio/virtgpu_prime.c       |  5 +-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_gem.c          |  6 +-
>>   include/drm/drm_framebuffer.h                |  7 ---
>>   11 files changed, 35 insertions(+), 106 deletions(-)
>>
>> -- 
>> 2.50.0
>>

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

Re: [PATCH 0/9] drm: Revert general use of struct drm_gem_object.dma_buf

[Simona Vetter]

On Tue, Jul 15, 2025 at 09:41:12AM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 14.07.25 um 14:39 schrieb Simona Vetter:
> > On Fri, Jul 11, 2025 at 11:35:15AM +0200, Thomas Zimmermann wrote:
> > > Revert the use of drm_gem_object.dma_buf back to .import_attach->dmabuf
> > > in the affected places. Also revert any fixes on top. Separates references
> > > to imported and exported DMA bufs within a GEM object; as before.
> > > 
> > > Using the dma_buf as the one authoritative field for the DMA buf turns
> > > out to be fragile. The GEM object's dma_buf pointer can be NULL if
> > > userspace releases the GEM handle too early. Sima mentioned that the fix
> > > in commit 5307dce878d4 ("drm/gem: Acquire references on GEM handles for
> > > framebuffers") is conceptionally broken. Linus still notices boot-up
> > > hangs that might be related.
> > > 
> > > Reverting the whole thing is the only sensible action here.
> > > 
> > > Tested on virtio; and amdgpu, simpledrm plus udl for dma-buf sharing.
> > > 
> > > Thomas Zimmermann (9):
> > >    Revert "drm/framebuffer: Acquire internal references on GEM handles"
> > >    Revert "drm/gem: Acquire references on GEM handles for framebuffers"
> > Ok, I think all the below we should still apply for -fixes, because
> > fundamentally gem_bo->dma_buf is not invariant over the lifetime of the
> > buffer, while gem_bo->import_attach.dmabuf is. And so we blow up.
> > 
> > For display drivers the handle_count reference mostly papers over the
> > issues, but even display drivers are allowed to keep internal references
> > to the underlying gem bo for longer. So there could be a bunch of really
> > tricky bugs lurking.
> > 
> > For render drivers it's even clearer, they don't have framebuffers as
> > objects, so there the fb handle_count references does not help.
> > 
> > I'm not opposed to trying to unify these fields for imported dma_buf, but
> > currently they're just not. Hence all the reverts.
> 
> Thanks for the write up.
> 
> > 
> > The patches also need Fixes: and as needed, cc: stable added for merging.
> > With that and the above text as additional justification added:
> > 
> > Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
> > 
> > Also we'd need to chase down any addiotional conversions that have only
> > landed in -next meanwhile of course.
> > 
> > ₣or the handle_count patches I'm less sure. I don't think they're
> > justified for fixing the gem_bo->dma_buf NULL pointer issues, but they do
> > probably help with the GETFB/2 confusion Christian has pointed out.
> > Personally my preference is:
> > 1. Apply the two reverts.
> > 2. Create an igt testcase for the GETFB confusion
> > 3. Figure out what the right fix for that is, which might or might not be
> > the handle_count reference of drm_fb.
> > 
> > But with my maintainer hat on I don't mind about the exact path, as long
> > as we get there somehow. If you decide to do land the reverts, they also
> > have my:
> > 
> > Reviewed-by: Simona Vetter <simona.vetter@ffwll.ch>
> 
> Let's first revert all the dma_buf switching in drm-misc and other trees.
> They should
> be easy. If we revert the framebuffer-related A changes first, we might end
> up with
> these intermediate errors again.
> 
> There's no hurry with the framebuffer changes. We can address them after
> upstream
> picked up the dma-buf reverts.

Yeah I think that's the most prudent path forward, otherwise we might
accidentally regress linux-next in an avoidable way.
-Sima

> 
> Best regards
> Thomas
> 
> > 
> > Cheers, Sima
> > 
> > >    Revert "drm/virtio: Use dma_buf from GEM object instance"
> > >    Revert "drm/vmwgfx: Use dma_buf from GEM object instance"
> > >    Revert "drm/etnaviv: Use dma_buf from GEM object instance"
> > >    Revert "drm/prime: Use dma_buf from GEM object instance"
> > >    Revert "drm/gem-framebuffer: Use dma_buf from GEM object instance"
> > >    Revert "drm/gem-shmem: Use dma_buf from GEM object instance"
> > >    Revert "drm/gem-dma: Use dma_buf from GEM object instance"
> > > 
> > >   drivers/gpu/drm/drm_framebuffer.c            | 31 +---------
> > >   drivers/gpu/drm/drm_gem.c                    | 64 +++-----------------
> > >   drivers/gpu/drm/drm_gem_dma_helper.c         |  2 +-
> > >   drivers/gpu/drm/drm_gem_framebuffer_helper.c |  8 ++-
> > >   drivers/gpu/drm/drm_gem_shmem_helper.c       |  4 +-
> > >   drivers/gpu/drm/drm_internal.h               |  2 -
> > >   drivers/gpu/drm/drm_prime.c                  |  8 ++-
> > >   drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c  |  4 +-
> > >   drivers/gpu/drm/virtio/virtgpu_prime.c       |  5 +-
> > >   drivers/gpu/drm/vmwgfx/vmwgfx_gem.c          |  6 +-
> > >   include/drm/drm_framebuffer.h                |  7 ---
> > >   11 files changed, 35 insertions(+), 106 deletions(-)
> > > 
> > > -- 
> > > 2.50.0
> > > 
> 
> -- 
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)
> 

-- 
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch