drm, fbdev emulation and drm_dev_unplug()

[Daniel Vetter]

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