Re: [PATCH 1/5] fbdev: Hot-unplug firmware fb devices on forced removal

On 1/24/22 14:52, Javier Martinez Canillas wrote:

[snip]

...

@@ -1898,9 +1917,13 @@ EXPORT_SYMBOL(register_framebuffer); void unregister_framebuffer(struct fb_info *fb_info) {

• mutex_lock(&registration_lock);

• bool forced_out = fb_info->forced_out;
• if (!forced_out)

• mutex_lock(&registration_lock);
  

  do_unregister_framebuffer(fb_info);

• mutex_unlock(&registration_lock);

• if (!forced_out)

• mutex_unlock(&registration_lock);
  

}

I'm not sure to follow the logic here. The forced_out bool is set when the platform device is unregistered in do_remove_conflicting_framebuffers(), but shouldn't the struct platform_driver .remove callback be executed even in this case ?

That is, the platform_device_unregister() will trigger the call to the .remove callback that in turn will call unregister_framebuffer().

Shouldn't we always hold the mutex when calling do_unregister_framebuffer() ?

Scratch that, I got it now. That's exactly the reason why you skip the mutext_lock(). After adding the check for dev, feel free to add:

Reviewed-by: Javier Martinez Canillas javierm@redhat.com

Best regards,