[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 <redacted>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Krzysztof Helt <redacted>
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

On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz [off-list ref] wrote:

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.

On Sat, Oct 25, 2008 at 01:46:15PM -0700, Andrew Morton wrote:

On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz [off-list ref] wrote:

quoted

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 <redacted>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Krzysztof Helt <redacted>
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?

quoted

 
+#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

quoted

+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

On Sun, 26 Oct 2008 00:43:01 +0200 Marcin Slusarz [off-list ref] 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 :)

Hi!

quoted

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