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

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

[Marcin Slusarz]

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 <marcin.slusarz@gmail.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Krzysztof Helt <krzysztof.h1@wp.pl>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-fbdev-devel@lists.sourceforge.net
---
 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

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

[Andrew Morton]

On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz <marcin.slusarz@gmail.com> 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 <marcin.slusarz@gmail.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Krzysztof Helt <krzysztof.h1@wp.pl>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: linux-fbdev-devel@lists.sourceforge.net
> ---
>  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.

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

[Marcin Slusarz]

On Sat, Oct 25, 2008 at 01:46:15PM -0700, Andrew Morton wrote:
> On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz <marcin.slusarz@gmail.com> 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 <marcin.slusarz@gmail.com>
> > Cc: Antonino Daplas <adaplas@gmail.com>
> > Cc: Krzysztof Helt <krzysztof.h1@wp.pl>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: linux-fbdev-devel@lists.sourceforge.net
> > ---
> >  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

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

[Andrew Morton]

On Sun, 26 Oct 2008 00:43:01 +0200 Marcin Slusarz <marcin.slusarz@gmail.com> 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 :)

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

[Pavel Machek]

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