This is unused code since
commit 6104c37094e729f3d4ce65797002112735d49cd1
Author: Daniel Vetter <[email protected]>
Date: Tue Aug 1 17:32:07 2017 +0200
fbcon: Make fbcon a built-time depency for fbdev
when fbcon was made a compile-time static dependency of fbdev. We
can't exit fbcon anymore without exiting fbdev first, which only works
if all fbdev drivers have unloaded already. Hence this is all dead
code.
v2: I missed that fbcon_exit is also called from con_deinit stuff, and
there fbcon_has_exited prevents double-cleanup. But we can fix that
by properly resetting con2fb_map[] to all -1, which is used everywhere
else to indicate "no fb_info allocate to this console". With that
change the double-cleanup (which resulted in a module refcount underflow,
among other things) is prevented.
Aside: con2fb_map is a signed char, so don't register more than 128 fb_info
or hilarity will ensue.
Signed-off-by: Daniel Vetter <[email protected]>
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: "Noralf Trønnes" <[email protected]>
Cc: Yisheng Xie <[email protected]>
Cc: Konstantin Khorenko <[email protected]>
Cc: Prarit Bhargava <[email protected]>
Cc: Kees Cook <[email protected]>
---
drivers/video/fbdev/core/fbcon.c | 33 +-------------------------------
1 file changed, 1 insertion(+), 32 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 2b2082615ca1..a1be589b692f 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -112,7 +112,6 @@ static int softback_lines;
static int first_fb_vc;
static int last_fb_vc = MAX_NR_CONSOLES - 1;
static int fbcon_is_default = 1;
-static int fbcon_has_exited;
static int primary_device = -1;
static int fbcon_has_console_bind;
@@ -1050,7 +1049,6 @@ static const char *fbcon_startup(void)
info->var.bits_per_pixel);
fbcon_add_cursor_timer(info);
- fbcon_has_exited = 0;
return display_desc;
}
@@ -3336,14 +3334,6 @@ static int fbcon_event_notify(struct notifier_block *self,
struct fb_blit_caps *caps;
int idx, ret = 0;
- /*
- * ignore all events except driver registration and deregistration
- * if fbcon is not active
- */
- if (fbcon_has_exited && !(action == FB_EVENT_FB_REGISTERED ||
- action == FB_EVENT_FB_UNREGISTERED))
- goto done;
-
switch(action) {
case FB_EVENT_SUSPEND:
fbcon_suspended(info);
@@ -3396,7 +3386,6 @@ static int fbcon_event_notify(struct notifier_block *self,
fbcon_remap_all(idx);
break;
}
-done:
return ret;
}
@@ -3443,9 +3432,6 @@ static ssize_t store_rotate(struct device *device,
int rotate, idx;
char **last = NULL;
- if (fbcon_has_exited)
- return count;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3468,9 +3454,6 @@ static ssize_t store_rotate_all(struct device *device,
int rotate, idx;
char **last = NULL;
- if (fbcon_has_exited)
- return count;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3491,9 +3474,6 @@ static ssize_t show_rotate(struct device *device,
struct fb_info *info;
int rotate = 0, idx;
- if (fbcon_has_exited)
- return 0;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3514,9 +3494,6 @@ static ssize_t show_cursor_blink(struct device *device,
struct fbcon_ops *ops;
int idx, blink = -1;
- if (fbcon_has_exited)
- return 0;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3543,9 +3520,6 @@ static ssize_t store_cursor_blink(struct device *device,
int blink, idx;
char **last = NULL;
- if (fbcon_has_exited)
- return count;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3668,9 +3642,6 @@ static void fbcon_exit(void)
struct fb_info *info;
int i, j, mapped;
- if (fbcon_has_exited)
- return;
-
#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
if (deferred_takeover) {
dummycon_unregister_output_notifier(&fbcon_output_nb);
@@ -3695,7 +3666,7 @@ static void fbcon_exit(void)
for (j = first_fb_vc; j <= last_fb_vc; j++) {
if (con2fb_map[j] == i) {
mapped = 1;
- break;
+ con2fb_map[j] = -1;
}
}
@@ -3718,8 +3689,6 @@ static void fbcon_exit(void)
info->queue.func = NULL;
}
}
-
- fbcon_has_exited = 1;
}
void __init fb_console_init(void)
--
2.20.1
This is unused code since
commit 6104c37094e729f3d4ce65797002112735d49cd1
Author: Daniel Vetter <[email protected]>
Date: Tue Aug 1 17:32:07 2017 +0200
fbcon: Make fbcon a built-time depency for fbdev
when fbcon was made a compile-time static dependency of fbdev. We
can't exit fbcon anymore without exiting fbdev first, which only works
if all fbdev drivers have unloaded already. Hence this is all dead
code.
v2: I missed that fbcon_exit is also called from con_deinit stuff, and
there fbcon_has_exited prevents double-cleanup. But we can fix that
by properly resetting con2fb_map[] to all -1, which is used everywhere
else to indicate "no fb_info allocate to this console". With that
change the double-cleanup (which resulted in a module refcount underflow,
among other things) is prevented.
Aside: con2fb_map is a signed char, so don't register more than 128 fb_info
or hilarity will ensue.
v3: CI showed me that I still didn't fully understand what's going on
here. The leaked references in con2fb_map have been used upon
rebinding the fb console in fbcon_init. It worked because fbdev
unregistering still cleaned out con2fb_map, and reset it to info_idx.
If the last fbdev driver unregistered, then it also reset info_idx,
and unregistered the fbcon driver.
Imo that's all a bit fragile, so let's keep the con2fb_map reset to
-1, and in fbcon_init pick info_idx if we're starting fresh. That
means unbinding and rebinding will cleanse the mapping, but why are
you doing that if you want to retain the mapping, so should be fine.
Also, I think info_idx == -1 is impossible in fbcon_init - we
unregister the fbcon in that case. So catch&warn about that.
Signed-off-by: Daniel Vetter <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Bartlomiej Zolnierkiewicz <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: Hans de Goede <[email protected]>
Cc: "Noralf Trønnes" <[email protected]>
Cc: Yisheng Xie <[email protected]>
Cc: Konstantin Khorenko <[email protected]>
Cc: Prarit Bhargava <[email protected]>
Cc: Kees Cook <[email protected]>
---
drivers/video/fbdev/core/fbcon.c | 39 +++++---------------------------
1 file changed, 6 insertions(+), 33 deletions(-)
diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 5424051c8e1a..622d336cfc81 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -112,7 +112,6 @@ static int softback_lines;
static int first_fb_vc;
static int last_fb_vc = MAX_NR_CONSOLES - 1;
static int fbcon_is_default = 1;
-static int fbcon_has_exited;
static int primary_device = -1;
static int fbcon_has_console_bind;
@@ -1050,7 +1049,6 @@ static const char *fbcon_startup(void)
info->var.bits_per_pixel);
fbcon_add_cursor_timer(info);
- fbcon_has_exited = 0;
return display_desc;
}
@@ -1064,9 +1062,13 @@ static void fbcon_init(struct vc_data *vc, int init)
int logo = 1, new_rows, new_cols, rows, cols, charcnt = 256;
int cap, ret;
- if (info_idx == -1 || info == NULL)
+ if (WARN_ON(info_idx == -1))
return;
+ if (con2fb_map[vc->vc_num] == -1)
+ con2fb_map[vc->vc_num] = info_idx;
+
+ info = registered_fb[con2fb_map[vc->vc_num]];
cap = info->flags;
if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET)
@@ -3336,14 +3338,6 @@ static int fbcon_event_notify(struct notifier_block *self,
struct fb_blit_caps *caps;
int idx, ret = 0;
- /*
- * ignore all events except driver registration and deregistration
- * if fbcon is not active
- */
- if (fbcon_has_exited && !(action == FB_EVENT_FB_REGISTERED ||
- action == FB_EVENT_FB_UNREGISTERED))
- goto done;
-
switch(action) {
case FB_EVENT_SUSPEND:
fbcon_suspended(info);
@@ -3396,7 +3390,6 @@ static int fbcon_event_notify(struct notifier_block *self,
fbcon_remap_all(idx);
break;
}
-done:
return ret;
}
@@ -3443,9 +3436,6 @@ static ssize_t store_rotate(struct device *device,
int rotate, idx;
char **last = NULL;
- if (fbcon_has_exited)
- return count;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3468,9 +3458,6 @@ static ssize_t store_rotate_all(struct device *device,
int rotate, idx;
char **last = NULL;
- if (fbcon_has_exited)
- return count;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3491,9 +3478,6 @@ static ssize_t show_rotate(struct device *device,
struct fb_info *info;
int rotate = 0, idx;
- if (fbcon_has_exited)
- return 0;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3514,9 +3498,6 @@ static ssize_t show_cursor_blink(struct device *device,
struct fbcon_ops *ops;
int idx, blink = -1;
- if (fbcon_has_exited)
- return 0;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3543,9 +3524,6 @@ static ssize_t store_cursor_blink(struct device *device,
int blink, idx;
char **last = NULL;
- if (fbcon_has_exited)
- return count;
-
console_lock();
idx = con2fb_map[fg_console];
@@ -3668,9 +3646,6 @@ static void fbcon_exit(void)
struct fb_info *info;
int i, j, mapped;
- if (fbcon_has_exited)
- return;
-
#ifdef CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER
if (deferred_takeover) {
dummycon_unregister_output_notifier(&fbcon_output_nb);
@@ -3695,7 +3670,7 @@ static void fbcon_exit(void)
for (j = first_fb_vc; j <= last_fb_vc; j++) {
if (con2fb_map[j] == i) {
mapped = 1;
- break;
+ con2fb_map[j] = -1;
}
}
@@ -3718,8 +3693,6 @@ static void fbcon_exit(void)
info->queue.func = NULL;
}
}
-
- fbcon_has_exited = 1;
}
void __init fb_console_init(void)
--
2.20.1
Hi Daniel,
On 5/21/19 4:23 PM, Daniel Vetter wrote:
> This is unused code since
>
> commit 6104c37094e729f3d4ce65797002112735d49cd1
> Author: Daniel Vetter <[email protected]>
> Date: Tue Aug 1 17:32:07 2017 +0200
>
> fbcon: Make fbcon a built-time depency for fbdev
>
> when fbcon was made a compile-time static dependency of fbdev. We
> can't exit fbcon anymore without exiting fbdev first, which only works
> if all fbdev drivers have unloaded already. Hence this is all dead
> code.
>
> v2: I missed that fbcon_exit is also called from con_deinit stuff, and
> there fbcon_has_exited prevents double-cleanup. But we can fix that
> by properly resetting con2fb_map[] to all -1, which is used everywhere
> else to indicate "no fb_info allocate to this console". With that
> change the double-cleanup (which resulted in a module refcount underflow,
> among other things) is prevented.
>
> Aside: con2fb_map is a signed char, so don't register more than 128 fb_info
> or hilarity will ensue.
>
> v3: CI showed me that I still didn't fully understand what's going on
> here. The leaked references in con2fb_map have been used upon
> rebinding the fb console in fbcon_init. It worked because fbdev
> unregistering still cleaned out con2fb_map, and reset it to info_idx.
> If the last fbdev driver unregistered, then it also reset info_idx,
> and unregistered the fbcon driver.
>
> Imo that's all a bit fragile, so let's keep the con2fb_map reset to
> -1, and in fbcon_init pick info_idx if we're starting fresh. That
> means unbinding and rebinding will cleanse the mapping, but why are
> you doing that if you want to retain the mapping, so should be fine.
>
> Also, I think info_idx == -1 is impossible in fbcon_init - we
> unregister the fbcon in that case. So catch&warn about that.
>
> Signed-off-by: Daniel Vetter <[email protected]>
> Cc: Maarten Lankhorst <[email protected]>
> Cc: Bartlomiej Zolnierkiewicz <[email protected]>
> Cc: Daniel Vetter <[email protected]>
> Cc: Hans de Goede <[email protected]>
> Cc: "Noralf Trønnes" <[email protected]>
> Cc: Yisheng Xie <[email protected]>
> Cc: Konstantin Khorenko <[email protected]>
> Cc: Prarit Bhargava <[email protected]>
> Cc: Kees Cook <[email protected]>
> ---
> drivers/video/fbdev/core/fbcon.c | 39 +++++---------------------------
> 1 file changed, 6 insertions(+), 33 deletions(-)
This patch was #09/33 in your patch series, now it is independent change.
Do you want me to apply it now or should I wait for the new version of
the whole series?
[ I looked at all patches in the series and they look fine to me.
After outstanding issues are fixed I'll be happy to apply them all
to fbdev-for-next (I can create immutable branch if needed). ]
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On Wed, May 22, 2019 at 12:04 PM Bartlomiej Zolnierkiewicz
<[email protected]> wrote:
>
>
> Hi Daniel,
>
> On 5/21/19 4:23 PM, Daniel Vetter wrote:
> > This is unused code since
> >
> > commit 6104c37094e729f3d4ce65797002112735d49cd1
> > Author: Daniel Vetter <[email protected]>
> > Date: Tue Aug 1 17:32:07 2017 +0200
> >
> > fbcon: Make fbcon a built-time depency for fbdev
> >
> > when fbcon was made a compile-time static dependency of fbdev. We
> > can't exit fbcon anymore without exiting fbdev first, which only works
> > if all fbdev drivers have unloaded already. Hence this is all dead
> > code.
> >
> > v2: I missed that fbcon_exit is also called from con_deinit stuff, and
> > there fbcon_has_exited prevents double-cleanup. But we can fix that
> > by properly resetting con2fb_map[] to all -1, which is used everywhere
> > else to indicate "no fb_info allocate to this console". With that
> > change the double-cleanup (which resulted in a module refcount underflow,
> > among other things) is prevented.
> >
> > Aside: con2fb_map is a signed char, so don't register more than 128 fb_info
> > or hilarity will ensue.
> >
> > v3: CI showed me that I still didn't fully understand what's going on
> > here. The leaked references in con2fb_map have been used upon
> > rebinding the fb console in fbcon_init. It worked because fbdev
> > unregistering still cleaned out con2fb_map, and reset it to info_idx.
> > If the last fbdev driver unregistered, then it also reset info_idx,
> > and unregistered the fbcon driver.
> >
> > Imo that's all a bit fragile, so let's keep the con2fb_map reset to
> > -1, and in fbcon_init pick info_idx if we're starting fresh. That
> > means unbinding and rebinding will cleanse the mapping, but why are
> > you doing that if you want to retain the mapping, so should be fine.
> >
> > Also, I think info_idx == -1 is impossible in fbcon_init - we
> > unregister the fbcon in that case. So catch&warn about that.
> >
> > Signed-off-by: Daniel Vetter <[email protected]>
> > Cc: Maarten Lankhorst <[email protected]>
> > Cc: Bartlomiej Zolnierkiewicz <[email protected]>
> > Cc: Daniel Vetter <[email protected]>
> > Cc: Hans de Goede <[email protected]>
> > Cc: "Noralf Trønnes" <[email protected]>
> > Cc: Yisheng Xie <[email protected]>
> > Cc: Konstantin Khorenko <[email protected]>
> > Cc: Prarit Bhargava <[email protected]>
> > Cc: Kees Cook <[email protected]>
> > ---
> > drivers/video/fbdev/core/fbcon.c | 39 +++++---------------------------
> > 1 file changed, 6 insertions(+), 33 deletions(-)
> This patch was #09/33 in your patch series, now it is independent change.
>
> Do you want me to apply it now or should I wait for the new version of
> the whole series?
It's an in-reply-to replacement for the totally broken one, so that
patchwork picks things up correctly (and therefore our CI). I'm not
sure how far that convention is used beyond dri-devel ...
I did fix up all the issues pointed out thus far, but I haven't fully
appeased our CI just yet. Once that's done I'll resend.
Thanks, Daniel
> [ I looked at all patches in the series and they look fine to me.
> After outstanding issues are fixed I'll be happy to apply them all
> to fbdev-for-next (I can create immutable branch if needed). ]
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch