From 9d79799193b728b62c9899d931b5009da1f89b67 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 13 Apr 2022 10:21:28 +0200 Subject: [PATCH] fbcon: Fix delayed takeover locking I messed up the delayed takover path in the locking conversion in 6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister"). If CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER is enabled, fbcon take-over doesn't take place when calling fbcon_fb_registered(). Instead, is deferred using a workqueue and its fbcon_register_existing_fbs() function calls to fbcon_fb_registered() again for each registered fbcon fb. This leads to the console_lock tried to be held twice, causing a deadlock. 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. While reviewing the broken commit I realized that I've left some outdated comments about the locking behind. Fix those too. v2: Improve commit message (Javier) Reported-by: Nathan Chancellor Tested-by: Nathan Chancellor Reviewed-by: Javier Martinez Canillas Cc: Nathan Chancellor Fixes: 6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister") Cc: Sam Ravnborg Cc: Thomas Zimmermann Cc: Du Cheng Cc: Claudio Suarez Cc: Greg Kroah-Hartman Cc: Tetsuo Handa Cc: Matthew Wilcox Cc: Zheyu Ma Cc: Guenter Roeck Cc: Helge Deller Cc: Geert Uytterhoeven Cc: Javier Martinez Canillas Signed-off-by: Daniel Vetter Link: https://patchwork.freedesktop.org/patch/msgid/20220413082128.348186-1-daniel.vetter@ffwll.ch --- 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 c56dd2f..34744a1 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.7.4