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