On Wed, Apr 13, 2022 at 11:25:02AM +0200, Daniel Vetter wrote:
> On Wed, Apr 13, 2022 at 10:21:28AM +0200, Daniel Vetter wrote:
> > I messed up the delayed takover path in the locking conversion in
> > 6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister").
> >
> > Fix it by re-extracting the lockless function and using it in the
> > delayed takeover path, where we need to hold the lock already to
> > iterate over the list of already registered fb. Well the current code
> > still is broken in there (since the list is protected by a
> > registration_lock, which we can't take here because it nests the other
> > way round with console_lock), but in the future this will be a list
> > protected by console_lock when this is all sorted out.
> >
> > Reported-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> > Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
>
> Nathan, if you can supply a tested-by today still I could push it before I
> disappear into easter w/e. I'm pretty sure this is it, but better safe
> than sorry.
> -Daniel
Yup, sorry for the delay, timezones and such :( This patch resolves the
problem I was seeing, thank you for the quick response and fix!
Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> > Fixes: 6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister")
> > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx>
> > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx>
> > Cc: Du Cheng <ducheng2@xxxxxxxxx>
> > Cc: Claudio Suarez <cssk@xxxxxxxx>
> > Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> > Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> > Cc: Zheyu Ma <zheyuma97@xxxxxxxxx>
> > Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> > Cc: Helge Deller <deller@xxxxxx>
> > Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > Cc: Javier Martinez Canillas <javierm@xxxxxxxxxx>
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> > ---
> > drivers/video/fbdev/core/fbcon.c | 28 ++++++++++++++++++----------
> > 1 file changed, 18 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> > index 6a7d470beec7..b4e43b39d9a8 100644
> > --- a/drivers/video/fbdev/core/fbcon.c
> > +++ b/drivers/video/fbdev/core/fbcon.c
> > @@ -2772,7 +2772,6 @@ static void fbcon_unbind(void)
> > static inline void fbcon_unbind(void) {}
> > #endif /* CONFIG_VT_HW_CONSOLE_BINDING */
> >
> > -/* called with console_lock held */
> > void fbcon_fb_unbind(struct fb_info *info)
> > {
> > int i, new_idx = -1;
> > @@ -2822,7 +2821,6 @@ void fbcon_fb_unbind(struct fb_info *info)
> > console_unlock();
> > }
> >
> > -/* called with console_lock held */
> > void fbcon_fb_unregistered(struct fb_info *info)
> > {
> > int i, idx;
> > @@ -2928,14 +2926,11 @@ MODULE_PARM_DESC(lockless_register_fb,
> > "Lockless framebuffer registration for debugging [default=off]");
> >
> > /* called with console_lock held */
> > -int fbcon_fb_registered(struct fb_info *info)
> > +static int do_fb_registered(struct fb_info *info)
> > {
> > int ret = 0, i, idx;
> >
> > - if (!lockless_register_fb)
> > - console_lock();
> > - else
> > - atomic_inc(&ignore_console_lock_warning);
> > + WARN_CONSOLE_UNLOCKED();
> >
> > fbcon_registered_fb[info->node] = info;
> > fbcon_num_registered_fb++;
> > @@ -2945,7 +2940,7 @@ int fbcon_fb_registered(struct fb_info *info)
> >
> > if (deferred_takeover) {
> > pr_info("fbcon: Deferring console take-over\n");
> > - goto out;
> > + return 0;
> > }
> >
> > if (info_idx == -1) {
> > @@ -2965,7 +2960,20 @@ int fbcon_fb_registered(struct fb_info *info)
> > }
> > }
> >
> > -out:
> > + return ret;
> > +}
> > +
> > +int fbcon_fb_registered(struct fb_info *info)
> > +{
> > + int ret;
> > +
> > + if (!lockless_register_fb)
> > + console_lock();
> > + else
> > + atomic_inc(&ignore_console_lock_warning);
> > +
> > + ret = do_fb_registered(info);
> > +
> > if (!lockless_register_fb)
> > console_unlock();
> > else
> > @@ -3280,7 +3288,7 @@ static void fbcon_register_existing_fbs(struct work_struct *work)
> > logo_shown = FBCON_LOGO_DONTSHOW;
> >
> > fbcon_for_each_registered_fb(i)
> > - fbcon_fb_registered(fbcon_registered_fb[i]);
> > + do_fb_registered(fbcon_registered_fb[i]);
> >
> > console_unlock();
> > }
> > --
> > 2.34.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
