On Mon, Aug 21, 2017 at 08:53:08PM +0200, Noralf Trønnes wrote: > Hi, > > I'm looking into what happens if fbdev has open file handles when > drm_dev_unplug() is called. udl is the only user of drm_dev_unplug(), > but AFAICT it will keel over if unplugged with a /dev/fb handle open > since fbdev is torn down during drm_driver->unload. > > My first attempt was to rely on the drm_device ref counting and tear > down fbdev in drm_driver->release. This requires that a ref is taken on > drm_device in &fb_ops.fb_open and dropped in &fb_ops.fb_release. > The problem is that .fb_release is called under console_lock(), which > means that unregister_framebuffer() will deadlock (assuming last ref on > drm_device here). Trying to do trickery with console_lock to get this > through is bound to be painful, so a straightforward solution is to fork > of fbdev teardown to a worker. > Since we are already in drm_driver->release, this won't work. > > My next idea is to refcount struct drm_fb_helper, take one ref on > drm_device and handle the lifetime of fbdev that way. Now it's possible to > do teardown in a worker and when that's done, drop the ref on drm_device. > The plan was to make this ref counting optional for drivers. > > Is ref counting drm_fb_helper a good/bad idea, or is there another > solution to this? I think we'd need to do something similar to what we do with the main drm devices: - On unplug we unregister the framebuffer to stop any userspace apps from opening it and creating new references. - Every time someone opens the /dev/fb0 node for our emulated fbdev, we call drm_dev_ref(), and drm_dev_unref() on close. - We need to sprinkle drm_dev_is_unplugged() checks over all the fbdev entry points. - Getting this right for the mmaps will be additional fun, but then we don't really get that right for drm either ... - (Aside, those should be rename to _get()/_put() for ocd consistency, it's the prefererred refcounting naming convention in the kernel). I have no idea whether fbdev supports this, it was kinda created before hotplug was a thing :-( But from a quick look we do have fb_open/close callbacks in fb_ops, so this should be doable. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
