[PATCH] fbcon: Fix delayed takeover locking

[Daniel Vetter]

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

> 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