[Intel-gfx] [PATCH] fbcon: Fix delayed takeover locking

[Nathan Chancellor]

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 at kernel.org>
> > Cc: Nathan Chancellor <nathan at kernel.org>
> 
> 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 at kernel.org>

> > Fixes: 6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister")
> > Cc: Sam Ravnborg <sam at ravnborg.org>
> > Cc: Thomas Zimmermann <tzimmermann at suse.de>
> > Cc: Du Cheng <ducheng2 at gmail.com>
> > Cc: Claudio Suarez <cssk at net-c.es>
> > Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> > Cc: Tetsuo Handa <penguin-kernel at I-love.SAKURA.ne.jp>
> > Cc: Matthew Wilcox <willy at infradead.org>
> > Cc: Zheyu Ma <zheyuma97 at gmail.com>
> > Cc: Guenter Roeck <linux at roeck-us.net>
> > Cc: Helge Deller <deller at gmx.de>
> > Cc: Geert Uytterhoeven <geert at linux-m68k.org>
> > Cc: Javier Martinez Canillas <javierm at redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  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