LinuxLists.cc - [PATCH v5 0/2] console: Add persistent scrollback buffers for all VGA console

[permalink] [raw]

Subject: [PATCH v5 1/2] console: Move scrollback data into its own struct

This refactoring is in preparation for persistent scrollback
support for VGA console.

Signed-off-by: Manuel Schölling <[email protected]>
---
drivers/video/console/vgacon.c | 91 ++++++++++++++++++++++--------------------
1 file changed, 47 insertions(+), 44 deletions(-)

diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index c22a562..48b9764 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -162,31 +162,34 @@ static inline void vga_set_mem_top(struct vc_data *c)

#ifdef CONFIG_VGACON_SOFT_SCROLLBACK
/* software scrollback */
-static void *vgacon_scrollback;
-static int vgacon_scrollback_tail;
-static int vgacon_scrollback_size;
-static int vgacon_scrollback_rows;
-static int vgacon_scrollback_cnt;
-static int vgacon_scrollback_cur;
-static int vgacon_scrollback_save;
-static int vgacon_scrollback_restore;
+static struct vgacon_scrollback_info {
+ void *data;
+ int tail;
+ int size;
+ int rows;
+ int cnt;
+ int cur;
+ int save;
+ int restore;
+} vgacon_scrollback;

static void vgacon_scrollback_init(int pitch)
{
int rows = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024/pitch;

- if (vgacon_scrollback) {
- vgacon_scrollback_cnt = 0;
- vgacon_scrollback_tail = 0;
- vgacon_scrollback_cur = 0;
- vgacon_scrollback_rows = rows - 1;
- vgacon_scrollback_size = rows * pitch;
+ if (vgacon_scrollback.data) {
+ vgacon_scrollback.cnt = 0;
+ vgacon_scrollback.tail = 0;
+ vgacon_scrollback.cur = 0;
+ vgacon_scrollback.rows = rows - 1;
+ vgacon_scrollback.size = rows * pitch;
}
}

static void vgacon_scrollback_startup(void)
{
- vgacon_scrollback = kcalloc(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE, 1024, GFP_NOWAIT);
+ vgacon_scrollback.data = kcalloc(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE,
+ 1024, GFP_NOWAIT);
vgacon_scrollback_init(vga_video_num_columns * 2);
}

@@ -194,38 +197,38 @@ static void vgacon_scrollback_update(struct vc_data *c, int t, int count)
{
void *p;

- if (!vgacon_scrollback_size || c->vc_num != fg_console)
+ if (!vgacon_scrollback.size || c->vc_num != fg_console)
return;

p = (void *) (c->vc_origin + t * c->vc_size_row);

while (count--) {
- scr_memcpyw(vgacon_scrollback + vgacon_scrollback_tail,
+ scr_memcpyw(vgacon_scrollback.data + vgacon_scrollback.tail,
p, c->vc_size_row);
- vgacon_scrollback_cnt++;
+ vgacon_scrollback.cnt++;
p += c->vc_size_row;
- vgacon_scrollback_tail += c->vc_size_row;
+ vgacon_scrollback.tail += c->vc_size_row;

- if (vgacon_scrollback_tail >= vgacon_scrollback_size)
- vgacon_scrollback_tail = 0;
+ if (vgacon_scrollback.tail >= vgacon_scrollback.size)
+ vgacon_scrollback.tail = 0;

- if (vgacon_scrollback_cnt > vgacon_scrollback_rows)
- vgacon_scrollback_cnt = vgacon_scrollback_rows;
+ if (vgacon_scrollback.cnt > vgacon_scrollback.rows)
+ vgacon_scrollback.cnt = vgacon_scrollback.rows;

- vgacon_scrollback_cur = vgacon_scrollback_cnt;
+ vgacon_scrollback.cur = vgacon_scrollback.cnt;
}
}

static void vgacon_restore_screen(struct vc_data *c)
{
- vgacon_scrollback_save = 0;
+ vgacon_scrollback.save = 0;

- if (!vga_is_gfx && !vgacon_scrollback_restore) {
+ if (!vga_is_gfx && !vgacon_scrollback.restore) {
scr_memcpyw((u16 *) c->vc_origin, (u16 *) c->vc_screenbuf,
c->vc_screenbuf_size > vga_vram_size ?
vga_vram_size : c->vc_screenbuf_size);
- vgacon_scrollback_restore = 1;
- vgacon_scrollback_cur = vgacon_scrollback_cnt;
+ vgacon_scrollback.restore = 1;
+ vgacon_scrollback.cur = vgacon_scrollback.cnt;
}
}

@@ -239,41 +242,41 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)
return;
}

- if (!vgacon_scrollback)
+ if (!vgacon_scrollback.data)
return;

- if (!vgacon_scrollback_save) {
+ if (!vgacon_scrollback.save) {
vgacon_cursor(c, CM_ERASE);
vgacon_save_screen(c);
- vgacon_scrollback_save = 1;
+ vgacon_scrollback.save = 1;
}

- vgacon_scrollback_restore = 0;
- start = vgacon_scrollback_cur + lines;
+ vgacon_scrollback.restore = 0;
+ start = vgacon_scrollback.cur + lines;
end = start + abs(lines);

if (start < 0)
start = 0;

- if (start > vgacon_scrollback_cnt)
- start = vgacon_scrollback_cnt;
+ if (start > vgacon_scrollback.cnt)
+ start = vgacon_scrollback.cnt;

if (end < 0)
end = 0;

- if (end > vgacon_scrollback_cnt)
- end = vgacon_scrollback_cnt;
+ if (end > vgacon_scrollback.cnt)
+ end = vgacon_scrollback.cnt;

- vgacon_scrollback_cur = start;
+ vgacon_scrollback.cur = start;
count = end - start;
- soff = vgacon_scrollback_tail - ((vgacon_scrollback_cnt - end) *
+ soff = vgacon_scrollback.tail - ((vgacon_scrollback.cnt - end) *
c->vc_size_row);
soff -= count * c->vc_size_row;

if (soff < 0)
- soff += vgacon_scrollback_size;
+ soff += vgacon_scrollback.size;

- count = vgacon_scrollback_cnt - start;
+ count = vgacon_scrollback.cnt - start;

if (count > c->vc_rows)
count = c->vc_rows;
@@ -287,13 +290,13 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)

count *= c->vc_size_row;
/* how much memory to end of buffer left? */
- copysize = min(count, vgacon_scrollback_size - soff);
- scr_memcpyw(d, vgacon_scrollback + soff, copysize);
+ copysize = min(count, vgacon_scrollback.size - soff);
+ scr_memcpyw(d, vgacon_scrollback.data + soff, copysize);
d += copysize;
count -= copysize;

if (count) {
- scr_memcpyw(d, vgacon_scrollback, count);
+ scr_memcpyw(d, vgacon_scrollback.data, count);
d += count;
}

--
2.1.4

2016-11-20 21:59:02

[permalink] [raw]

Subject: [PATCH v5 2/2] console: Add persistent scrollback buffers for all VGA consoles

Add a scrollback buffers for each VGA console. The benefit is that
the scrollback history is not flushed when switching between consoles
but is persistent.
The buffers are allocated on demand when a new console is opened.

This breaks tools like clear_console that rely on flushing the
scrollback history by switching back and forth between consoles
which is why this feature is disabled by default.
Use the escape sequence \e[3J instead for flushing the buffer.

Signed-off-by: Manuel Schölling <[email protected]>
---
drivers/video/console/Kconfig | 25 +++++++-
drivers/video/console/vgacon.c | 131 +++++++++++++++++++++++++++--------------
2 files changed, 108 insertions(+), 48 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 38da6e2..c5742d2 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -43,9 +43,28 @@ config VGACON_SOFT_SCROLLBACK_SIZE
range 1 1024
default "64"
help
- Enter the amount of System RAM to allocate for the scrollback
- buffer. Each 64KB will give you approximately 16 80x25
- screenfuls of scrollback buffer
+ Enter the amount of System RAM to allocate for scrollback
+ buffers of VGA consoles. Each 64KB will give you approximately
+ 16 80x25 screenfuls of scrollback buffer.
+
+config VGACON_SOFT_SCROLLBACK_PERSISTENT
+ bool "Persistent Scrollback History for each console"
+ depends on VGACON_SOFT_SCROLLBACK
+ default n
+ help
+ Say Y here if the scrollback history should persist when switching
+ between consoles. Otherwise, the scrollback history will be flushed
+ each time the console is switched.
+
+ This feature might break your tool of choice to flush the scrollback
+ buffer, e.g. clear(1) will work fine but Debian's clear_console(1)
+ will be broken, which might cause security issues.
+ You can use the escape sequence \e[3J instead if this feature is
+ activated.
+
+ Note that a buffer of VGACON_SOFT_SCROLLBACK_SIZE is taken for each
+ created tty device.
+ So if you use a RAM-constrained system, say N here.

config MDA_CONSOLE
depends on !M68K && !PARISC && ISA
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 48b9764..4d7845a 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -162,7 +162,7 @@ static inline void vga_set_mem_top(struct vc_data *c)

#ifdef CONFIG_VGACON_SOFT_SCROLLBACK
/* software scrollback */
-static struct vgacon_scrollback_info {
+struct vgacon_scrollback_info {
void *data;
int tail;
int size;
@@ -171,64 +171,104 @@ static struct vgacon_scrollback_info {
int cur;
int save;
int restore;
-} vgacon_scrollback;
+};
+static struct vgacon_scrollback_info *vgacon_scrollback_cur;
+#ifdef CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT
+static struct vgacon_scrollback_info vgacon_scrollbacks[MAX_NR_CONSOLES];
+#else
+static struct vgacon_scrollback_info vgacon_scrollbacks[1];
+#endif
+
+static void vgacon_scrollback_reset(size_t reset_size)
+{
+ if (vgacon_scrollback_cur->data && reset_size > 0)
+ memset(vgacon_scrollback_cur->data, 0, reset_size);

-static void vgacon_scrollback_init(int pitch)
+ vgacon_scrollback_cur->cnt = 0;
+ vgacon_scrollback_cur->tail = 0;
+ vgacon_scrollback_cur->cur = 0;
+}
+
+static void vgacon_scrollback_init(int vc_num)
{
- int rows = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024/pitch;
-
- if (vgacon_scrollback.data) {
- vgacon_scrollback.cnt = 0;
- vgacon_scrollback.tail = 0;
- vgacon_scrollback.cur = 0;
- vgacon_scrollback.rows = rows - 1;
- vgacon_scrollback.size = rows * pitch;
+ int pitch = vga_video_num_columns * 2;
+ size_t size = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024;
+ int rows = size/pitch;
+ void *data;
+
+ data = kmalloc_array(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE, 1024,
+ GFP_NOWAIT);
+
+ vgacon_scrollbacks[vc_num].data = data;
+ vgacon_scrollback_cur = &vgacon_scrollbacks[vc_num];
+
+ vgacon_scrollback_cur->rows = rows - 1;
+ vgacon_scrollback_cur->size = rows * pitch;
+
+ vgacon_scrollback_reset(size);
+}
+
+static void vgacon_scrollback_switch(int vc_num)
+{
+ vc_num = CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT ? vc_num : 0;
+
+ if (!vgacon_scrollbacks[vc_num].data)
+ vgacon_scrollback_init(vc_num);
+ else {
+#ifdef CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT
+ vgacon_scrollback_cur = &vgacon_scrollbacks[vc_num];
+#else
+ size_t size = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024;
+ vgacon_scrollback_reset(size);
+#endif
}
}

static void vgacon_scrollback_startup(void)
{
- vgacon_scrollback.data = kcalloc(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE,
- 1024, GFP_NOWAIT);
- vgacon_scrollback_init(vga_video_num_columns * 2);
+ vgacon_scrollback_cur = &vgacon_scrollbacks[0];
+ vgacon_scrollback_init(0);
}

static void vgacon_scrollback_update(struct vc_data *c, int t, int count)
{
void *p;

- if (!vgacon_scrollback.size || c->vc_num != fg_console)
+ if (!vgacon_scrollback_cur->data || !vgacon_scrollback_cur->size
+ || c->vc_num != fg_console)
return;

p = (void *) (c->vc_origin + t * c->vc_size_row);

while (count--) {
- scr_memcpyw(vgacon_scrollback.data + vgacon_scrollback.tail,
+ scr_memcpyw(vgacon_scrollback_cur->data +
+ vgacon_scrollback_cur->tail,
p, c->vc_size_row);
- vgacon_scrollback.cnt++;
+
+ vgacon_scrollback_cur->cnt++;
p += c->vc_size_row;
- vgacon_scrollback.tail += c->vc_size_row;
+ vgacon_scrollback_cur->tail += c->vc_size_row;

- if (vgacon_scrollback.tail >= vgacon_scrollback.size)
- vgacon_scrollback.tail = 0;
+ if (vgacon_scrollback_cur->tail >= vgacon_scrollback_cur->size)
+ vgacon_scrollback_cur->tail = 0;

- if (vgacon_scrollback.cnt > vgacon_scrollback.rows)
- vgacon_scrollback.cnt = vgacon_scrollback.rows;
+ if (vgacon_scrollback_cur->cnt > vgacon_scrollback_cur->rows)
+ vgacon_scrollback_cur->cnt = vgacon_scrollback_cur->rows;

- vgacon_scrollback.cur = vgacon_scrollback.cnt;
+ vgacon_scrollback_cur->cur = vgacon_scrollback_cur->cnt;
}
}

static void vgacon_restore_screen(struct vc_data *c)
{
- vgacon_scrollback.save = 0;
+ vgacon_scrollback_cur->save = 0;

- if (!vga_is_gfx && !vgacon_scrollback.restore) {
+ if (!vga_is_gfx && !vgacon_scrollback_cur->restore) {
scr_memcpyw((u16 *) c->vc_origin, (u16 *) c->vc_screenbuf,
c->vc_screenbuf_size > vga_vram_size ?
vga_vram_size : c->vc_screenbuf_size);
- vgacon_scrollback.restore = 1;
- vgacon_scrollback.cur = vgacon_scrollback.cnt;
+ vgacon_scrollback_cur->restore = 1;
+ vgacon_scrollback_cur->cur = vgacon_scrollback_cur->cnt;
}
}

@@ -242,41 +282,41 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)
return;
}

- if (!vgacon_scrollback.data)
+ if (!vgacon_scrollback_cur->data)
return;

- if (!vgacon_scrollback.save) {
+ if (!vgacon_scrollback_cur->save) {
vgacon_cursor(c, CM_ERASE);
vgacon_save_screen(c);
- vgacon_scrollback.save = 1;
+ vgacon_scrollback_cur->save = 1;
}

- vgacon_scrollback.restore = 0;
- start = vgacon_scrollback.cur + lines;
+ vgacon_scrollback_cur->restore = 0;
+ start = vgacon_scrollback_cur->cur + lines;
end = start + abs(lines);

if (start < 0)
start = 0;

- if (start > vgacon_scrollback.cnt)
- start = vgacon_scrollback.cnt;
+ if (start > vgacon_scrollback_cur->cnt)
+ start = vgacon_scrollback_cur->cnt;

if (end < 0)
end = 0;

- if (end > vgacon_scrollback.cnt)
- end = vgacon_scrollback.cnt;
+ if (end > vgacon_scrollback_cur->cnt)
+ end = vgacon_scrollback_cur->cnt;

- vgacon_scrollback.cur = start;
+ vgacon_scrollback_cur->cur = start;
count = end - start;
- soff = vgacon_scrollback.tail - ((vgacon_scrollback.cnt - end) *
- c->vc_size_row);
+ soff = vgacon_scrollback_cur->tail -
+ (c->vc_size_row * (vgacon_scrollback_cur->cnt - end));
soff -= count * c->vc_size_row;

if (soff < 0)
- soff += vgacon_scrollback.size;
+ soff += vgacon_scrollback_cur->size;

- count = vgacon_scrollback.cnt - start;
+ count = vgacon_scrollback_cur->cnt - start;

if (count > c->vc_rows)
count = c->vc_rows;
@@ -290,13 +330,13 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)

count *= c->vc_size_row;
/* how much memory to end of buffer left? */
- copysize = min(count, vgacon_scrollback.size - soff);
- scr_memcpyw(d, vgacon_scrollback.data + soff, copysize);
+ copysize = min(count, vgacon_scrollback_cur->size - soff);
+ scr_memcpyw(d, vgacon_scrollback_cur->data + soff, copysize);
d += copysize;
count -= copysize;

if (count) {
- scr_memcpyw(d, vgacon_scrollback.data, count);
+ scr_memcpyw(d, vgacon_scrollback_cur->data, count);
d += count;
}

@@ -309,6 +349,7 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)
#define vgacon_scrollback_startup(...) do { } while (0)
#define vgacon_scrollback_init(...) do { } while (0)
#define vgacon_scrollback_update(...) do { } while (0)
+#define vgacon_scrollback_switch(...) do { } while (0)

static void vgacon_restore_screen(struct vc_data *c)
{
@@ -783,7 +824,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_scrollback_switch(c->vc_num);
return 0; /* Redrawing not needed */
}

--
2.1.4

2016-11-20 22:24:21

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v5 2/2] console: Add persistent scrollback buffers for all VGA consoles

Hi Manuel,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.9-rc6 next-20161117]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Manuel-Sch-lling/console-Move-scrollback-data-into-its-own-struct/20161121-060257
config: x86_64-randconfig-x018-201647 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All errors (new ones prefixed by >>):

drivers/video/console/vgacon.c: In function 'vgacon_scrollback_switch':
>> drivers/video/console/vgacon.c:232:11: error: 'CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT' undeclared (first use in this function)
vc_num = CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT ? vc_num : 0;
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/video/console/vgacon.c:232:11: note: each undeclared identifier is reported only once for each function it appears in

vim +/CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT +232 drivers/video/console/vgacon.c

226
227 vgacon_scrollback_reset(size);
228 }
229
230 static void vgacon_scrollback_switch(int vc_num)
231 {
> 232 vc_num = CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT ? vc_num : 0;
233
234 if (!vgacon_scrollbacks[vc_num].data)
235 vgacon_scrollback_init(vc_num);

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Attachments:

(No filename) (1.54 kB)
.config.gz (31.51 kB)
Download all attachments

2016-11-21 20:17:19

[permalink] [raw]

Subject: Re: [PATCH v5 2/2] console: Add persistent scrollback buffers for all VGA consoles

On Sun, Nov 20, 2016 at 10:58:08PM +0100, Manuel Sch?lling wrote:
> Add a scrollback buffers for each VGA console. The benefit is that
> the scrollback history is not flushed when switching between consoles
> but is persistent.
> The buffers are allocated on demand when a new console is opened.
>
> This breaks tools like clear_console that rely on flushing the
> scrollback history by switching back and forth between consoles
> which is why this feature is disabled by default.
> Use the escape sequence \e[3J instead for flushing the buffer.
>
> Signed-off-by: Manuel Sch?lling <[email protected]>
> ---

First, big thanks for this fix, it's something that greatly annoyed me
since forever!

The thing about clear_console is unfortunate: they abused the bug you're
fixing. I've asked to use \e[3J (https://bugs.debian.org/845177) so there's
hope it'll be applied in stretch; with Debian configuring its glibc to
support only kernels from two releases before (in jessie that's 2.6.32, in
stretch 3.2)[1] there's hope we can flip the default in several years.

Do you suspect any other program relies on VT switch to clear the
scrollback?

But alas, this commit breaks that very \e[3J. It does only a \e[2J, leaving
the scrollback uncleared. For comparison, both mainline and with just your
preparatory commit, \e[3J works as expected.

Meow!

[1]. Well, the dependency goes the other way, but suggestions I'm
intentionally making this error to push an agenda are evil lies. :p
--
A true bird-watcher waves his tail while doing so.

2016-11-22 16:56:59

[permalink] [raw]

Subject: Re: [PATCH v5 2/2] console: Add persistent scrollback buffers for all VGA consoles

Hi Adam,

On Mo, 2016-11-21 at 21:17 +0100, Adam Borowski wrote:
> On Sun, Nov 20, 2016 at 10:58:08PM +0100, Manuel Schölling wrote:
> > Add a scrollback buffers for each VGA console. The benefit is that
> > the scrollback history is not flushed when switching between consoles
> > but is persistent.
> > The buffers are allocated on demand when a new console is opened.
> >
> > This breaks tools like clear_console that rely on flushing the
> > scrollback history by switching back and forth between consoles
> > which is why this feature is disabled by default.
> > Use the escape sequence \e[3J instead for flushing the buffer.

> First, big thanks for this fix, it's something that greatly annoyed me
> since forever!
Yeah, me too! ;)

> The thing about clear_console is unfortunate: they abused the bug you're
> fixing. I've asked to use \e[3J (https://bugs.debian.org/845177) so there's
> hope it'll be applied in stretch; with Debian configuring its glibc to
> support only kernels from two releases before (in jessie that's 2.6.32, in
> stretch 3.2)[1] there's hope we can flip the default in several years.
>
> Do you suspect any other program relies on VT switch to clear the
> scrollback?
Not, AFAIK. Although I do not have a complete list of programs that are
suppose to do that.

> But alas, this commit breaks that very \e[3J. It does only a \e[2J, leaving
> the scrollback uncleared. For comparison, both mainline and with just your
> preparatory commit, \e[3J works as expected.
Really? All my tests worked fine: I compiled the kernel with the latest patches, started the kernel in QEMU and then did

$ openvt /bin/sh
$ echo -e '\e[3J' # scrollback buffer was flushed correctly
$ chvt 2
$ echo -e '\e[3J' # scrollback buffer was flushed correctly

Can you tell me how you tested it? Maybe I can reproduce the bug.

Thanks for spending the time to test it!

Bye,

Manuel

2016-11-23 17:33:59

[permalink] [raw]

Subject: Re: [PATCH v5 2/2] console: Add persistent scrollback buffers for all VGA consoles

On Tue, Nov 22, 2016 at 05:56:42PM +0100, Manuel Sch?lling wrote:
> On Mo, 2016-11-21 at 21:17 +0100, Adam Borowski wrote:
> > On Sun, Nov 20, 2016 at 10:58:08PM +0100, Manuel Sch?lling wrote:
> > > Add a scrollback buffers for each VGA console. The benefit is that
> > > the scrollback history is not flushed when switching between consoles
> > > but is persistent.
>
> > But alas, this commit breaks that very \e[3J. It does only a \e[2J, leaving
> > the scrollback uncleared. For comparison, both mainline and with just your
> > preparatory commit, \e[3J works as expected.
> Really? All my tests worked fine: I compiled the kernel with the latest patches, started the kernel in QEMU and then did
>
> $ openvt /bin/sh
> $ echo -e '\e[3J' # scrollback buffer was flushed correctly
> $ chvt 2
> $ echo -e '\e[3J' # scrollback buffer was flushed correctly
>
> Can you tell me how you tested it? Maybe I can reproduce the bug.

(Re-tested on v6 of the patch.)

On bare metal: boot, log in on tty1, printf '\e[3J', screen clears but when
I scroll back, it still has bootup messages. Switching to another tty then
back obviously doesn't clear it either. Same on any other tty (after
putting something into the scrollback of). Graphics card is nvidia GT240,
neither proprietary driver nor nouveau loaded; nouveau forces fbdev and your
patch is vgacon specific (hopefully just for now).

But then, I just reproduced this on qemu (-vga qxl) too, so it might be due
to a difference between our setups somehow. In case it's something related
to .config, mine's at https://angband.pl/tmp/config-4.9.0-rc6-debug2+.xz

Meow!
--
An imaginary friend squared is a real enemy.

2016-11-23 17:34:14

[permalink] [raw]

Subject: Re: [PATCH v5 2/2] console: Add persistent scrollback buffers for all VGA consoles

On Mo, 2016-11-21 at 21:17 +0100, Adam Borowski wrote:
> On Sun, Nov 20, 2016 at 10:58:08PM +0100, Manuel Schölling wrote:
> > Add a scrollback buffers for each VGA console. The benefit is that
> > the scrollback history is not flushed when switching between consoles
> > but is persistent.
> > The buffers are allocated on demand when a new console is opened.
> >
> > This breaks tools like clear_console that rely on flushing the
> > scrollback history by switching back and forth between consoles
> > which is why this feature is disabled by default.
> > Use the escape sequence \e[3J instead for flushing the buffer.
> >
> > Signed-off-by: Manuel Schölling <[email protected]>
> > ---

> But alas, this commit breaks that very \e[3J. It does only a \e[2J, leaving
> the scrollback uncleared. For comparison, both mainline and with just your
> preparatory commit, \e[3J works as expected.
Thanks again for reporting this issue. I was finally able to reproduce
it.
Looks like the same problem arises when implementing persistent
scrollback buffers for framebuffer consoles. I will have to think about
the underlying issue a bit more, but I guess that the consw struct needs
another field for a function that flushes the scrollback buffer.
Before this was just done by switching the console, which is fine if you
just have one buffer. But now each console has its own buffer, so simply
calling vc_data's vc_sw->con_switch() won't be sufficient anymore.

2016-11-27 16:52:18

[permalink] [raw]

Subject: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

Changes in v7:
- Add new callback to consw struct for flushing video console driver's
scrollback buffer. Fixes issues with escape sequence '\e[3J' reported
by Adam Borowski ([email protected]).
- Fix style issues
Changes in v6:
- Change of check if feature is enabled in
vgacon_scrollback_switch()
Changes in v5:
- Clearify documentation
- Skip superfluous array initialization
- Disable scrollback if buffer allocation fails
- Refactor vgacon_switch_scrollback()
- Rename vgacon_switch_scrollback() to vgacon_scrollback_switch()
- Add check for fg_console in vgacon_scrollback_update
Changes in v4.1:
- Fix compiler error
Changes in v4:
- Rename from VGACON_SOFT_SCROLLBACK_FOR_EACH_CONSOLE to
VGACON_SOFT_SCROLLBACK_PERSISTENT
- Split into two patches
- Rework documentation
- Remove cosmetic changes in comments (postponed)
Changes in v3:
- Add config option for this feature
- Fallback to old scrollback buffer if kcalloc() fails
- Remove ioctl() call again and add documentation about existing
escape sequence to flush the scrollback buffer
Changes in v2:
- Add ioctl() call to flush scrollback buffer
- (Patch v2 was not labeled as such, sorry)

Manuel Schölling (3):
console: Move scrollback data into its own struct
console: Add callback to flush scrollback buffer to consw struct
console: Add persistent scrollback buffers for all VGA consoles

drivers/tty/vt/vt.c | 9 +++
drivers/video/console/Kconfig | 25 ++++++-
drivers/video/console/vgacon.c | 165 ++++++++++++++++++++++++++++-------------
include/linux/console.h | 4 +
4 files changed, 148 insertions(+), 55 deletions(-)

--
2.1.4

2016-11-27 16:52:30

[permalink] [raw]

Subject: [PATCH v7 3/3] console: Add persistent scrollback buffers for all VGA consoles

Add a scrollback buffers for each VGA console. The benefit is that
the scrollback history is not flushed when switching between consoles
but is persistent.
The buffers are allocated on demand when a new console is opened.

This breaks tools like clear_console that rely on flushing the
scrollback history by switching back and forth between consoles
which is why this feature is disabled by default.
Use the escape sequence \e[3J instead for flushing the buffer.

Signed-off-by: Manuel Schölling <[email protected]>
---
drivers/video/console/Kconfig | 25 +++++++-
drivers/video/console/vgacon.c | 142 ++++++++++++++++++++++++++---------------
2 files changed, 111 insertions(+), 56 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 38da6e2..c5742d2 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -43,9 +43,28 @@ config VGACON_SOFT_SCROLLBACK_SIZE
range 1 1024
default "64"
help
- Enter the amount of System RAM to allocate for the scrollback
- buffer. Each 64KB will give you approximately 16 80x25
- screenfuls of scrollback buffer
+ Enter the amount of System RAM to allocate for scrollback
+ buffers of VGA consoles. Each 64KB will give you approximately
+ 16 80x25 screenfuls of scrollback buffer.
+
+config VGACON_SOFT_SCROLLBACK_PERSISTENT
+ bool "Persistent Scrollback History for each console"
+ depends on VGACON_SOFT_SCROLLBACK
+ default n
+ help
+ Say Y here if the scrollback history should persist when switching
+ between consoles. Otherwise, the scrollback history will be flushed
+ each time the console is switched.
+
+ This feature might break your tool of choice to flush the scrollback
+ buffer, e.g. clear(1) will work fine but Debian's clear_console(1)
+ will be broken, which might cause security issues.
+ You can use the escape sequence \e[3J instead if this feature is
+ activated.
+
+ Note that a buffer of VGACON_SOFT_SCROLLBACK_SIZE is taken for each
+ created tty device.
+ So if you use a RAM-constrained system, say N here.

config MDA_CONSOLE
depends on !M68K && !PARISC && ISA
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 9a7c2bb..ca23d22 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -162,7 +162,7 @@ static inline void vga_set_mem_top(struct vc_data *c)

#ifdef CONFIG_VGACON_SOFT_SCROLLBACK
/* software scrollback */
-static struct vgacon_scrollback_info {
+struct vgacon_scrollback_info {
void *data;
int tail;
int size;
@@ -171,74 +171,110 @@ static struct vgacon_scrollback_info {
int cur;
int save;
int restore;
-} vgacon_scrollback;
+};
+
+static struct vgacon_scrollback_info *vgacon_scrollback_cur;
+#ifdef CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT
+static struct vgacon_scrollback_info vgacon_scrollbacks[MAX_NR_CONSOLES];
+#else
+static struct vgacon_scrollback_info vgacon_scrollbacks[1];
+#endif

-static void vgacon_scrollback_reset(size_t reset_size)
+static void vgacon_scrollback_reset(int vc_num, size_t reset_size)
{
- if (vgacon_scrollback.data && reset_size > 0)
- memset(vgacon_scrollback.data, 0, reset_size);
+ struct vgacon_scrollback_info *scrollback = &vgacon_scrollbacks[vc_num];
+
+ if (scrollback->data && reset_size > 0)
+ memset(scrollback->data, 0, reset_size);

- vgacon_scrollback.cnt = 0;
- vgacon_scrollback.tail = 0;
- vgacon_scrollback.cur = 0;
+ scrollback->cnt = 0;
+ scrollback->tail = 0;
+ scrollback->cur = 0;
}

-static void vgacon_scrollback_init(int pitch)
+static void vgacon_scrollback_init(int vc_num)
{
- int rows = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024/pitch;
-
- if (vgacon_scrollback.data) {
- vgacon_scrollback.cnt = 0;
- vgacon_scrollback.tail = 0;
- vgacon_scrollback.cur = 0;
- vgacon_scrollback.rows = rows - 1;
- vgacon_scrollback.size = rows * pitch;
+ int pitch = vga_video_num_columns * 2;
+ size_t size = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024;
+ int rows = size / pitch;
+ void *data;
+
+ data = kmalloc_array(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE, 1024,
+ GFP_NOWAIT);
+
+ vgacon_scrollbacks[vc_num].data = data;
+ vgacon_scrollback_cur = &vgacon_scrollbacks[vc_num];
+
+ vgacon_scrollback_cur->rows = rows - 1;
+ vgacon_scrollback_cur->size = rows * pitch;
+
+ vgacon_scrollback_reset(vc_num, size);
+}
+
+static void vgacon_scrollback_switch(int vc_num)
+{
+#ifndef CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT
+ vc_num = 0;
+#endif
+
+ if (!vgacon_scrollbacks[vc_num].data) {
+ vgacon_scrollback_init(vc_num);
+ } else {
+#ifdef CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT
+ vgacon_scrollback_cur = &vgacon_scrollbacks[vc_num];
+#else
+ size_t size = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024;
+
+ vgacon_scrollback_reset(vc_num, size);
+#endif
}
}

static void vgacon_scrollback_startup(void)
{
- vgacon_scrollback.data = kcalloc(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE,
- 1024, GFP_NOWAIT);
- vgacon_scrollback_init(vga_video_num_columns * 2);
+ vgacon_scrollback_cur = &vgacon_scrollbacks[0];
+ vgacon_scrollback_init(0);
}

static void vgacon_scrollback_update(struct vc_data *c, int t, int count)
{
void *p;

- if (!vgacon_scrollback.size || c->vc_num != fg_console)
+ if (!vgacon_scrollback_cur->data || !vgacon_scrollback_cur->size ||
+ c->vc_num != fg_console)
return;

p = (void *) (c->vc_origin + t * c->vc_size_row);

while (count--) {
- scr_memcpyw(vgacon_scrollback.data + vgacon_scrollback.tail,
+ scr_memcpyw(vgacon_scrollback_cur->data +
+ vgacon_scrollback_cur->tail,
p, c->vc_size_row);
- vgacon_scrollback.cnt++;
+
+ vgacon_scrollback_cur->cnt++;
p += c->vc_size_row;
- vgacon_scrollback.tail += c->vc_size_row;
+ vgacon_scrollback_cur->tail += c->vc_size_row;

- if (vgacon_scrollback.tail >= vgacon_scrollback.size)
- vgacon_scrollback.tail = 0;
+ if (vgacon_scrollback_cur->tail >= vgacon_scrollback_cur->size)
+ vgacon_scrollback_cur->tail = 0;

- if (vgacon_scrollback.cnt > vgacon_scrollback.rows)
- vgacon_scrollback.cnt = vgacon_scrollback.rows;
+ if (vgacon_scrollback_cur->cnt > vgacon_scrollback_cur->rows)
+ vgacon_scrollback_cur->cnt = vgacon_scrollback_cur->rows;

- vgacon_scrollback.cur = vgacon_scrollback.cnt;
+ vgacon_scrollback_cur->cur = vgacon_scrollback_cur->cnt;
}
}

static void vgacon_restore_screen(struct vc_data *c)
{
- vgacon_scrollback.save = 0;
+ vgacon_scrollback_cur->save = 0;

- if (!vga_is_gfx && !vgacon_scrollback.restore) {
+ if (!vga_is_gfx && !vgacon_scrollback_cur->restore) {
scr_memcpyw((u16 *) c->vc_origin, (u16 *) c->vc_screenbuf,
c->vc_screenbuf_size > vga_vram_size ?
vga_vram_size : c->vc_screenbuf_size);
- vgacon_scrollback.restore = 1;
- vgacon_scrollback.cur = vgacon_scrollback.cnt;
+ vgacon_scrollback_cur->restore = 1;
+ vgacon_scrollback_cur->cur = vgacon_scrollback_cur->cnt;
}
}

@@ -252,41 +288,41 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)
return;
}

- if (!vgacon_scrollback.data)
+ if (!vgacon_scrollback_cur->data)
return;

- if (!vgacon_scrollback.save) {
+ if (!vgacon_scrollback_cur->save) {
vgacon_cursor(c, CM_ERASE);
vgacon_save_screen(c);
- vgacon_scrollback.save = 1;
+ vgacon_scrollback_cur->save = 1;
}

- vgacon_scrollback.restore = 0;
- start = vgacon_scrollback.cur + lines;
+ vgacon_scrollback_cur->restore = 0;
+ start = vgacon_scrollback_cur->cur + lines;
end = start + abs(lines);

if (start < 0)
start = 0;

- if (start > vgacon_scrollback.cnt)
- start = vgacon_scrollback.cnt;
+ if (start > vgacon_scrollback_cur->cnt)
+ start = vgacon_scrollback_cur->cnt;

if (end < 0)
end = 0;

- if (end > vgacon_scrollback.cnt)
- end = vgacon_scrollback.cnt;
+ if (end > vgacon_scrollback_cur->cnt)
+ end = vgacon_scrollback_cur->cnt;

- vgacon_scrollback.cur = start;
+ vgacon_scrollback_cur->cur = start;
count = end - start;
- soff = vgacon_scrollback.tail - ((vgacon_scrollback.cnt - end) *
- c->vc_size_row);
+ soff = vgacon_scrollback_cur->tail -
+ ((vgacon_scrollback_cur->cnt - end) * c->vc_size_row);
soff -= count * c->vc_size_row;

if (soff < 0)
- soff += vgacon_scrollback.size;
+ soff += vgacon_scrollback_cur->size;

- count = vgacon_scrollback.cnt - start;
+ count = vgacon_scrollback_cur->cnt - start;

if (count > c->vc_rows)
count = c->vc_rows;
@@ -300,13 +336,13 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)

count *= c->vc_size_row;
/* how much memory to end of buffer left? */
- copysize = min(count, vgacon_scrollback.size - soff);
- scr_memcpyw(d, vgacon_scrollback.data + soff, copysize);
+ copysize = min(count, vgacon_scrollback_cur->size - soff);
+ scr_memcpyw(d, vgacon_scrollback_cur->data + soff, copysize);
d += copysize;
count -= copysize;

if (count) {
- scr_memcpyw(d, vgacon_scrollback.data, count);
+ scr_memcpyw(d, vgacon_scrollback_cur->data, count);
d += count;
}

@@ -320,13 +356,13 @@ static void vgacon_flush_scrollback(struct vc_data *c)
{
size_t size = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024;

- if (c->vc_num == fg_console)
- vgacon_scrollback_reset(size);
+ vgacon_scrollback_reset(c->vc_num, size);
}
#else
#define vgacon_scrollback_startup(...) do { } while (0)
#define vgacon_scrollback_init(...) do { } while (0)
#define vgacon_scrollback_update(...) do { } while (0)
+#define vgacon_scrollback_switch(...) do { } while (0)

static void vgacon_restore_screen(struct vc_data *c)
{
@@ -805,7 +841,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_scrollback_switch(c->vc_num);
return 0; /* Redrawing not needed */
}

--
2.1.4

2016-11-27 16:52:40

[permalink] [raw]

Subject: [PATCH v7 1/3] console: Move scrollback data into its own struct

2016-11-27 16:52:49

[permalink] [raw]

Subject: [PATCH v7 2/3] console: Add callback to flush scrollback buffer to consw struct

This new callback is in preparation for persistent scrollback buffer
support for VGA consoles.
With a single scrollback buffer for all consoles, we could flush the
buffer just by invocating consw->con_switch(). But when each VGA console
has its own scrollback buffer, we need a new callback to tell the
video console driver which buffer to flush.

Signed-off-by: Manuel Schölling <[email protected]>
---
drivers/tty/vt/vt.c | 9 +++++++++
drivers/video/console/vgacon.c | 24 +++++++++++++++++++++++-
include/linux/console.h | 4 ++++
3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 4c10a9d..9d3ce50 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -625,6 +625,14 @@ static void save_screen(struct vc_data *vc)
vc->vc_sw->con_save_screen(vc);
}

+static void flush_scrollback(struct vc_data *vc)
+{
+ WARN_CONSOLE_UNLOCKED();
+
+ if (vc->vc_sw->con_flush_scrollback)
+ vc->vc_sw->con_flush_scrollback(vc);
+}
+
/*
* Redrawing of screen
*/
@@ -1171,6 +1179,7 @@ static void csi_J(struct vc_data *vc, int vpar)
case 3: /* erase scroll-back buffer (and whole display) */
scr_memsetw(vc->vc_screenbuf, vc->vc_video_erase_char,
vc->vc_screenbuf_size);
+ flush_scrollback(vc);
set_origin(vc);
if (con_is_visible(vc))
update_screen(vc);
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 48b9764..9a7c2bb 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -173,6 +173,16 @@ static struct vgacon_scrollback_info {
int restore;
} vgacon_scrollback;

+static void vgacon_scrollback_reset(size_t reset_size)
+{
+ if (vgacon_scrollback.data && reset_size > 0)
+ memset(vgacon_scrollback.data, 0, reset_size);
+
+ vgacon_scrollback.cnt = 0;
+ vgacon_scrollback.tail = 0;
+ vgacon_scrollback.cur = 0;
+}
+
static void vgacon_scrollback_init(int pitch)
{
int rows = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024/pitch;
@@ -305,6 +315,14 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)
} else
vgacon_cursor(c, CM_MOVE);
}
+
+static void vgacon_flush_scrollback(struct vc_data *c)
+{
+ size_t size = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024;
+
+ if (c->vc_num == fg_console)
+ vgacon_scrollback_reset(size);
+}
#else
#define vgacon_scrollback_startup(...) do { } while (0)
#define vgacon_scrollback_init(...) do { } while (0)
@@ -322,6 +340,10 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)
vga_vram_size);
vga_set_mem_top(c);
}
+
+static void vgacon_flush_scrollback(struct vc_data *c)
+{
+}
#endif /* CONFIG_VGACON_SOFT_SCROLLBACK */

static const char *vgacon_startup(void)
@@ -1329,7 +1351,6 @@ static bool vgacon_scroll(struct vc_data *c, unsigned int t, unsigned int b,
return true;
}

-
/*
* The console `switch' structure for the VGA based console
*/
@@ -1362,6 +1383,7 @@ const struct consw vga_con = {
.con_save_screen = vgacon_save_screen,
.con_build_attr = vgacon_build_attr,
.con_invert_region = vgacon_invert_region,
+ .con_flush_scrollback = vgacon_flush_scrollback,
};
EXPORT_SYMBOL(vga_con);

diff --git a/include/linux/console.h b/include/linux/console.h
index 9c26c66..5949d18 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -73,6 +73,10 @@ struct consw {
u16 *(*con_screen_pos)(struct vc_data *, int);
unsigned long (*con_getxy)(struct vc_data *, unsigned long, int *, int *);
/*
+ * Flush the video console driver's scrollback buffer
+ */
+ void (*con_flush_scrollback)(struct vc_data *);
+ /*
* Prepare the console for the debugger. This includes, but is not
* limited to, unblanking the console, loading an appropriate
* palette, and allowing debugger generated output.
--
2.1.4

2016-11-27 21:38:39

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

Hi Manuel,

I've just patched next-20161125 with this set and given it a run.

Scrollback persistence works fine, just as in earlier versions.

This time I didn't forget to test clear operation.

The only important concern is that after logout, the scrollback is not
wiped by /bin/login or /sbin/agetty (not sure who of them is responsible
for that). What do you see on your workstations in this case?

I guess we need to do something of the following:
- catch some control character sequences to wipe the scrollback
- indicate (by some flag) some feature capability for this
- request update in terminfo database or whatever, to let ncurses know
that it is capable of scrollback wiping by some control charater
sequences

Some useless notes follow.

I see the user experience is subpar to what I'm accustomed to (I use
Konsole and "Clear Scrollback and Reset" action, default shortcut is
Ctrl+Shift+K). The strange behaviour moments have nothing to do with
current patchset but are properties of vgacon, though. (I compared it
with another PC which runs without this patchset, and it looks like it
runs vgacon, too, however, I'm not sure how to ensure this at runtime.)

clear(1) doesn't wipe the scrollback at all, it is still reachable, all
of it.

echo -e "\e[3J" seems to wipe the scrollback, but if you do it several
times in a row, every time you (or at last I do) get your prompt a bit
lower, so after many times you end up with blank screen and the prompt
at the bottom of the screen.

Have you encountered this, or is it something specific to my setup (I
use bash prompt spanning to multiple lines, and calling "stty sane" from
inside every PS1 evaluation. I can share the config if you request it).

2016-11-27 23:16:10

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

On Sun, Nov 27, 2016 at 09:37:30PM +0000, Andrey Utkin wrote:
> I've just patched next-20161125 with this set and given it a run.
>
> Scrollback persistence works fine, just as in earlier versions.
>
> This time I didn't forget to test clear operation.
>
> The only important concern is that after logout, the scrollback is not
> wiped by /bin/login or /sbin/agetty (not sure who of them is responsible
> for that). What do you see on your workstations in this case?

If you're on Debian or a derivative, that's clear_console. It uses a
switch-vt-then-back hack which obviously doesn't work with scrollback
persistence. Reported as https://bugs.debian.org/845177 -- I'll molest the
maintainer if the patch doesn't get applied soon, so we can have the fix in
time for stretch (then Ubuntu zesty).

Because of a sad lack of a time machine, old systems will use clear_console
with that hack until they die, that's why
CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT defaults to n; in a few years it'll
be ok to flip it.

> I guess we need to do something of the following:
> - catch some control character sequences to wipe the scrollback

\e[3J

> - indicate (by some flag) some feature capability for this

Terminfo calls this flag "E3".

> - request update in terminfo database or whatever, to let ncurses know
> that it is capable of scrollback wiping by some control charater
> sequences

Already there for quite a while.

> clear(1) doesn't wipe the scrollback at all, it is still reachable, all
> of it.

It does for me on the console. The man page says:

# clear clears your screen if this is possible, including its scrollback
# buffer (if the extended "E3" capability is defined). clear looks in the
# environment for the terminal type and then in the terminfo database to
# determine how to clear the screen.

Because of its reliance on terminfo, you need to have TERM=linux in your
environment; also, screen/tmux obviously breaks this.

> echo -e "\e[3J" seems to wipe the scrollback, but if you do it several
> times in a row, every time you (or at last I do) get your prompt a bit
> lower, so after many times you end up with blank screen and the prompt
> at the bottom of the screen.

Yeah, none of \e[J subcommands move the cursor at all. As you use echo
without -n, you move two lines lower, and even with -n the command you typed
takes a line. You want to move the cursor explicitly, add "\e[H".

Meow!
--
The bill declaring Jesus as the King of Poland fails to specify whether
the addition is at the top or end of the list of kings. What should the
historians do?

2016-11-27 23:38:26

by Jakub Wilk

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

* Adam Borowski <[email protected]>, 2016-11-28, 00:15:
>>clear(1) doesn't wipe the scrollback at all, it is still reachable, all of
>>it.
>
>It does for me on the console. The man page says:
>
># clear clears your screen if this is possible, including its scrollback
># buffer (if the extended "E3" capability is defined). clear looks in the
># environment for the terminal type and then in the terminfo database to
># determine how to clear the screen.
>
>Because of its reliance on terminfo, you need to have TERM=linux in your
>environment; also, screen/tmux obviously breaks this.

The "linux" terminfo entry didn't have E3 until very recently.
You will need ncurses >= 20160514.

--
Jakub Wilk

2016-11-27 23:54:56

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

\e[3J works well now, thanks!

I haven't found any more problems; your changes also appear to make no
regressions in at least nouveau fb (which obviously doesn't have this goodie
yet).

Patch 2 doesn't apply cleanly on current Linus' tree but it's just a matter
of more fuzz than "git am" allows.

Tested-by: Adam Borowski <[email protected]>

Meow!
--
The bill declaring Jesus as the King of Poland fails to specify whether
the addition is at the top or end of the list of kings. What should the
historians do?

2016-11-28 00:02:30

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

On Mon, Nov 28, 2016 at 12:15:48AM +0100, Adam Borowski wrote:
> On Sun, Nov 27, 2016 at 09:37:30PM +0000, Andrey Utkin wrote:
> > I've just patched next-20161125 with this set and given it a run.
> >
> > Scrollback persistence works fine, just as in earlier versions.
> >
> > This time I didn't forget to test clear operation.
> >
> > The only important concern is that after logout, the scrollback is not
> > wiped by /bin/login or /sbin/agetty (not sure who of them is responsible
> > for that). What do you see on your workstations in this case?
>
> If you're on Debian or a derivative, that's clear_console. It uses a
> switch-vt-then-back hack which obviously doesn't work with scrollback
> persistence. Reported as https://bugs.debian.org/845177 -- I'll molest the
> maintainer if the patch doesn't get applied soon, so we can have the fix in
> time for stretch (then Ubuntu zesty).

I'm on Gentoo.

> Because of a sad lack of a time machine, old systems will use clear_console
> with that hack until they die, that's why
> CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT defaults to n; in a few years it'll
> be ok to flip it.
>
> > I guess we need to do something of the following:
> > - catch some control character sequences to wipe the scrollback
>
> \e[3J
>
> > - indicate (by some flag) some feature capability for this
>
> Terminfo calls this flag "E3".
>
> > - request update in terminfo database or whatever, to let ncurses know
> > that it is capable of scrollback wiping by some control charater
> > sequences
>
> Already there for quite a while.
>
> > clear(1) doesn't wipe the scrollback at all, it is still reachable, all
> > of it.
>
> It does for me on the console. The man page says:
>
> # clear clears your screen if this is possible, including its scrollback
> # buffer (if the extended "E3" capability is defined). clear looks in the
> # environment for the terminal type and then in the terminfo database to
> # determine how to clear the screen.
>
> Because of its reliance on terminfo, you need to have TERM=linux in your
> environment; also, screen/tmux obviously breaks this.

I wonder whether my ncurses is not bleeding-edge enough, or I have some
non-standard config. Anyway, thanks for explanation.

> > echo -e "\e[3J" seems to wipe the scrollback, but if you do it several
> > times in a row, every time you (or at last I do) get your prompt a bit
> > lower, so after many times you end up with blank screen and the prompt
> > at the bottom of the screen.
>
> Yeah, none of \e[J subcommands move the cursor at all. As you use echo
> without -n, you move two lines lower, and even with -n the command you typed
> takes a line. You want to move the cursor explicitly, add "\e[H".

Thanks for explanation.

2016-11-28 21:24:06

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

On Mo, 2016-11-28 at 00:53 +0100, Adam Borowski wrote:
> \e[3J works well now, thanks!
Great to hear that!

> Tested-by: Adam Borowski <[email protected]>
Thanks, Adam, for spending all this time testing the patches!

2016-11-28 21:28:42

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

Hi Andrey,

Adam already discussed some of your notes, but I want to catch up one
this one:

On So, 2016-11-27 at 21:37 +0000, Andrey Utkin wrote:
> I see the user experience is subpar to what I'm accustomed to (I use
> Konsole and "Clear Scrollback and Reset" action, default shortcut is
> Ctrl+Shift+K). The strange behaviour moments have nothing to do with
> current patchset but are properties of vgacon, though. (I compared it
> with another PC which runs without this patchset, and it looks like it
> runs vgacon, too, however, I'm not sure how to ensure this at runtime.)
I'm not sure what you mean with 'subpar'. Ctrl+Shift+K would probably be
nice - but it might interfere with some shortcuts of programs.
Are you missing any other features?
(I am working on persistent scrollback for framebuffer consoles in the
mean time...)

Bye,

Manuel

2016-11-29 10:01:22

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

On Mon, Nov 28, 2016 at 10:28:19PM +0100, Manuel Sch?lling wrote:
> Hi Andrey,
>
> Adam already discussed some of your notes, but I want to catch up one
> this one:
>
> On So, 2016-11-27 at 21:37 +0000, Andrey Utkin wrote:
> > I see the user experience is subpar to what I'm accustomed to (I use
> > Konsole and "Clear Scrollback and Reset" action, default shortcut is
> > Ctrl+Shift+K). The strange behaviour moments have nothing to do with
> > current patchset but are properties of vgacon, though. (I compared it
> > with another PC which runs without this patchset, and it looks like it
> > runs vgacon, too, however, I'm not sure how to ensure this at runtime.)
> I'm not sure what you mean with 'subpar'. Ctrl+Shift+K would probably be
> nice - but it might interfere with some shortcuts of programs.
> Are you missing any other features?
> (I am working on persistent scrollback for framebuffer consoles in the
> mean time...)

I meant that in my terminal of choice, I have single action which does
both of these things at same time:
- scrollback cleaning (also not leaving screen worth of emptiness in
scrollback)
- prompt positioning to top line

That's all. I don't appeal to bring Ctrl+Shift+K hotkey or whatever
else.

Since these cosmetic matters are inherent to vgacon I'm fine with it.

Regarding logout scrollback clearing not working for me. ncurses-6.0-rc1
which I tested it with is the latest available in Gentoo portage, please
confirm whether I need any newer version, or should I tune something
else. I'd appreciate if you also tested your patch with gentoo setup.

2016-11-29 10:44:39

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

On Tue, Nov 29, 2016 at 10:01:15AM +0000, Andrey Utkin wrote:
> Regarding logout scrollback clearing not working for me. ncurses-6.0-rc1
> which I tested it with is the latest available in Gentoo portage, please
> confirm whether I need any newer version, or should I tune something
> else. I'd appreciate if you also tested your patch with gentoo setup.

Could you please check whether its terminfo carries the required
definitions? In ncurses sources, that's misc/terminfo.src

The relevant parts are:
.--====
# The 3.0 kernel adds support for clearing scrollback buffer (capability E3).
# It is the same as xterm's erase-saved-lines feature.
linux3.0|linux 3.0 kernels,
E3=\E[3J, use=linux2.6,

# This is Linux console for ncurses.
linux|linux console,
use=linux3.0,
`----

I believe the first part was added first; if that's true it's possible this
will work for you:
TERM=linux3.0 clear

I'm not sure what Gentoo does to clear the console during logout: it might
just invoke "clear" (or its underlying ncurses implementation), it might
carry a copy of Debian's "clear_console"[1], it might do something else
entirely.

Meow!

[1]. It originally came from Ubuntu, forked there from "clear".
--
The bill declaring Jesus as the King of Poland fails to specify whether
the addition is at the top or end of the list of kings. What should the
historians do?

2016-11-29 16:35:23

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

Hi Andrey,

On Di, 2016-11-29 at 10:01 +0000, Andrey Utkin wrote:
> Regarding logout scrollback clearing not working for me. ncurses-6.0-rc1
> which I tested it with is the latest available in Gentoo portage, please
> confirm whether I need any newer version, or should I tune something
> else. I'd appreciate if you also tested your patch with gentoo setup.
Are you sure ncurses is involved at all?
My Debian agetty(8) manpage says:

-J,--noclear
Do not clear the screen before prompting for the login name (the
screen is normally cleared).

And digging into the source code of agetty shows these lines [1]:

static void termio_clear(int fd)
{
/*
* Do not write a full reset (ESC c) because this destroys
* the unicode mode again if the terminal was in unicode
* mode. Also it clears the CONSOLE_MAGIC features which
* are required for some languages/console-fonts.
* Just put the cursor to the home position (ESC [ H),
* erase everything below the cursor (ESC [ J), and set the
* scrolling region to the full window (ESC [ r)
*/
write_all(fd, "\033[r\033[H\033[J", 9);
}

So I guess that agetty relies on on switching the console for flushing
the scrollback buffer and we'd had to add the \E[3J sequence here.

Note that up until now I just had a look at the theory (manpage and
source code). I'd need some days to find time to show at runtime that
this really is the reason why the buffer is not flushed.

Bye,

Manuel

[1] https://github.com/karelzak/util-linux/blob/master/term-utils/agetty.c#L1175

2016-12-01 21:03:59

[permalink] [raw]

Subject: Re: [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles

Hi Andrey,

On Di, 2016-11-29 at 10:01 +0000, Andrey Utkin wrote:
> On Mon, Nov 28, 2016 at 10:28:19PM +0100, Manuel Schölling wrote:
> Regarding logout scrollback clearing not working for me. ncurses-6.0-rc1
> which I tested it with is the latest available in Gentoo portage, please
> confirm whether I need any newer version, or should I tune something
> else. I'd appreciate if you also tested your patch with gentoo setup.

I finally setup gentoo running agetty (util-linux-2.26.2) and patching
the file term-utils/agetty.c with

static void termio_clear(int fd)
{
/*
* Do not write a full reset (ESC c) because this destroys
* the unicode mode again if the terminal was in unicode
* mode. Also it clears the CONSOLE_MAGIC features which
* are required for some languages/console-fonts.
* Just put the cursor to the home position (ESC [ H),
* erase everything below the cursor (ESC [ J), and set the
* scrolling region to the full window (ESC [ r)
*/
- write_all(fd, "\033[r\033[H\033[J", 9);
+ write_all(fd, "\033[3J\033[r\033[H\033[J", 13);
}

solves the issue with the scrollback buffer after log out.
Let me know if you agree that this is the right way to go and I will
send a patch to the maintainer of util-linux.

Thanks again for spending all this time to test the patch!

Have a good weekend!

Manuel

2016-12-01 21:31:46