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 <[email protected]> > --- > 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
