Message ID | 20220405210335.3434130-15-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | fbcon cleanups | expand |
Hi Daniel, On Tue, Apr 05, 2022 at 11:03:32PM +0200, Daniel Vetter wrote: > Ideally console_lock becomes an implementation detail of fbcon.c and > doesn't show up anywhere in fbmem.c. We're still pretty far from that, > but at least the register/unregister code is there now. > > With this the do_fb_ioctl() handler is the only code in fbmem.c still > calling console_lock(). > > Acked-by: Sam Ravnborg <sam@ravnborg.org> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > Cc: Daniel Vetter <daniel@ffwll.ch> > 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: Sam Ravnborg <sam@ravnborg.org> > Cc: Zheyu Ma <zheyuma97@gmail.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Alex Deucher <alexander.deucher@amd.com> > Cc: Zhen Lei <thunder.leizhen@huawei.com> > Cc: Xiyu Yang <xiyuyang19@fudan.edu.cn> This patch as commit 6e7da3af008b ("fbcon: Move console_lock for register/unlink/unregister") in next-20220411 causes a lock up on my test Intel desktop; I get no output on my display and I cannot ssh into the machine. Bisect log below. If back out of this patch and the ones that follow in this series on top of next-20220411, everything works properly (i.e. 'git diff 6e7da3af008b^..efc3acbc105a | git apply -R'). What information would be helpful for debugging this? The system has an i7-11700 in it and it is booting under UEFI, so I assume it should be using EFI_FB. I am happy to offer any debugging information or test any patches. # bad: [d12d7e1cfe38e0c36d28c7a9fbbc436ad0d17c14] Add linux-next specific files for 20220411 # good: [8b57b3046107b50ebecb65537a172ef3d6cec673] Merge tag 'tty-5.18-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty git bisect start 'd12d7e1cfe38e0c36d28c7a9fbbc436ad0d17c14' '8b57b3046107b50ebecb65537a172ef3d6cec673' # bad: [8f0b3ef070bd35e80da1caa85824fc344fb6b82e] Merge branch 'for-linux-next' of git://anongit.freedesktop.org/drm/drm-misc git bisect bad 8f0b3ef070bd35e80da1caa85824fc344fb6b82e # good: [69ae2d6587df089ec1c21bddddc7852df403d3ce] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git git bisect good 69ae2d6587df089ec1c21bddddc7852df403d3ce # good: [43ffc7321ed6ce13e5424fa687576f4442c25f9d] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git git bisect good 43ffc7321ed6ce13e5424fa687576f4442c25f9d # good: [d44c2642c40b6c11e4a0afc76d491287c074376c] drm/gem: Delete gem array fencing helpers git bisect good d44c2642c40b6c11e4a0afc76d491287c074376c # good: [3223e922ccf8b5c3dd0b05faeaba407655ee0774] orinoco: Prepare cleanup of powerpc's asm/prom.h git bisect good 3223e922ccf8b5c3dd0b05faeaba407655ee0774 # good: [689333136327b6cd618df85d83d79f2aa620d585] fbcon: Move fbcon_bmove(_rec) functions git bisect good 689333136327b6cd618df85d83d79f2aa620d585 # good: [840db0076b05a1dd4c2ded8d08f8481e0297d09f] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git git bisect good 840db0076b05a1dd4c2ded8d08f8481e0297d09f # bad: [6e7da3af008b72520f5318507f455f344b27f022] fbcon: Move console_lock for register/unlink/unregister git bisect bad 6e7da3af008b72520f5318507f455f344b27f022 # good: [6b2060cf9138a2cd5f3468a949d3869abed049ef] fb: Delete fb_info->queue git bisect good 6b2060cf9138a2cd5f3468a949d3869abed049ef # good: [d443d93864726ad68c0a741d1e7b03934a9af143] fbcon: move more common code into fb_open() git bisect good d443d93864726ad68c0a741d1e7b03934a9af143 # good: [43553559121ca90965b572cf8a1d6d0fd618b449] fbcon: Consistently protect deferred_takeover with console_lock() git bisect good 43553559121ca90965b572cf8a1d6d0fd618b449 # first bad commit: [6e7da3af008b72520f5318507f455f344b27f022] fbcon: Move console_lock for register/unlink/unregister Cheers, Nathan
On Mon, Apr 11, 2022 at 03:27:47PM -0700, Nathan Chancellor wrote: > Hi Daniel, > > On Tue, Apr 05, 2022 at 11:03:32PM +0200, Daniel Vetter wrote: > > Ideally console_lock becomes an implementation detail of fbcon.c and > > doesn't show up anywhere in fbmem.c. We're still pretty far from that, > > but at least the register/unregister code is there now. > > > > With this the do_fb_ioctl() handler is the only code in fbmem.c still > > calling console_lock(). > > > > Acked-by: Sam Ravnborg <sam@ravnborg.org> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Daniel Vetter <daniel@ffwll.ch> > > 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: Sam Ravnborg <sam@ravnborg.org> > > Cc: Zheyu Ma <zheyuma97@gmail.com> > > Cc: Guenter Roeck <linux@roeck-us.net> > > Cc: Alex Deucher <alexander.deucher@amd.com> > > Cc: Zhen Lei <thunder.leizhen@huawei.com> > > Cc: Xiyu Yang <xiyuyang19@fudan.edu.cn> > > This patch as commit 6e7da3af008b ("fbcon: Move console_lock for > register/unlink/unregister") in next-20220411 causes a lock up on my > test Intel desktop; I get no output on my display and I cannot ssh into > the machine. Bisect log below. If back out of this patch and the ones > that follow in this series on top of next-20220411, everything works > properly (i.e. 'git diff 6e7da3af008b^..efc3acbc105a | git apply -R'). > > What information would be helpful for debugging this? The system has an > i7-11700 in it and it is booting under UEFI, so I assume it should be > using EFI_FB. I am happy to offer any debugging information or test any > patches. I think I got it, typing a patch now for you to test. Thanks a lot for the bug report and bisect! -Daniel > > # bad: [d12d7e1cfe38e0c36d28c7a9fbbc436ad0d17c14] Add linux-next specific files for 20220411 > # good: [8b57b3046107b50ebecb65537a172ef3d6cec673] Merge tag 'tty-5.18-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty > git bisect start 'd12d7e1cfe38e0c36d28c7a9fbbc436ad0d17c14' '8b57b3046107b50ebecb65537a172ef3d6cec673' > # bad: [8f0b3ef070bd35e80da1caa85824fc344fb6b82e] Merge branch 'for-linux-next' of git://anongit.freedesktop.org/drm/drm-misc > git bisect bad 8f0b3ef070bd35e80da1caa85824fc344fb6b82e > # good: [69ae2d6587df089ec1c21bddddc7852df403d3ce] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git > git bisect good 69ae2d6587df089ec1c21bddddc7852df403d3ce > # good: [43ffc7321ed6ce13e5424fa687576f4442c25f9d] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git > git bisect good 43ffc7321ed6ce13e5424fa687576f4442c25f9d > # good: [d44c2642c40b6c11e4a0afc76d491287c074376c] drm/gem: Delete gem array fencing helpers > git bisect good d44c2642c40b6c11e4a0afc76d491287c074376c > # good: [3223e922ccf8b5c3dd0b05faeaba407655ee0774] orinoco: Prepare cleanup of powerpc's asm/prom.h > git bisect good 3223e922ccf8b5c3dd0b05faeaba407655ee0774 > # good: [689333136327b6cd618df85d83d79f2aa620d585] fbcon: Move fbcon_bmove(_rec) functions > git bisect good 689333136327b6cd618df85d83d79f2aa620d585 > # good: [840db0076b05a1dd4c2ded8d08f8481e0297d09f] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git > git bisect good 840db0076b05a1dd4c2ded8d08f8481e0297d09f > # bad: [6e7da3af008b72520f5318507f455f344b27f022] fbcon: Move console_lock for register/unlink/unregister > git bisect bad 6e7da3af008b72520f5318507f455f344b27f022 > # good: [6b2060cf9138a2cd5f3468a949d3869abed049ef] fb: Delete fb_info->queue > git bisect good 6b2060cf9138a2cd5f3468a949d3869abed049ef > # good: [d443d93864726ad68c0a741d1e7b03934a9af143] fbcon: move more common code into fb_open() > git bisect good d443d93864726ad68c0a741d1e7b03934a9af143 > # good: [43553559121ca90965b572cf8a1d6d0fd618b449] fbcon: Consistently protect deferred_takeover with console_lock() > git bisect good 43553559121ca90965b572cf8a1d6d0fd618b449 > # first bad commit: [6e7da3af008b72520f5318507f455f344b27f022] fbcon: Move console_lock for register/unlink/unregister > > Cheers, > Nathan
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index 4f9752ee9189..abb419a091c6 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -2756,10 +2756,12 @@ void fbcon_fb_unbind(struct fb_info *info) int i, new_idx = -1; int idx = info->node; - WARN_CONSOLE_UNLOCKED(); + console_lock(); - if (!fbcon_has_console_bind) + if (!fbcon_has_console_bind) { + console_unlock(); return; + } for (i = first_fb_vc; i <= last_fb_vc; i++) { if (con2fb_map[i] != idx && @@ -2794,6 +2796,8 @@ void fbcon_fb_unbind(struct fb_info *info) } fbcon_unbind(); } + + console_unlock(); } /* called with console_lock held */ @@ -2801,10 +2805,12 @@ void fbcon_fb_unregistered(struct fb_info *info) { int i, idx; - WARN_CONSOLE_UNLOCKED(); + console_lock(); - if (deferred_takeover) + if (deferred_takeover) { + console_unlock(); return; + } idx = info->node; for (i = first_fb_vc; i <= last_fb_vc; i++) { @@ -2833,6 +2839,7 @@ void fbcon_fb_unregistered(struct fb_info *info) if (!num_registered_fb) do_unregister_con_driver(&fb_con); + console_unlock(); } void fbcon_remap_all(struct fb_info *info) @@ -2890,19 +2897,27 @@ static inline void fbcon_select_primary(struct fb_info *info) } #endif /* CONFIG_FRAMEBUFFER_DETECT_PRIMARY */ +static bool lockless_register_fb; +module_param_named_unsafe(lockless_register_fb, lockless_register_fb, bool, 0400); +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) { int ret = 0, i, idx; - WARN_CONSOLE_UNLOCKED(); + if (!lockless_register_fb) + console_lock(); + else + atomic_inc(&ignore_console_lock_warning); idx = info->node; fbcon_select_primary(info); if (deferred_takeover) { pr_info("fbcon: Deferring console take-over\n"); - return 0; + goto out; } if (info_idx == -1) { @@ -2922,6 +2937,12 @@ int fbcon_fb_registered(struct fb_info *info) } } +out: + if (!lockless_register_fb) + console_unlock(); + else + atomic_dec(&ignore_console_lock_warning); + return ret; } diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 0e68d9456bc2..bdd00d381bbc 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1590,14 +1590,9 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a, } } -static bool lockless_register_fb; -module_param_named_unsafe(lockless_register_fb, lockless_register_fb, bool, 0400); -MODULE_PARM_DESC(lockless_register_fb, - "Lockless framebuffer registration for debugging [default=off]"); - static int do_register_framebuffer(struct fb_info *fb_info) { - int i, ret; + int i; struct fb_videomode mode; if (fb_check_foreignness(fb_info)) @@ -1666,17 +1661,7 @@ static int do_register_framebuffer(struct fb_info *fb_info) } #endif - if (!lockless_register_fb) - console_lock(); - else - atomic_inc(&ignore_console_lock_warning); - ret = fbcon_fb_registered(fb_info); - - if (!lockless_register_fb) - console_unlock(); - else - atomic_dec(&ignore_console_lock_warning); - return ret; + return fbcon_fb_registered(fb_info); } static void unbind_console(struct fb_info *fb_info) @@ -1686,9 +1671,7 @@ static void unbind_console(struct fb_info *fb_info) if (WARN_ON(i < 0 || i >= FB_MAX || registered_fb[i] != fb_info)) return; - console_lock(); fbcon_fb_unbind(fb_info); - console_unlock(); } static void unlink_framebuffer(struct fb_info *fb_info) @@ -1731,9 +1714,7 @@ static void do_unregister_framebuffer(struct fb_info *fb_info) fb_notifier_call_chain(FB_EVENT_FB_UNREGISTERED, &event); } #endif - console_lock(); fbcon_fb_unregistered(fb_info); - console_unlock(); /* this may free fb info */ put_fb_info(fb_info);