On 2026-08-28 at 01:14:57 +0200, Andi Shyti wrote: > Hi Krzysztof, > > ... > > > Amend the problem by removing debugfs entries and marking > > registered flag as false upon error in drm_dev_register(). > > > > Signed-off-by: Krzysztof Karas <[email protected]> > > Co-developed-by: Krzysztof Niemiec <[email protected]> > > Signed-off-by: Krzysztof Niemiec <[email protected]> > > I think the right order should be: > > Co-developed-by: Krzysztof Karas <[email protected]> > Signed-off-by: Krzysztof Karas <[email protected]> > Signed-off-by: Krzysztof Niemiec <[email protected]> > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index e51ed959da89..b98e8af6f1c1 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -1140,6 +1140,8 @@ int drm_dev_register(struct drm_device *dev, unsigned > > long flags) > > drm_minor_unregister(dev, DRM_MINOR_ACCEL); > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY); > > drm_minor_unregister(dev, DRM_MINOR_RENDER); > > + drm_debugfs_dev_fini(dev); > > I think sashiko is right here, this should be removed. > > > + dev->registered = false; > > dev->registered = false should be set at the very beginning, > maybe something like this:
Why? Is .unload() supposed to expect dev->registered = false? I've seen that moving the flag around might(?) make a bit of a mess (e0f32f78e51b), and before this patch .unload() is being called with registered = true. So changing it at the beginning of the unwind might mess up drivers that rely on it, but it will make no difference for drivers that don't, so I'd rather keep it this way. Unless we make explicit guarantees that .unload() will be called with ->registered = false, in the middle of the call to drm_dev_register(). > > err_unload: > dev->registered = false; > if (dev->driver->unload) > dev->driver->unload(dev); > goto err_cleanup; > err_minors: > dev->registered = false; > err_clenup: > ... > > Thanks, > Andi
