vgacon: remember scrollback buffer on console switch

Started by Marcin Slusarz on Oct 25, 2008 · 5 messages from 3 authors · Last reply Oct 30, 2008

Subsystems (3)

Maintainers (3)

STALE6589d

open the first message on its own page

Message details and related content
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: <redacted>
---
 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
Message details and related content
On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz [off-list ref] wrote:
quoted hunk
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: <redacted>
---
 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.
Message details and related content
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: <redacted>
---
 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)

14 more quoted lines

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
Message details and related content
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 :)
Message details and related content
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
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help