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
> 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
