Messages in this thread | | | Date | Wed, 11 May 2022 13:47:03 +0200 | Subject | Re: [PATCH v5 3/7] fbdev: Restart conflicting fb removal loop when unregistering devices | From | Thomas Zimmermann <> |
| |
Hi Javier
Am 11.05.22 um 13:30 schrieb Javier Martinez Canillas: > Drivers that want to remove registered conflicting framebuffers prior to > register their own framebuffer, calls remove_conflicting_framebuffers(). > > This function takes the registration_lock mutex, to prevent a races when > drivers register framebuffer devices. But if a conflicting framebuffer > device is found, the underlaying platform device is unregistered and this > will lead to the platform driver .remove callback to be called, which in > turn will call to the unregister_framebuffer() that takes the same lock. > > To prevent this, a struct fb_info.forced_out field was used as indication > to unregister_framebuffer() whether the mutex has to be grabbed or not. > > A cleaner solution is to drop the lock before platform_device_unregister() > so unregister_framebuffer() can take it when called from the fbdev driver, > and just grab the lock again after the device has been registered and do > a removal loop restart. > > Since the framebuffer devices will already be removed, the loop would just > finish when no more conflicting framebuffers are found. > > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I'd like to shrink this patchset. This looks like it can be merged immediately?
Best regards Thomas
> --- > > (no changes since v1) > > drivers/video/fbdev/core/fbmem.c | 22 +++++++++++++++------- > include/linux/fb.h | 1 - > 2 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c > index b445a7a00def..2fda5917c212 100644 > --- a/drivers/video/fbdev/core/fbmem.c > +++ b/drivers/video/fbdev/core/fbmem.c > @@ -1555,6 +1555,7 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, > { > int i; > > +restart_removal: > /* check all firmware fbs and kick off if the base addr overlaps */ > for_each_registered_fb(i) { > struct apertures_struct *gen_aper; > @@ -1587,12 +1588,23 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, > pr_warn("fb%d: no device set\n", i); > do_unregister_framebuffer(registered_fb[i]); > } else if (dev_is_platform(device)) { > - registered_fb[i]->forced_out = true; > + /* > + * Drop the lock because if the device is unregistered, its > + * driver will call to unregister_framebuffer(), that takes > + * this lock. > + */ > + mutex_unlock(®istration_lock); > platform_device_unregister(to_platform_device(device)); > + mutex_lock(®istration_lock); > } else { > pr_warn("fb%d: cannot remove device\n", i); > do_unregister_framebuffer(registered_fb[i]); > } > + /* > + * Restart the removal loop now that the device has been > + * unregistered and its associated framebuffer gone. > + */ > + goto restart_removal; > } > } > } > @@ -1899,13 +1911,9 @@ EXPORT_SYMBOL(register_framebuffer); > void > unregister_framebuffer(struct fb_info *fb_info) > { > - bool forced_out = fb_info->forced_out; > - > - if (!forced_out) > - mutex_lock(®istration_lock); > + mutex_lock(®istration_lock); > do_unregister_framebuffer(fb_info); > - if (!forced_out) > - mutex_unlock(®istration_lock); > + mutex_unlock(®istration_lock); > } > EXPORT_SYMBOL(unregister_framebuffer); > > diff --git a/include/linux/fb.h b/include/linux/fb.h > index 69c67c70fa78..bbe1e4571899 100644 > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -511,7 +511,6 @@ struct fb_info { > } *apertures; > > bool skip_vt_switch; /* no VT switch on suspend/resume required */ > - bool forced_out; /* set when being removed by another driver */ > }; > > static inline struct apertures_struct *alloc_apertures(unsigned int max_num) {
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [unhandled content-type:application/pgp-signature] | |