2008-10-25 19:58:54

by Marcin Ślusarz

[permalink] [raw]
Subject: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

Add support for persistent console history, surviving
console switches. It allocates new scrollback buffer only when
user switches console for the first time.

Signed-off-by: Marcin Slusarz <[email protected]>
Cc: Antonino Daplas <[email protected]>
Cc: Krzysztof Helt <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: [email protected]
---
drivers/video/console/Kconfig | 11 ++++++
drivers/video/console/vgacon.c | 75 +++++++++++++++++++++++++++++++++++++--
2 files changed, 82 insertions(+), 4 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 2f50a80..09e3d98 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -43,6 +43,17 @@ config VGACON_SOFT_SCROLLBACK_SIZE
buffer. Each 64KB will give you approximately 16 80x25
screenfuls of scrollback buffer

+config VGACON_REMEMBER_SCROLLBACK
+ bool "Remember scrollback buffer on console switch"
+ depends on VGACON_SOFT_SCROLLBACK
+ default y
+ help
+ Say 'Y' here if you want the scrollback buffer to be remembered
+ on console switch and restored when you switch back.
+
+ Note: every VGA console will use its own buffer, but it will be
+ allocated only when you switch to this console for the first time.
+
config MDA_CONSOLE
depends on !M68K && !PARISC && ISA
tristate "MDA text console (dual-headed) (EXPERIMENTAL)"
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 448d209..15ee7e1 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -174,9 +174,11 @@ static int vgacon_scrollback_cur;
static int vgacon_scrollback_save;
static int vgacon_scrollback_restore;

+#define SCROLLBACK_SIZE (CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024)
+
static void vgacon_scrollback_init(int pitch)
{
- int rows = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024/pitch;
+ int rows = SCROLLBACK_SIZE / pitch;

if (vgacon_scrollback) {
vgacon_scrollback_cnt = 0;
@@ -187,15 +189,76 @@ static void vgacon_scrollback_init(int pitch)
}
}

+#ifdef CONFIG_VGACON_REMEMBER_SCROLLBACK
+static struct vgacon_scrollback_info {
+ void *data;
+ int cnt;
+ int tail;
+ int cur;
+ int rows;
+ int size;
+} vgacon_scrollbacks[MAX_NR_CONSOLES];
+static int vgacon_last_vc_num;
+
+static void vgacon_switch_scrollback(struct vc_data *c)
+{
+ int num = c->vc_num;
+ struct vgacon_scrollback_info *old_scrollback =
+ &vgacon_scrollbacks[vgacon_last_vc_num];
+ struct vgacon_scrollback_info *new_scrollback =
+ &vgacon_scrollbacks[num];
+
+ old_scrollback->cnt = vgacon_scrollback_cnt;
+ old_scrollback->tail = vgacon_scrollback_tail;
+ old_scrollback->cur = vgacon_scrollback_cur;
+ old_scrollback->rows = vgacon_scrollback_rows;
+ old_scrollback->size = vgacon_scrollback_size;
+
+ if (!new_scrollback->data) {
+ int rows = SCROLLBACK_SIZE / c->vc_size_row;
+
+ new_scrollback->data = kmalloc(SCROLLBACK_SIZE, GFP_KERNEL);
+ new_scrollback->cnt = 0;
+ new_scrollback->tail = 0;
+ new_scrollback->cur = 0;
+ new_scrollback->rows = rows - 1;
+ new_scrollback->size = rows * c->vc_size_row;
+
+ if (!new_scrollback->data) {
+ printk(KERN_WARNING "VGAcon: failed to allocate memory for scrollback of console %d, using scrollback of console %d.\n",
+ num, vgacon_last_vc_num);
+ new_scrollback->data = old_scrollback->data;
+ old_scrollback->data = NULL;
+ }
+ }
+
+ vgacon_scrollback = new_scrollback->data;
+ vgacon_scrollback_cnt = new_scrollback->cnt;
+ vgacon_scrollback_tail = new_scrollback->tail;
+ vgacon_scrollback_cur = new_scrollback->cur;
+ vgacon_scrollback_rows = new_scrollback->rows;
+ vgacon_scrollback_size = new_scrollback->size;
+
+ vgacon_last_vc_num = num;
+}
+#else
+static inline void vgacon_switch_scrollback(struct vc_data *c)
+{
+ vgacon_scrollback_init(c->vc_size_row);
+}
+#endif
/*
* Called only duing init so call of alloc_bootmen is ok.
* Marked __init_refok to silence modpost.
*/
static void __init_refok vgacon_scrollback_startup(void)
{
- vgacon_scrollback = alloc_bootmem(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE
- * 1024);
+ vgacon_scrollback = alloc_bootmem(SCROLLBACK_SIZE);
vgacon_scrollback_init(vga_video_num_columns * 2);
+#ifdef CONFIG_VGACON_REMEMBER_SCROLLBACK
+ vgacon_scrollbacks[0].data = vgacon_scrollback;
+ vgacon_last_vc_num = 0;
+#endif
}

static void vgacon_scrollback_update(struct vc_data *c, int t, int count)
@@ -317,6 +380,10 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines)
#define vgacon_scrollback_init(...) do { } while (0)
#define vgacon_scrollback_update(...) do { } while (0)

+static inline void vgacon_switch_scrollback(struct vc_data *c)
+{
+}
+
static void vgacon_restore_screen(struct vc_data *c)
{
if (c->vc_origin != c->vc_visible_origin)
@@ -823,7 +890,7 @@ static int vgacon_switch(struct vc_data *c)
vgacon_doresize(c, c->vc_cols, c->vc_rows);
}

- vgacon_scrollback_init(c->vc_size_row);
+ vgacon_switch_scrollback(c);
return 0; /* Redrawing not needed */
}

--
1.5.6.4


2008-10-25 20:47:15

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz <[email protected]> wrote:

> Add support for persistent console history, surviving
> console switches. It allocates new scrollback buffer only when
> user switches console for the first time.
>
> Signed-off-by: Marcin Slusarz <[email protected]>
> Cc: Antonino Daplas <[email protected]>
> Cc: Krzysztof Helt <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: [email protected]
> ---
> drivers/video/console/Kconfig | 11 ++++++
> drivers/video/console/vgacon.c | 75 +++++++++++++++++++++++++++++++++++++--
> 2 files changed, 82 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index 2f50a80..09e3d98 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -43,6 +43,17 @@ config VGACON_SOFT_SCROLLBACK_SIZE
> buffer. Each 64KB will give you approximately 16 80x25
> screenfuls of scrollback buffer
>
> +config VGACON_REMEMBER_SCROLLBACK
> + bool "Remember scrollback buffer on console switch"
> + depends on VGACON_SOFT_SCROLLBACK
> + default y
> + help
> + Say 'Y' here if you want the scrollback buffer to be remembered
> + on console switch and restored when you switch back.
> +
> + Note: every VGA console will use its own buffer, but it will be
> + allocated only when you switch to this console for the first time.

I'd question the value in adding the config option. Why not make the
feature unconditionally present?

>
> +#define SCROLLBACK_SIZE (CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024)
> +
> ...
>
> +#ifdef CONFIG_VGACON_REMEMBER_SCROLLBACK
> +static struct vgacon_scrollback_info {
> + void *data;
> + int cnt;
> + int tail;
> + int cur;
> + int rows;
> + int size;
> +} vgacon_scrollbacks[MAX_NR_CONSOLES];

Perhaps you were concerned about memory consumption?

If so, it would be much much better to make this feature switchable at
runtime (module parameter/boot option or a /proc or /sys knob).

> +static int vgacon_last_vc_num;

We have lots of global state here with no apparent locking protecting
it. Possibly there's some higher-level lock which provides
seralisation? If so, the addition of a comment explaining all
this would be good.

2008-10-25 22:43:40

by Marcin Ślusarz

[permalink] [raw]
Subject: Re: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

On Sat, Oct 25, 2008 at 01:46:15PM -0700, Andrew Morton wrote:
> On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz <[email protected]> wrote:
>
> > Add support for persistent console history, surviving
> > console switches. It allocates new scrollback buffer only when
> > user switches console for the first time.
> >
> > Signed-off-by: Marcin Slusarz <[email protected]>
> > Cc: Antonino Daplas <[email protected]>
> > Cc: Krzysztof Helt <[email protected]>
> > Cc: Andrew Morton <[email protected]>
> > Cc: [email protected]
> > ---
> > drivers/video/console/Kconfig | 11 ++++++
> > drivers/video/console/vgacon.c | 75 +++++++++++++++++++++++++++++++++++++--
> > 2 files changed, 82 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> > index 2f50a80..09e3d98 100644
> > --- a/drivers/video/console/Kconfig
> > +++ b/drivers/video/console/Kconfig
> > @@ -43,6 +43,17 @@ config VGACON_SOFT_SCROLLBACK_SIZE
> > buffer. Each 64KB will give you approximately 16 80x25
> > screenfuls of scrollback buffer
> >
> > +config VGACON_REMEMBER_SCROLLBACK
> > + bool "Remember scrollback buffer on console switch"
> > + depends on VGACON_SOFT_SCROLLBACK
> > + default y
> > + help
> > + Say 'Y' here if you want the scrollback buffer to be remembered
> > + on console switch and restored when you switch back.
> > +
> > + Note: every VGA console will use its own buffer, but it will be
> > + allocated only when you switch to this console for the first time.
>
> I'd question the value in adding the config option. Why not make the
> feature unconditionally present?
>
> >
> > +#define SCROLLBACK_SIZE (CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024)
> > +
> > ...
> >
> > +#ifdef CONFIG_VGACON_REMEMBER_SCROLLBACK
> > +static struct vgacon_scrollback_info {
> > + void *data;
> > + int cnt;
> > + int tail;
> > + int cur;
> > + int rows;
> > + int size;
> > +} vgacon_scrollbacks[MAX_NR_CONSOLES];
>
> Perhaps you were concerned about memory consumption?

Yes. I could imagine scenario where this memory would be considered "wasted".

> If so, it would be much much better to make this feature switchable at
> runtime (module parameter/boot option or a /proc or /sys knob).

/sys knob seems to be the most flexible option.

/sys/class/vtconsole/vtconX/persistent_history? 0/1

> > +static int vgacon_last_vc_num;
>
> We have lots of global state here with no apparent locking protecting
> it. Possibly there's some higher-level lock which provides
> seralisation? If so, the addition of a comment explaining all
> this would be good.

I checked it and this code is called under console_sem.

vgacon_switch_scrollback <- vgacon_switch <- con_switch <- redraw_screen <- switch_screen <- complete_change_console <-
1) vt_ioctl (calls acquire_console_sem before complete_change_console)
2) change_console <- console_callback (calls acquire_console_sem before change_console)

Thanks for a review!

PS: why DECLARE_MUTEX _defines_ _semaphore_? there are only 8 uses of this
macro so it's not a big problem to rename it to e.g. DEFINE_SEMAPHORE (I can
provide a patch)

Marcin

2008-10-25 23:15:37

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

On Sun, 26 Oct 2008 00:43:01 +0200 Marcin Slusarz <[email protected]> wrote:

> PS: why DECLARE_MUTEX _defines_ _semaphore_?

The kernel gets definition-vs-declaration confused in several places.

> there are only 8 uses of this
> macro so it's not a big problem to rename it to e.g. DEFINE_SEMAPHORE (I can
> provide a patch)

I'd say that s/declare/define/ at such a late stage in the semaphore's
lifetime would be of dubious value. But getting "MUTEX" out of that
macro's name would be a very good thing - it's a bad overlap with struct
mutex. Send patch :)

2008-10-30 14:52:01

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

Hi!

> > If so, it would be much much better to make this feature switchable at
> > runtime (module parameter/boot option or a /proc or /sys knob).
>
> /sys knob seems to be the most flexible option.
>
> /sys/class/vtconsole/vtconX/persistent_history? 0/1

0/number-of-lines-to-remember?
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html