There's a pile of things protecting against racing hotunplugs:
- drm_dev_enter/exit against the underlying device disappearing
- drm_dev_get/put against drm_device disappearing
- we unregister everything in drm_dev_unregister to stop userspace from
creating new open files to access the drm_device
- fbcon gets synchronously torn down (the console_lock is huge, so there's
an upside to monolithic locking here).
- and for fbdev there's get_fb_info and put_fb_info
And I think we've almost got this right, except for a tiny thing: An fbdev
ioctl might hold an elevate fb_info reference, which prevents us from
doing the final cleanup in drm_fbdev_release. But it doesn't prevent
the final cleanup of fbhelper->dev, which means even if the driver protects
all hw access with drm_dev_enter/exit, we might oops way before that when
trying to access freed memory.
Fix this by holding a drm_device reference for fbdev, which we drop from
drm_fbdev_release.
This might lead to a reference loop, where the drm_device won't get cleaned
up because of the fbdev reference, and fbdev isn't torn down yet because
the drm_device cleanup code is written the wrong way round. But I think
all drivers using the generic fbdev code get this right (because they use
drm_client, so really can't get it wrong), hence this should be ok.
Also, more reasons to switch drivers over to generic fbdev support.
Since I had to re-review a pile of code also add a comment about why
drm_fbdev_client_unregister can't race.
Cc: Gerd Hoffmann <kraxel@xxxxxxxxxx>
Cc: Noralf Trønnes <noralf@xxxxxxxxxxx>
Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
--
Entirely untested (aside from it compiles), but I think we need
something like this to completely plug all fbdev related hotunplug
issues. No need to roll out some fancy srcu scheme to fbdev, this
seems to be solved with refcounting already.
-Daniel
---
drivers/gpu/drm/drm_fb_helper.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0ec98e046b59..62a1981d369e 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1992,6 +1992,7 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
static void drm_fbdev_release(struct drm_fb_helper *fb_helper)
{
+ drm_dev_put(fb_helper->dev);
drm_fbdev_cleanup(fb_helper);
drm_client_release(&fb_helper->client);
kfree(fb_helper);
@@ -2123,6 +2124,7 @@ static void drm_fbdev_client_unregister(struct drm_client_dev *client)
{
struct drm_fb_helper *fb_helper = drm_fb_helper_from_client(client);
+ /* Protected against races by the clientlist_mutex. */
if (fb_helper->fbdev)
/* drm_fbdev_fb_destroy() takes care of cleanup */
drm_fb_helper_unregister_fbi(fb_helper);
@@ -2243,6 +2245,8 @@ int drm_fbdev_generic_setup(struct drm_device *dev, unsigned int preferred_bpp)
preferred_bpp = 32;
fb_helper->preferred_bpp = preferred_bpp;
+ drm_dev_get(dev);
+
ret = drm_fbdev_client_hotplug(&fb_helper->client);
if (ret)
DRM_DEV_DEBUG(dev->dev, "client hotplug ret=%d\n", ret);
--
2.24.0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
