[PATCH 0/2] drm,drm/xe: Relax helper module reference requirement

[PATCH 0/2] drm,drm/xe: Relax helper module reference requirement

[Thomas Hellström]

Helper components that hold a drm device reference also need to hold
a module reference on the (sub)driver that owns the DRM device.

This can be undesirable since it's not possible to rmmod the module
without unbinding the driver first.

Provide means to relax that requirement and then relax it for the
drm/xe driver: Keep a device-count in the xe driver (Patch 2) and add a
function to drm to wait to ensure any device release callbacks are
done executing. (Patch 1).

The remaining requirement is that whatever module calls drm_dev_put()
needs to ensure it stays in memory until that function returns.
That is true also for the unload-protected driver itself
At unload time it needs to ensure that any code inside the module
itself ending up calling drm_dev_put() is waited for before
unloading the module. This change doesn't protect against that.

As an example, this series would allow dropping the module reference
held by drm_pagemap if the modules owning the drm device is
sufficently protected.

Thomas Hellström (2):
  drm: Provide a drm_dev_release_barrier() function to wait for device
    release callbacks
  drm/xe: Don't unload the driver until all drm devices are freed

 drivers/gpu/drm/drm_drv.c      | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_device.c | 19 +++++++++++++++++++
 drivers/gpu/drm/xe/xe_device.h |  2 ++
 drivers/gpu/drm/xe/xe_module.c |  5 ++++-
 include/drm/drm_drv.h          |  1 +
 5 files changed, 51 insertions(+), 1 deletion(-)

-- 
2.53.0

[PATCH 1/2] drm: Provide a drm_dev_release_barrier() function to wait for device release callbacks

[Thomas Hellström]

If helper components, like for example drm_pagemap hold references to
drm devices, it's typically possible for the drm driver module to be
unloaded without that reference being dropped, resulting in execution out
of freed memory. Such components are therefore required to hold a
module refcount on the module that created the drm device, and ensure
that module reference is dropped after all references to the drm
device are dropped.

To relax that, drivers can keep a drm device-count and ensure that the
module isn't unloaded until the drm device-count has dropped to zero
and that the caller that decremented the last device-count has finished
executing driver callbacks.

To help with the latter, add a drm_dev_release_barrier() function.
The function ensures that any caller that has started executing
device release callbacks has also finished executing them.

Use SRCU for the implementation.

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/drm_drv.c | 25 +++++++++++++++++++++++++
 include/drm/drm_drv.h     |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 2915118436ce..9fa72adf173d 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -898,21 +898,46 @@ struct drm_device *drm_dev_alloc(const struct drm_driver *driver,
 }
 EXPORT_SYMBOL(drm_dev_alloc);
 
+DEFINE_STATIC_SRCU(drm_dev_release_srcu);
+
 static void drm_dev_release(struct kref *ref)
 {
 	struct drm_device *dev = container_of(ref, struct drm_device, ref);
+	int idx;
 
 	/* Just in case register/unregister was never called */
 	drm_debugfs_dev_fini(dev);
 
+	idx = srcu_read_lock(&drm_dev_release_srcu);
 	if (dev->driver->release)
 		dev->driver->release(dev);
 
 	drm_managed_release(dev);
+	srcu_read_unlock(&drm_dev_release_srcu, idx);
 
 	kfree(dev->managed.final_kfree);
 }
 
+/**
+ * drm_dev_release_barrier() - Ensure drm device release callbacks are finished
+ *
+ * If a device release method or any of the drm managed release callbacks
+ * have been called for a device, wait until all of them have finished
+ * executing. This function can be used to help determine whether it's safe
+ * to unload a driver module.
+ *
+ * Assume for example the driver maintains a device count which is decremented
+ * using a drmm callback or a device release callback. From a drm device
+ * lifetime POV, it's then safe to unload the driver when that device-count
+ * has reached zero and drm_dev_release_barrier() has been called.
+ */
+void drm_dev_release_barrier(void)
+{
+	synchronize_srcu(&drm_dev_release_srcu);
+}
+EXPORT_SYMBOL(drm_dev_release_barrier);
+
+
 /**
  * drm_dev_get - Take reference of a DRM device
  * @dev: device to take reference of or NULL
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 42fc085f986d..b288a885cf45 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -489,6 +489,7 @@ void drm_dev_exit(int idx);
 void drm_dev_unplug(struct drm_device *dev);
 int drm_dev_wedged_event(struct drm_device *dev, unsigned long method,
 			 struct drm_wedge_task_info *info);
+void drm_dev_release_barrier(void);
 
 /**
  * drm_dev_is_unplugged - is a DRM device unplugged
-- 
2.53.0

[PATCH 2/2] drm/xe: Don't unload the driver until all drm devices are freed

[Thomas Hellström]

Don't unload the driver until all drm devices are freed.
This will help ensure that helper components don't need to
maintain a module reference while holding a drm device reference.

Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
---
 drivers/gpu/drm/xe/xe_device.c | 19 +++++++++++++++++++
 drivers/gpu/drm/xe/xe_device.h |  2 ++
 drivers/gpu/drm/xe/xe_module.c |  5 ++++-
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
index 3462645ca13c..738124a73064 100644
--- a/drivers/gpu/drm/xe/xe_device.c
+++ b/drivers/gpu/drm/xe/xe_device.c
@@ -415,6 +415,8 @@ static struct drm_driver driver = {
 	.patchlevel = DRIVER_PATCHLEVEL,
 };
 
+static atomic_t xe_device_count;
+
 static void xe_device_destroy(struct drm_device *dev, void *dummy)
 {
 	struct xe_device *xe = to_xe_device(dev);
@@ -434,6 +436,9 @@ static void xe_device_destroy(struct drm_device *dev, void *dummy)
 		destroy_workqueue(xe->destroy_wq);
 
 	ttm_device_fini(&xe->ttm);
+
+	if (atomic_dec_and_test(&xe_device_count))
+		wake_up_var(&xe_device_count);
 }
 
 struct xe_device *xe_device_create(struct pci_dev *pdev,
@@ -459,6 +464,7 @@ struct xe_device *xe_device_create(struct pci_dev *pdev,
 		return ERR_PTR(err);
 
 	xe_bo_dev_init(&xe->bo_device);
+	atomic_inc(&xe_device_count);
 	err = drmm_add_action_or_reset(&xe->drm, xe_device_destroy, NULL);
 	if (err)
 		return ERR_PTR(err);
@@ -1397,3 +1403,16 @@ struct xe_vm *xe_device_asid_to_vm(struct xe_device *xe, u32 asid)
 
 	return vm;
 }
+
+/**
+ * xe_device_exit() - Device subsystem exit function.
+ *
+ * Exit function to be called at module unload time.
+ */
+void xe_device_exit(void)
+{
+	/* Wait for all devices to be freed. */
+	wait_var_event(&xe_device_count, !atomic_read(&xe_device_count));
+	/* Wait for any driver release callbacks to complete */
+	drm_dev_release_barrier();
+}
diff --git a/drivers/gpu/drm/xe/xe_device.h b/drivers/gpu/drm/xe/xe_device.h
index 39464650533b..d827a443e3d4 100644
--- a/drivers/gpu/drm/xe/xe_device.h
+++ b/drivers/gpu/drm/xe/xe_device.h
@@ -206,6 +206,8 @@ bool xe_is_xe_file(const struct file *file);
 
 struct xe_vm *xe_device_asid_to_vm(struct xe_device *xe, u32 asid);
 
+void xe_device_exit(void);
+
 /*
  * Occasionally it is seen that the G2H worker starts running after a delay of more than
  * a second even after being queued and activated by the Linux workqueue subsystem. This
diff --git a/drivers/gpu/drm/xe/xe_module.c b/drivers/gpu/drm/xe/xe_module.c
index 4cb578182912..816e02f4bdaf 100644
--- a/drivers/gpu/drm/xe/xe_module.c
+++ b/drivers/gpu/drm/xe/xe_module.c
@@ -11,7 +11,7 @@
 #include <drm/drm_module.h>
 
 #include "xe_defaults.h"
-#include "xe_device_types.h"
+#include "xe_device.h"
 #include "xe_drv.h"
 #include "xe_configfs.h"
 #include "xe_hw_fence.h"
@@ -97,6 +97,9 @@ struct init_funcs {
 };
 
 static const struct init_funcs init_funcs[] = {
+	{
+		.exit = xe_device_exit,
+	},
 	{
 		.init = xe_check_nomodeset,
 	},
-- 
2.53.0

Re: [PATCH 1/2] drm: Provide a drm_dev_release_barrier() function to wait for device release callbacks

[Christian König]

On 3/16/26 17:20, Thomas Hellström wrote:
> If helper components, like for example drm_pagemap hold references to
> drm devices, it's typically possible for the drm driver module to be
> unloaded without that reference being dropped, 

That is an extremely bad idea to begin with, drm_devices should reference the module who issued them.

> resulting in execution out
> of freed memory. Such components are therefore required to hold a
> module refcount on the module that created the drm device, and ensure
> that module reference is dropped after all references to the drm
> device are dropped.
> 
> To relax that, drivers can keep a drm device-count and ensure that the
> module isn't unloaded until the drm device-count has dropped to zero
> and that the caller that decremented the last device-count has finished
> executing driver callbacks.

Stop for a second. The question is why would anybody want to relax that?

That unbinding a driver from a device is separated from module unloading is a design we had in Linux since the very beginning.

Regards,
Christian.

> 
> To help with the latter, add a drm_dev_release_barrier() function.
> The function ensures that any caller that has started executing
> device release callbacks has also finished executing them.
> 
> Use SRCU for the implementation.
> 
> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_drv.c | 25 +++++++++++++++++++++++++
>  include/drm/drm_drv.h     |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 2915118436ce..9fa72adf173d 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -898,21 +898,46 @@ struct drm_device *drm_dev_alloc(const struct drm_driver *driver,
>  }
>  EXPORT_SYMBOL(drm_dev_alloc);
>  
> +DEFINE_STATIC_SRCU(drm_dev_release_srcu);
> +
>  static void drm_dev_release(struct kref *ref)
>  {
>  	struct drm_device *dev = container_of(ref, struct drm_device, ref);
> +	int idx;
>  
>  	/* Just in case register/unregister was never called */
>  	drm_debugfs_dev_fini(dev);
>  
> +	idx = srcu_read_lock(&drm_dev_release_srcu);
>  	if (dev->driver->release)
>  		dev->driver->release(dev);
>  
>  	drm_managed_release(dev);
> +	srcu_read_unlock(&drm_dev_release_srcu, idx);
>  
>  	kfree(dev->managed.final_kfree);
>  }
>  
> +/**
> + * drm_dev_release_barrier() - Ensure drm device release callbacks are finished
> + *
> + * If a device release method or any of the drm managed release callbacks
> + * have been called for a device, wait until all of them have finished
> + * executing. This function can be used to help determine whether it's safe
> + * to unload a driver module.
> + *
> + * Assume for example the driver maintains a device count which is decremented
> + * using a drmm callback or a device release callback. From a drm device
> + * lifetime POV, it's then safe to unload the driver when that device-count
> + * has reached zero and drm_dev_release_barrier() has been called.
> + */
> +void drm_dev_release_barrier(void)
> +{
> +	synchronize_srcu(&drm_dev_release_srcu);
> +}
> +EXPORT_SYMBOL(drm_dev_release_barrier);
> +
> +
>  /**
>   * drm_dev_get - Take reference of a DRM device
>   * @dev: device to take reference of or NULL
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 42fc085f986d..b288a885cf45 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -489,6 +489,7 @@ void drm_dev_exit(int idx);
>  void drm_dev_unplug(struct drm_device *dev);
>  int drm_dev_wedged_event(struct drm_device *dev, unsigned long method,
>  			 struct drm_wedge_task_info *info);
> +void drm_dev_release_barrier(void);
>  
>  /**
>   * drm_dev_is_unplugged - is a DRM device unplugged

Re: [PATCH 1/2] drm: Provide a drm_dev_release_barrier() function to wait for device release callbacks

[Thomas Hellström]

On Mon, 2026-03-16 at 18:42 +0100, Christian König wrote:
> On 3/16/26 17:20, Thomas Hellström wrote:
> > If helper components, like for example drm_pagemap hold references
> > to
> > drm devices, it's typically possible for the drm driver module to
> > be
> > unloaded without that reference being dropped, 
> 
> That is an extremely bad idea to begin with, drm_devices should
> reference the module who issued them.

They typically don't. If that were the case you wouldn't be able to
rmmod a module and have device cleanup happen:

module_exit();

pci_unregister_driver()-><starts device removal>
devm_release()-><drop drm device reference>
drmm_release()
...
<module unloads>

I was under the impression that this was the behaviour of most device
drivers and also drm drivers if display isn't enabled.
(used to work with xe but doesn't anymore for unknown reason).

Module references are typically held by open files, display and dma-
bufs, but not by devices.

Like surely you can rmmod for example a serial device driver and have
it implicitly unbind its devices, but not if a device have a file
opened. 

> 
> > resulting in execution out
> > of freed memory. Such components are therefore required to hold a
> > module refcount on the module that created the drm device, and
> > ensure
> > that module reference is dropped after all references to the drm
> > device are dropped.
> > 
> > To relax that, drivers can keep a drm device-count and ensure that
> > the
> > module isn't unloaded until the drm device-count has dropped to
> > zero
> > and that the caller that decremented the last device-count has
> > finished
> > executing driver callbacks.
> 
> Stop for a second. The question is why would anybody want to relax
> that?
> 
> That unbinding a driver from a device is separated from module
> unloading is a design we had in Linux since the very beginning.
> 
> 
And that is kept here, You can unbind and rebind a device at any time,
the difference is that for a module whose devices are unused by user-
space, a rmmod triggers an implicit unbind / cleanup instead of removal
being blocked requiring an explicit unbind of all devices.

Thanks,
Thomas


> Regards,
> Christian.
> 
> > 
> > To help with the latter, add a drm_dev_release_barrier() function.
> > The function ensures that any caller that has started executing
> > device release callbacks has also finished executing them.
> > 
> > Use SRCU for the implementation.
> > 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_drv.c | 25 +++++++++++++++++++++++++
> >  include/drm/drm_drv.h     |  1 +
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 2915118436ce..9fa72adf173d 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -898,21 +898,46 @@ struct drm_device *drm_dev_alloc(const struct
> > drm_driver *driver,
> >  }
> >  EXPORT_SYMBOL(drm_dev_alloc);
> >  
> > +DEFINE_STATIC_SRCU(drm_dev_release_srcu);
> > +
> >  static void drm_dev_release(struct kref *ref)
> >  {
> >  	struct drm_device *dev = container_of(ref, struct
> > drm_device, ref);
> > +	int idx;
> >  
> >  	/* Just in case register/unregister was never called */
> >  	drm_debugfs_dev_fini(dev);
> >  
> > +	idx = srcu_read_lock(&drm_dev_release_srcu);
> >  	if (dev->driver->release)
> >  		dev->driver->release(dev);
> >  
> >  	drm_managed_release(dev);
> > +	srcu_read_unlock(&drm_dev_release_srcu, idx);
> >  
> >  	kfree(dev->managed.final_kfree);
> >  }
> >  
> > +/**
> > + * drm_dev_release_barrier() - Ensure drm device release callbacks
> > are finished
> > + *
> > + * If a device release method or any of the drm managed release
> > callbacks
> > + * have been called for a device, wait until all of them have
> > finished
> > + * executing. This function can be used to help determine whether
> > it's safe
> > + * to unload a driver module.
> > + *
> > + * Assume for example the driver maintains a device count which is
> > decremented
> > + * using a drmm callback or a device release callback. From a drm
> > device
> > + * lifetime POV, it's then safe to unload the driver when that
> > device-count
> > + * has reached zero and drm_dev_release_barrier() has been called.
> > + */
> > +void drm_dev_release_barrier(void)
> > +{
> > +	synchronize_srcu(&drm_dev_release_srcu);
> > +}
> > +EXPORT_SYMBOL(drm_dev_release_barrier);
> > +
> > +
> >  /**
> >   * drm_dev_get - Take reference of a DRM device
> >   * @dev: device to take reference of or NULL
> > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > index 42fc085f986d..b288a885cf45 100644
> > --- a/include/drm/drm_drv.h
> > +++ b/include/drm/drm_drv.h
> > @@ -489,6 +489,7 @@ void drm_dev_exit(int idx);
> >  void drm_dev_unplug(struct drm_device *dev);
> >  int drm_dev_wedged_event(struct drm_device *dev, unsigned long
> > method,
> >  			 struct drm_wedge_task_info *info);
> > +void drm_dev_release_barrier(void);
> >  
> >  /**
> >   * drm_dev_is_unplugged - is a DRM device unplugged

Re: [PATCH 1/2] drm: Provide a drm_dev_release_barrier() function to wait for device release callbacks

[Thomas Hellström]

On Mon, 2026-03-16 at 21:10 +0100, Thomas Hellström wrote:
> On Mon, 2026-03-16 at 18:42 +0100, Christian König wrote:
> > On 3/16/26 17:20, Thomas Hellström wrote:
> > > If helper components, like for example drm_pagemap hold
> > > references
> > > to
> > > drm devices, it's typically possible for the drm driver module to
> > > be
> > > unloaded without that reference being dropped, 
> > 
> > That is an extremely bad idea to begin with, drm_devices should
> > reference the module who issued them.
> 
> They typically don't. If that were the case you wouldn't be able to
> rmmod a module and have device cleanup happen:
> 
> module_exit();
> 
> pci_unregister_driver()-><starts device removal>
> devm_release()-><drop drm device reference>
> drmm_release()
> ...
> <module unloads>
> 
> I was under the impression that this was the behaviour of most device
> drivers and also drm drivers if display isn't enabled.
> (used to work with xe but doesn't anymore for unknown reason).

FWIW, it's display and mei taking an additional reference on the xe
module. Otherwise rmmod triggers an implicit unbind just as expected.

/Thomas