Hi Daniel,
On Mon, Jan 31, 2022 at 10:05:49PM +0100, Daniel Vetter wrote:
> There's a bunch of confusions going on here:
> - The deferred fbcon setup notifier should only be cleaned up from
> fb_console_exit(), to be symmetric with fb_console_init()
> - We also need to make sure we don't race with the work, which means
> temporarily dropping the console lock (or we can deadlock)
> - That also means no point in clearing deferred_takeover, we are
> unloading everything anyway.
> - Finally rename fbcon_exit to fbcon_release_all and move it, since
> that's what's it doing when being called from consw->con_deinit
> through fbcon_deinit.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Claudio Suarez <cssk@xxxxxxxx>
> Cc: Du Cheng <ducheng2@xxxxxxxxx>
> Cc: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> ---
> drivers/video/fbdev/core/fbcon.c | 63 ++++++++++++++++----------------
> 1 file changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 5c14e24d14a1..22581952b4fd 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -185,7 +185,6 @@ static void fbcon_set_disp(struct fb_info *info, struct fb_var_screeninfo *var,
> int unit);
> static void fbcon_modechanged(struct fb_info *info);
> static void fbcon_set_all_vcs(struct fb_info *info);
> -static void fbcon_exit(void);
>
> static struct device *fbcon_device;
>
> @@ -1149,6 +1148,27 @@ static void fbcon_free_font(struct fbcon_display *p, bool freefont)
>
> static void set_vc_hi_font(struct vc_data *vc, bool set);
>
> +static void fbcon_release_all(void)
> +{
> + struct fb_info *info;
> + int i, j, mapped;
> +
> + for_each_registered_fb(i) {
> + mapped = 0;
> + info = registered_fb[i];
> +
> + for (j = first_fb_vc; j <= last_fb_vc; j++) {
> + if (con2fb_map[j] == i) {
> + mapped = 1;
> + con2fb_map[j] = -1;
> + }
> + }
> +
> + if (mapped)
> + fbcon_release(info);
> + }
> +}
> +
> static void fbcon_deinit(struct vc_data *vc)
> {
> struct fbcon_display *p = &fb_display[vc->vc_num];
> @@ -1188,7 +1208,7 @@ static void fbcon_deinit(struct vc_data *vc)
> set_vc_hi_font(vc, false);
>
> if (!con_is_bound(&fb_con))
> - fbcon_exit();
> + fbcon_release_all();
>
> if (vc->vc_num == logo_shown)
> logo_shown = FBCON_LOGO_CANSHOW;
> @@ -3316,34 +3336,6 @@ static void fbcon_start(void)
> #endif
> }
>
> -static void fbcon_exit(void)
> -{
> - struct fb_info *info;
> - int i, j, mapped;
> -
> -#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> - if (deferred_takeover) {
> - dummycon_unregister_output_notifier(&fbcon_output_nb);
> - deferred_takeover = false;
> - }
> -#endif
> -
> - for_each_registered_fb(i) {
> - mapped = 0;
> - info = registered_fb[i];
> -
> - for (j = first_fb_vc; j <= last_fb_vc; j++) {
> - if (con2fb_map[j] == i) {
> - mapped = 1;
> - con2fb_map[j] = -1;
> - }
> - }
> -
> - if (mapped)
> - fbcon_release(info);
> - }
> -}
> -
> void __init fb_console_init(void)
> {
> int i;
> @@ -3383,10 +3375,19 @@ static void __exit fbcon_deinit_device(void)
>
> void __exit fb_console_exit(void)
> {
> +#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
> + console_lock();
> + if (deferred_takeover)
> + dummycon_unregister_output_notifier(&fbcon_output_nb);
> + console_unlock();
> +
> + cancel_work_sync(&fbcon_deferred_takeover_work);
> +#endif
> +
> console_lock();
> fbcon_deinit_device();
> device_destroy(fb_class, MKDEV(0, 0));
> - fbcon_exit();
> +
We loose the call to fbcon_release_all() here.
We have part of the old fbcon_exit() above, but miss the release parts.
Maybe I missed something obvious?
The rest looks fine.
Sam
> do_unregister_con_driver(&fb_con);
> console_unlock();
> }
> --
> 2.33.0
