[PATCH 0/2] drm: revert the remaining commits about dma_buf handling

[PATCH 0/2] drm: revert the remaining commits about dma_buf handling

[Melissa Wen]

Hi all,

I just talked with Thomas that these two patches are preventing amdgpu
driver to be unloaded:

`modprobe: FATAL: Module amdgpu is in use.`

and there is no process using the driver.

We agreed that the best approach now is to completely revert the work
done for improving DMA bug handling to avoid any loose ends. With these
reverts we are just back to the old behavior and amdgpu loading and
unloading will return to normal.

Best Regards,

Melissa

Melissa Wen (2):
  Revert "drm/framebuffer: Acquire internal references on GEM handles"
  Revert "drm/gem: Acquire references on GEM handles for framebuffers"

 drivers/gpu/drm/drm_framebuffer.c | 31 +--------------
 drivers/gpu/drm/drm_gem.c         | 64 +++++--------------------------
 drivers/gpu/drm/drm_internal.h    |  2 -
 include/drm/drm_framebuffer.h     |  7 ----
 4 files changed, 11 insertions(+), 93 deletions(-)

-- 
2.47.2

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

[Melissa Wen]

This reverts commit f6bfc9afc7510cb5e6fbe0a17c507917b0120280.

since we are going to revert commit 5307dce878d4 ("drm/gem: Acquire
references on GEM handles for framebuffers"), which it was fixing, this
commit won't be needed anymore.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 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 adbb73f00d68..61a7213f2389 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -863,23 +863,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;
@@ -888,7 +876,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++;
@@ -896,16 +884,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);
@@ -982,12 +961,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 cbeb76b2124f..09f80a84d61a 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 4bc89d33df59..e364fa36ee36 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -101,7 +101,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);
@@ -179,8 +179,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)
@@ -190,22 +192,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, info, 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 5a3bed48ab1f..ec1bf58e5714 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -163,7 +163,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.47.2

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

[Melissa Wen]

This reverts commit 5307dce878d4126e1b375587318955bd019c3741.

We've already reverted all other commits related to dma_bug handling and
there is still something wrong with this approach that does not allow
unloading a driver. By reverting this commit, we'd just go back ot the
old behavior.

Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 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 09f80a84d61a..12efc04fb896 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
@@ -434,8 +398,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 e364fa36ee36..4bc89d33df59 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -101,7 +101,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);
@@ -179,10 +179,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)
@@ -192,22 +190,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, info, 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 ec1bf58e5714..5265eac81077 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -163,8 +163,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.47.2

Re: [PATCH 0/2] drm: revert the remaining commits about dma_buf handling

[Thomas Zimmermann]

Hi

Am 19.09.25 um 17:54 schrieb Melissa Wen:
> Hi all,
>
> I just talked with Thomas that these two patches are preventing amdgpu
> driver to be unloaded:
>
> `modprobe: FATAL: Module amdgpu is in use.`
>
> and there is no process using the driver.
>
> We agreed that the best approach now is to completely revert the work
> done for improving DMA bug handling to avoid any loose ends. With these
> reverts we are just back to the old behavior and amdgpu loading and
> unloading will return to normal.

For both patches

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

If no complains come in, I'll send them out with this week's 
drm-misc-next-fixes.

Best regards
Thomas

>
> Best Regards,
>
> Melissa
>
> Melissa Wen (2):
>    Revert "drm/framebuffer: Acquire internal references on GEM handles"
>    Revert "drm/gem: Acquire references on GEM handles for framebuffers"
>
>   drivers/gpu/drm/drm_framebuffer.c | 31 +--------------
>   drivers/gpu/drm/drm_gem.c         | 64 +++++--------------------------
>   drivers/gpu/drm/drm_internal.h    |  2 -
>   include/drm/drm_framebuffer.h     |  7 ----
>   4 files changed, 11 insertions(+), 93 deletions(-)
>

-- 
--
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 2/2] Revert "drm/gem: Acquire references on GEM handles for framebuffers"

[Christian König]

On 19.09.25 17:54, Melissa Wen wrote:
> This reverts commit 5307dce878d4126e1b375587318955bd019c3741.
> 
> We've already reverted all other commits related to dma_bug handling and
> there is still something wrong with this approach that does not allow
> unloading a driver. By reverting this commit, we'd just go back ot the
> old behavior.

I don't think we want to do this.

Keeping the backing store alive for DMA-bufs while they are used for scanout is actually a really important bug fix.

Regards,
Christian.

> 
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>  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 09f80a84d61a..12efc04fb896 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
> @@ -434,8 +398,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 e364fa36ee36..4bc89d33df59 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -101,7 +101,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);
> @@ -179,10 +179,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)
> @@ -192,22 +190,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, info, 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 ec1bf58e5714..5265eac81077 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -163,8 +163,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);

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

[Thomas Zimmermann]

Hi

Am 22.09.25 um 10:34 schrieb Christian König:
> On 19.09.25 17:54, Melissa Wen wrote:
>> This reverts commit 5307dce878d4126e1b375587318955bd019c3741.
>>
>> We've already reverted all other commits related to dma_bug handling and
>> there is still something wrong with this approach that does not allow
>> unloading a driver. By reverting this commit, we'd just go back ot the
>> old behavior.
> I don't think we want to do this.
>
> Keeping the backing store alive for DMA-bufs while they are used for scanout is actually a really important bug fix.

That bug has rarely seen seen in practice. At least I'm not aware of any 
such report. And it's also just half of the fix IIRC. Not being able to 
unload the module is a regression OTOH. I'd rather go back to the old 
status quo than now having to deal with two problems.

Best regards
Thomas

>
> Regards,
> Christian.
>
>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>> ---
>>   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 09f80a84d61a..12efc04fb896 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
>> @@ -434,8 +398,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 e364fa36ee36..4bc89d33df59 100644
>> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> @@ -101,7 +101,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);
>> @@ -179,10 +179,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)
>> @@ -192,22 +190,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, info, 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 ec1bf58e5714..5265eac81077 100644
>> --- a/drivers/gpu/drm/drm_internal.h
>> +++ b/drivers/gpu/drm/drm_internal.h
>> @@ -163,8 +163,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);

-- 
--
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 2/2] Revert "drm/gem: Acquire references on GEM handles for framebuffers"

[Christian König]

On 22.09.25 10:40, Thomas Zimmermann wrote:
> Hi
> 
> Am 22.09.25 um 10:34 schrieb Christian König:
>> On 19.09.25 17:54, Melissa Wen wrote:
>>> This reverts commit 5307dce878d4126e1b375587318955bd019c3741.
>>>
>>> We've already reverted all other commits related to dma_bug handling and
>>> there is still something wrong with this approach that does not allow
>>> unloading a driver. By reverting this commit, we'd just go back ot the
>>> old behavior.
>> I don't think we want to do this.
>>
>> Keeping the backing store alive for DMA-bufs while they are used for scanout is actually a really important bug fix.
> 
> That bug has rarely seen seen in practice. At least I'm not aware of any such report. And it's also just half of the fix IIRC. Not being able to unload the module is a regression OTOH. I'd rather go back to the old status quo than now having to deal with two problems.

Yeah wait a second, not being able to unload the module is potentially the right thing to do.

So were is that module reference actually coming from? And why do we have it now and didn't had it previously?

Regards,
Christian.

> 
> Best regards
> Thomas
> 
>>
>> Regards,
>> Christian.
>>
>>> Signed-off-by: Melissa Wen <mwen@igalia.com>
>>> ---
>>>   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 09f80a84d61a..12efc04fb896 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
>>> @@ -434,8 +398,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 e364fa36ee36..4bc89d33df59 100644
>>> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>>> @@ -101,7 +101,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);
>>> @@ -179,10 +179,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)
>>> @@ -192,22 +190,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, info, 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 ec1bf58e5714..5265eac81077 100644
>>> --- a/drivers/gpu/drm/drm_internal.h
>>> +++ b/drivers/gpu/drm/drm_internal.h
>>> @@ -163,8 +163,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);
> 

Re: [PATCH 0/2] drm: revert the remaining commits about dma_buf handling

[Thomas Zimmermann]

Hi Melissa

Am 19.09.25 um 17:54 schrieb Melissa Wen:
> Hi all,
>
> I just talked with Thomas that these two patches are preventing amdgpu
> driver to be unloaded:
>
> `modprobe: FATAL: Module amdgpu is in use.`
>
> and there is no process using the driver.

What's the exact STR for this problem? After Christian's comments, I 
tried to reproduce the issue, but it works on my system. I do

- boot up with amdgpu in text mode (multiuser.target)
- login by serial console
- 'rmmod amdgpu'

That last step turns my test system's display off and unloads amdgpu. 
The kernel is a recent drm-tip at v6.17-rc6.

Best regards
Thomas

>
> We agreed that the best approach now is to completely revert the work
> done for improving DMA bug handling to avoid any loose ends. With these
> reverts we are just back to the old behavior and amdgpu loading and
> unloading will return to normal.
>
> Best Regards,
>
> Melissa
>
> Melissa Wen (2):
>    Revert "drm/framebuffer: Acquire internal references on GEM handles"
>    Revert "drm/gem: Acquire references on GEM handles for framebuffers"
>
>   drivers/gpu/drm/drm_framebuffer.c | 31 +--------------
>   drivers/gpu/drm/drm_gem.c         | 64 +++++--------------------------
>   drivers/gpu/drm/drm_internal.h    |  2 -
>   include/drm/drm_framebuffer.h     |  7 ----
>   4 files changed, 11 insertions(+), 93 deletions(-)
>

-- 
--
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/2] drm: revert the remaining commits about dma_buf handling

[Christian König]

On 22.09.25 13:54, Thomas Zimmermann wrote:
> Hi Melissa
> 
> Am 19.09.25 um 17:54 schrieb Melissa Wen:
>> Hi all,
>>
>> I just talked with Thomas that these two patches are preventing amdgpu
>> driver to be unloaded:
>>
>> `modprobe: FATAL: Module amdgpu is in use.`
>>
>> and there is no process using the driver.

Melissa could it be that you have a console loaded?

Regards,
Christian.

> 
> What's the exact STR for this problem? After Christian's comments, I tried to reproduce the issue, but it works on my system. I do
> 
> - boot up with amdgpu in text mode (multiuser.target)
> - login by serial console
> - 'rmmod amdgpu'
> 
> That last step turns my test system's display off and unloads amdgpu. The kernel is a recent drm-tip at v6.17-rc6.
> 
> Best regards
> Thomas
> 
>>
>> We agreed that the best approach now is to completely revert the work
>> done for improving DMA bug handling to avoid any loose ends. With these
>> reverts we are just back to the old behavior and amdgpu loading and
>> unloading will return to normal.
>>
>> Best Regards,
>>
>> Melissa
>>
>> Melissa Wen (2):
>>    Revert "drm/framebuffer: Acquire internal references on GEM handles"
>>    Revert "drm/gem: Acquire references on GEM handles for framebuffers"
>>
>>   drivers/gpu/drm/drm_framebuffer.c | 31 +--------------
>>   drivers/gpu/drm/drm_gem.c         | 64 +++++--------------------------
>>   drivers/gpu/drm/drm_internal.h    |  2 -
>>   include/drm/drm_framebuffer.h     |  7 ----
>>   4 files changed, 11 insertions(+), 93 deletions(-)
>>
> 

Re: [PATCH 0/2] drm: revert the remaining commits about dma_buf handling

[Thomas Zimmermann]

Am 22.09.25 um 13:55 schrieb Christian König:
> On 22.09.25 13:54, Thomas Zimmermann wrote:
>> Hi Melissa
>>
>> Am 19.09.25 um 17:54 schrieb Melissa Wen:
>>> Hi all,
>>>
>>> I just talked with Thomas that these two patches are preventing amdgpu
>>> driver to be unloaded:
>>>
>>> `modprobe: FATAL: Module amdgpu is in use.`
>>>
>>> and there is no process using the driver.
> Melissa could it be that you have a console loaded?

FTR I can also do 'rmmod amdgpu' from amdgpu's fbcon successfully.

>
> Regards,
> Christian.
>
>> What's the exact STR for this problem? After Christian's comments, I tried to reproduce the issue, but it works on my system. I do
>>
>> - boot up with amdgpu in text mode (multiuser.target)
>> - login by serial console
>> - 'rmmod amdgpu'
>>
>> That last step turns my test system's display off and unloads amdgpu. The kernel is a recent drm-tip at v6.17-rc6.
>>
>> Best regards
>> Thomas
>>
>>> We agreed that the best approach now is to completely revert the work
>>> done for improving DMA bug handling to avoid any loose ends. With these
>>> reverts we are just back to the old behavior and amdgpu loading and
>>> unloading will return to normal.
>>>
>>> Best Regards,
>>>
>>> Melissa
>>>
>>> Melissa Wen (2):
>>>     Revert "drm/framebuffer: Acquire internal references on GEM handles"
>>>     Revert "drm/gem: Acquire references on GEM handles for framebuffers"
>>>
>>>    drivers/gpu/drm/drm_framebuffer.c | 31 +--------------
>>>    drivers/gpu/drm/drm_gem.c         | 64 +++++--------------------------
>>>    drivers/gpu/drm/drm_internal.h    |  2 -
>>>    include/drm/drm_framebuffer.h     |  7 ----
>>>    4 files changed, 11 insertions(+), 93 deletions(-)
>>>

-- 
--
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 2/2] Revert "drm/gem: Acquire references on GEM handles for framebuffers"

[Maarten Lankhorst]

Hey,

On 2025-09-19 17:54, Melissa Wen wrote:
> This reverts commit 5307dce878d4126e1b375587318955bd019c3741.
> 
> We've already reverted all other commits related to dma_bug handling and
> there is still something wrong with this approach that does not allow
> unloading a driver. By reverting this commit, we'd just go back ot the
> old behavior.

This is actually important to keep. i915 and xe already do this for similar reasons, so I'm glad it's enforced across all drivers.
Without this you can do this:
- CreateFB
- destroy gem object
- Have random memory being scanout.

I'd rather keep it. If amd no longer unloads, figure out why. I know there are igt tests for module load, do those work?

Best regards,
~Maarten

Re: [PATCH 0/2] drm: revert the remaining commits about dma_buf handling

[Melissa Wen]

On 22/09/2025 08:54, Thomas Zimmermann wrote:
> Hi Melissa
>
> Am 19.09.25 um 17:54 schrieb Melissa Wen:
>> Hi all,
>>
>> I just talked with Thomas that these two patches are preventing amdgpu
>> driver to be unloaded:
>>
>> `modprobe: FATAL: Module amdgpu is in use.`
>>
>> and there is no process using the driver.
>
> What's the exact STR for this problem? After Christian's comments, I 
> tried to reproduce the issue, but it works on my system. I do
>
> - boot up with amdgpu in text mode (multiuser.target)
> - login by serial console
> - 'rmmod amdgpu'
>
> That last step turns my test system's display off and unloads amdgpu. 
> The kernel is a recent drm-tip at v6.17-rc6.

- I booted up in graphical.target;
- Connected via ssh, dropped the graphical interface to text mode 
(multiuser.target), and tried to remove the module with `modprobe -r amdgpu`

The issue happened in the latest 6.16 kernel version from Debian (I 
don't have the machine with me atm to check the exact version), but also 
with mainline kernel from last week.

I bisected kernel from 6.16 and 6.15 and IIRC git bisect pointed to 
`drm/framebuffer: Acquire internal references on GEM handles`, but as it 
triggers another trace and AFAIU fixes `drm/gem: Acquire references on 
GEM handles for framebuffers`, I reverted both pacthes and amdgpu 
loaded/unloaded as expected.

IGT amdgpu-specific tests for loading and unloading are also failing 
because it's not able to unload the module.
I didn't run the generic test.

I'm using an AMD Cezanne laptop with a touch-screen capable display (hp 
envy x360 convertible) with Debian + Gnome.
I looked for process using the modules, but I didn't find any.

BTW, I don't discard that this work uncovered a preexist problem, for 
example.
Since those patches are addressing other issues, as mentioned by 
Maarten, I'll debug the amdgpu driver further and check the points raised.

Best Regards,

Melissa

>
> Best regards
> Thomas
>
>>
>> We agreed that the best approach now is to completely revert the work
>> done for improving DMA bug handling to avoid any loose ends. With these
>> reverts we are just back to the old behavior and amdgpu loading and
>> unloading will return to normal.
>>
>> Best Regards,
>>
>> Melissa
>>
>> Melissa Wen (2):
>>    Revert "drm/framebuffer: Acquire internal references on GEM handles"
>>    Revert "drm/gem: Acquire references on GEM handles for framebuffers"
>>
>>   drivers/gpu/drm/drm_framebuffer.c | 31 +--------------
>>   drivers/gpu/drm/drm_gem.c         | 64 +++++--------------------------
>>   drivers/gpu/drm/drm_internal.h    |  2 -
>>   include/drm/drm_framebuffer.h     |  7 ----
>>   4 files changed, 11 insertions(+), 93 deletions(-)
>>
>

Re: [PATCH 0/2] drm: revert the remaining commits about dma_buf handling

[Christian König]

On 22.09.25 15:42, Melissa Wen wrote:
> On 22/09/2025 08:54, Thomas Zimmermann wrote:
>> Hi Melissa
>>
>> Am 19.09.25 um 17:54 schrieb Melissa Wen:
>>> Hi all,
>>>
>>> I just talked with Thomas that these two patches are preventing amdgpu
>>> driver to be unloaded:
>>>
>>> `modprobe: FATAL: Module amdgpu is in use.`
>>>
>>> and there is no process using the driver.
>>
>> What's the exact STR for this problem? After Christian's comments, I tried to reproduce the issue, but it works on my system. I do
>>
>> - boot up with amdgpu in text mode (multiuser.target)
>> - login by serial console
>> - 'rmmod amdgpu'
>>
>> That last step turns my test system's display off and unloads amdgpu. The kernel is a recent drm-tip at v6.17-rc6.
> 
> - I booted up in graphical.target;
> - Connected via ssh, dropped the graphical interface to text mode (multiuser.target), and tried to remove the module with `modprobe -r amdgpu`
> 
> The issue happened in the latest 6.16 kernel version from Debian (I don't have the machine with me atm to check the exact version), but also with mainline kernel from last week.
> 
> I bisected kernel from 6.16 and 6.15 and IIRC git bisect pointed to `drm/framebuffer: Acquire internal references on GEM handles`, but as it triggers another trace and AFAIU fixes `drm/gem: Acquire references on GEM handles for framebuffers`, I reverted both pacthes and amdgpu loaded/unloaded as expected.
> 
> IGT amdgpu-specific tests for loading and unloading are also failing because it's not able to unload the module.
> I didn't run the generic test.
> 
> I'm using an AMD Cezanne laptop with a touch-screen capable display (hp envy x360 convertible) with Debian + Gnome.
> I looked for process using the modules, but I didn't find any.
> 
> BTW, I don't discard that this work uncovered a preexist problem, for example.
> Since those patches are addressing other issues, as mentioned by Maarten, I'll debug the amdgpu driver further and check the points raised.

Well could it be that the graphics target left a CRTC running and a framebuffer being scanned out?

Regards,
Christian.

> 
> Best Regards,
> 
> Melissa
> 
>>
>> Best regards
>> Thomas
>>
>>>
>>> We agreed that the best approach now is to completely revert the work
>>> done for improving DMA bug handling to avoid any loose ends. With these
>>> reverts we are just back to the old behavior and amdgpu loading and
>>> unloading will return to normal.
>>>
>>> Best Regards,
>>>
>>> Melissa
>>>
>>> Melissa Wen (2):
>>>    Revert "drm/framebuffer: Acquire internal references on GEM handles"
>>>    Revert "drm/gem: Acquire references on GEM handles for framebuffers"
>>>
>>>   drivers/gpu/drm/drm_framebuffer.c | 31 +--------------
>>>   drivers/gpu/drm/drm_gem.c         | 64 +++++--------------------------
>>>   drivers/gpu/drm/drm_internal.h    |  2 -
>>>   include/drm/drm_framebuffer.h     |  7 ----
>>>   4 files changed, 11 insertions(+), 93 deletions(-)
>>>
>>
> 

Re: [PATCH 0/2] drm: revert the remaining commits about dma_buf handling

[Thomas Zimmermann]

Hi

Am 22.09.25 um 15:42 schrieb Melissa Wen:
>
>
> On 22/09/2025 08:54, Thomas Zimmermann wrote:
>> Hi Melissa
>>
>> Am 19.09.25 um 17:54 schrieb Melissa Wen:
>>> Hi all,
>>>
>>> I just talked with Thomas that these two patches are preventing amdgpu
>>> driver to be unloaded:
>>>
>>> `modprobe: FATAL: Module amdgpu is in use.`
>>>
>>> and there is no process using the driver.
>>
>> What's the exact STR for this problem? After Christian's comments, I 
>> tried to reproduce the issue, but it works on my system. I do
>>
>> - boot up with amdgpu in text mode (multiuser.target)
>> - login by serial console
>> - 'rmmod amdgpu'
>>
>> That last step turns my test system's display off and unloads amdgpu. 
>> The kernel is a recent drm-tip at v6.17-rc6.
>
> - I booted up in graphical.target;
> - Connected via ssh, dropped the graphical interface to text mode 
> (multiuser.target), and tried to remove the module with `modprobe -r 
> amdgpu`

I do

- boot into graphical
- ssh into the test system
- via ssh: sudo systemctl isolate multi-user.target
- via ssh: sudo modprobe -r amdgpu

Works as expected.

>
> The issue happened in the latest 6.16 kernel version from Debian (I 
> don't have the machine with me atm to check the exact version), but 
> also with mainline kernel from last week.
>
> I bisected kernel from 6.16 and 6.15 and IIRC git bisect pointed to 
> `drm/framebuffer: Acquire internal references on GEM handles`, but as 
> it triggers another trace and AFAIU fixes `drm/gem: Acquire references 
> on GEM handles for framebuffers`, I reverted both pacthes and amdgpu 
> loaded/unloaded as expected.
>
> IGT amdgpu-specific tests for loading and unloading are also failing 
> because it's not able to unload the module.
> I didn't run the generic test.
>
> I'm using an AMD Cezanne laptop with a touch-screen capable display 
> (hp envy x360 convertible) with Debian + Gnome.
> I looked for process using the modules, but I didn't find any.

After you switched to multi-user, what is in /sys/kernel/debug/dri/<YOUR 
DRM DEV>/
framebuffer ?

There should only be a single fb allocated by fbcon.

>
> BTW, I don't discard that this work uncovered a preexist problem, for 
> example.
> Since those patches are addressing other issues, as mentioned by 
> Maarten, I'll debug the amdgpu driver further and check the points 
> raised.

Thanks

Best regards
Thomas


>
> Best Regards,
>
> Melissa
>
>>
>> Best regards
>> Thomas
>>
>>>
>>> We agreed that the best approach now is to completely revert the work
>>> done for improving DMA bug handling to avoid any loose ends. With these
>>> reverts we are just back to the old behavior and amdgpu loading and
>>> unloading will return to normal.
>>>
>>> Best Regards,
>>>
>>> Melissa
>>>
>>> Melissa Wen (2):
>>>    Revert "drm/framebuffer: Acquire internal references on GEM handles"
>>>    Revert "drm/gem: Acquire references on GEM handles for framebuffers"
>>>
>>>   drivers/gpu/drm/drm_framebuffer.c | 31 +--------------
>>>   drivers/gpu/drm/drm_gem.c         | 64 
>>> +++++--------------------------
>>>   drivers/gpu/drm/drm_internal.h    |  2 -
>>>   include/drm/drm_framebuffer.h     |  7 ----
>>>   4 files changed, 11 insertions(+), 93 deletions(-)
>>>
>>
>

-- 
--
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)