The drm device registration is done via drm_dev_register().
This function attempts to undo some of the initiatlization steps
under err_unload and err_minors labels, but this process is
incomplete - debugfs entries remain and the dev->registered flag
is still set to true.
On the other hand, drm_dev_unregister() tears down everything
that was initialized during registration step. This is confusing
when considering a failure in drm_dev_register(), because some
setup steps will be undone and some settings will remain, so it
is unclear whether a call to drm_dev_unregister() is a
requirement or not.
What is more, commit 6915190a50e8 ("drm/client: Support
emergency restore via sysrq for all clients") introduced a new
WARN_ON and added drm_client_sysrq_register/unregister() calls
to the drm device register/unregister paths. Client sysrqs are
registered only when drm_dev_register() is sure to complete
without failures, but drm_client_sysrq_unregister() gets called
every time during drm_dev_unregister() and prints warning
messages whenever the caller tries to clean up the mess with
immediate unregistration.
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]>
Reviewed-by: Maarten Lankhorst <[email protected]>
---
I noticed that some drivers use drm_dev_unregister() whenever
a call to drm_dev_register() and many do not. It is also a bit
confusing to me why drm_dev_register() does not completely
unwind all the initialization steps it performs, which leaves
me wondering if drm_dev_unregister() is required on the error
path or not.
The WARN_ON introduced in 6915190a50e8f7cf13dcbe534b02845be533b60a
exposed this problem, as there were no notifications from these
functions, so I noticed this mismatch only thanks to the warning
messages.
RFC -> v1:
* addedd title of commit 6915190a50e8 to commit message;
* rebased;
drivers/gpu/drm/drm_drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 2915118436ce..a502110696a3 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -1119,6 +1119,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);
+ dev->registered = false;
out_unlock:
if (drm_dev_needs_global_mutex(dev))
mutex_unlock(&drm_global_mutex);
--
2.43.0
--
Best Regards,
Krzysztof
