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