Re: [PATCH 2/3] fbdev/simplefb: Cleanup fb_info in .fb_destroy rather than .remove

Hello Thomas,

On 5/5/22 09:29, Thomas Zimmermann wrote:

[snip]

quoted

  static void simplefb_destroy(struct fb_info *info)
  {
  	struct simplefb_par *par = info->par;

@@ -94,6 +98,8 @@ static void simplefb_destroy(struct fb_info *info)
  	if (info->screen_base)
  		iounmap(info->screen_base);
  
+	framebuffer_release(info);
+
  	if (mem)
  		release_mem_region(mem->start, resource_size(mem));

The original problem with fbdev hot-unplug was that vmwgfx needed the 
framebuffer region to be released. If we release it only after userspace 
closed it's final file descriptor, vmwgfx could have already failed.

I still don't fully get why this code apparently works or at least 
doesn't blow up occasionally. Any ideas?

I believe that vmwgfx doesn't fail to probe (or any other DRM driver)
only when there are not user-space processes with a fbdev node opened
since otherwise as you said the memory wouldn't be released yet.

unregister_framebuffer() is called from the driver's .remove handler
and that decrement the fb_info refcount, so if reaches zero it will
call to the fb fops .destroy() handler and release the I/O memory.

In other words, in most cases (i.e: only fbcon bound to the fbdev)
the driver's removal/ device unbind and the memory release will be
at the same time.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat