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

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

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