2016-11-10 21:47:55

by Andrey Utkin

[permalink] [raw]
Subject: Re: [PATCH v3] console: Add persistent scrollback buffers for all VGA consoles

On Mon, Sep 19, 2016 at 12:06:57AM +0200, 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.

Hi,

I have noticed your message on #kernelnewbies IRC channel.

Thanks for the patch. This is definitely very useful, especially when
you boot into rescue system and use raw console and not a fancy
X11-based terminal app.

>
> It always annoys me when I switch back to a console and I can just
> read half of an error backtrace. This should fix issues like these.

Personal feelings don't belong to commit messages. Previous paragraph is
enough to express the purpose.

>
> 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.

Have never heard of such tool as clear_console. Why mention it at all,
where is it used, which are practical configurations which are broken in
this way?

Does this break clear(1)?

>
> Signed-off-by: Manuel Sch?lling <[email protected]>
> ---
> 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)
> ---
> drivers/video/console/Kconfig | 23 +++++-
> drivers/video/console/vgacon.c | 160 +++++++++++++++++++++++++++--------------
> 2 files changed, 128 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
> index 38da6e2..67e52f0 100644
> --- a/drivers/video/console/Kconfig
> +++ b/drivers/video/console/Kconfig
> @@ -43,9 +43,26 @@ 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.

Indentation amended, I don't know which way it should be, please
double-check or mimic old indentation to stay on safe side.

> +
> +config VGACON_SOFT_SCROLLBACK_FOR_EACH_CONSOLE

Feels like bad naming. What about VGACON_SOFT_SCROLLBACK_PERSISTENT?

> + bool "Persistent Scrollback History for each console"
> + depends on VGACON_SOFT_SCROLLBACK
> + default n
> + help
> + Say Y here if for each VGA console a scrollback buffer should
> + be allocated. The scrollback history will persist when switching

Unnatural order of sentence IMHO, however I'm not native English.

> + between consoles. If you say N here, scrollback is only supported
> + for the active VGA console and scrollback history will be flushed
> + when switching between consoles.

I think you may put less text here. You can skip details like "buffer
... allocated", just explain the difference in behaviour.

> +
> + This breaks legacy versions of tools like clear_console which
> + might cause security issues.
> + Use the escape sequence \e[3J instead if this feature is activated.

See above. I'd drop this paragraph if clear(1) is not broken.

> +
> + If you use a RAM-constrained system, say N here.

Which exactly cases to consider? I can't think of any practical one. You
may want to help user to figure out if he should consider this by
explaining that VGACON_SOFT_SCROLLBACK_SIZE is taken for each created
tty device (right?). Or drop this line :)

>
> config MDA_CONSOLE
> depends on !M68K && !PARISC && ISA
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 1157661..e70e8fe 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -1,5 +1,5 @@
> /*
> - * linux/drivers/video/vgacon.c -- Low level VGA based console driver
> + * linux/drivers/video/console/vgacon.c -- Low level VGA based console driver

Cosmetic changes in separate patch please.

> *
> * Created 28 Sep 1997 by Geert Uytterhoeven
> *
> @@ -181,70 +181,125 @@ 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 void vgacon_scrollback_init(int pitch)
> +struct vgacon_scrollback_info {
> + void *data;
> + int tail;
> + int size;
> + int rows;
> + int cnt;
> + int cur;
> + int save;
> + int restore;
> +};

Refactor in separate patch please.
It would be much nicer if you make first patch in series which would
gather all these variables into struct and uses it everywhere, leaving
external behaviour the same. Then, in following patches, change the
behaviour, one trait at a time.

> +static struct vgacon_scrollback_info *vgacon_scrollback_cur;

Could you use name vgacon_scrollback instead of new
vgacon_scrollback_cur? It doesn't change anything conceptually, but is a
bit shorter and seems to mean current scrollback context anyway, right?

I think I see nothing to comment on below.

Looking forward to see your reply. Would definitely like to test this
when I have some time.


2016-11-16 17:39:18

by Manuel Schölling

[permalink] [raw]
Subject: [PATCH v4] console: Add persistent scrollback buffers for all VGA console

---
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)



2016-11-16 17:40:31

by Manuel Schölling

[permalink] [raw]
Subject: Re: [PATCH v3] console: Add persistent scrollback buffers for all VGA consoles

Hi,

I finally found the time to rework the patches, but there are a few
things I want to discuss:

> > 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.
>
> Have never heard of such tool as clear_console. Why mention it at all,
> where is it used, which are practical configurations which are broken in
> this way?
clear_console(1) is part of Debian's bash package and (I think) a
inofficial 'downstream' patch developed by Ubuntu [1]. There has been
security concerns (which is also why I disabled this feature by
default).
However, if there might be security issues, I would strongly suggest to
keep this part in the documentation - it does no harm anyway, does it?

> Does this break clear(1)?
No, my tests with clear(1) worked fine.

> > - 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.
>
> Indentation amended, I don't know which way it should be, please
> double-check or mimic old indentation to stay on safe side.
Double-checked! That's at least how I read the docs.

> > +static struct vgacon_scrollback_info *vgacon_scrollback_cur;
>
> Could you use name vgacon_scrollback instead of new
> vgacon_scrollback_cur? It doesn't change anything conceptually, but is a
> bit shorter and seems to mean current scrollback context anyway, right?
>
> I think I see nothing to comment on below.
There is a already an array called 'vgacon_scrollbacks[N]', so I would
not like to have two variables named 'vgacon_scrollback' and
'vgacon_scrollbacks'. Either {'vgacon_scrollback_cur' and
'vgacon_scrollbacks'} or {'vgacon_scrollback' and
'vgacon_scrollback_list'}.
You can make a suggestion, if you like.

All other point you mentioned where fixed as you suggested.

Thanks again for your help, Andrey!

Best,

Manuel

[1]
http://manpages.ubuntu.com/manpages/xenial/en/man1/clear_console.1.html


2016-11-16 20:31:27

by Manuel Schölling

[permalink] [raw]
Subject: [PATCH v4.1 0/2] console: Add persistent scrollback buffers for all VGA console

Well, that's embarrassing: I changed the order of some local commits
in the last minute, so my patches included a compiler error.

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 (2):
console: Move scrollback data into its own struct
console: Add persistent scrollback buffers for all VGA consoles

drivers/video/console/Kconfig | 23 +++++-
drivers/video/console/vgacon.c | 158 ++++++++++++++++++++++++++++-------------
2 files changed, 127 insertions(+), 54 deletions(-)

--
2.1.4

2016-11-16 20:31:36

by Manuel Schölling

[permalink] [raw]
Subject: [PATCH v4.1 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 | 23 ++++++-
drivers/video/console/vgacon.c | 134 +++++++++++++++++++++++++++++------------
2 files changed, 114 insertions(+), 43 deletions(-)

diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
index 38da6e2..8167540 100644
--- a/drivers/video/console/Kconfig
+++ b/drivers/video/console/Kconfig
@@ -43,9 +43,26 @@ 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 breaks legacy versions of tools like clear_console which
+ might cause security issues.
+ 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 1fee18f..8314f69 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,63 +171,116 @@ 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);
+
+ vgacon_scrollback_cur->cnt = 0;
+ vgacon_scrollback_cur->tail = 0;
+ vgacon_scrollback_cur->cur = 0;
+}
+
+static void vgacon_scrollback_init(int vc_num)
+{
+ int pitch = vga_video_num_columns * 2;
+ size_t size = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024;
+ int rows = size/pitch;
+ void *data;
+
+ data = kcalloc(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE, 1024, GFP_NOWAIT);
+ if (data) {
+ 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;

-static void vgacon_scrollback_init(int pitch)
+ vgacon_scrollback_reset(0);
+ } else {
+ pr_warn("VGAcon: failed to allocate memory for scrollback. Trying to reuse previous buffer.\n");
+ /* Leave vgacon_scrollback_cur untouched but reset its content */
+ vgacon_scrollback_reset(size);
+ }
+}
+
+static void vgacon_switch_scrollback(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;
+#ifdef CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT
+ if (!vgacon_scrollbacks[vc_num].data)
+ vgacon_scrollback_init(vc_num);
+ else
+ vgacon_scrollback_cur = &vgacon_scrollbacks[vc_num];
+#else
+ vc_num = 0;
+
+ if (!vgacon_scrollbacks[vc_num].data)
+ vgacon_scrollback_init(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);
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(vgacon_scrollbacks); ++i)
+ vgacon_scrollbacks[i].data = NULL;
+
+ 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)
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;
}
}

@@ -241,17 +294,17 @@ 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)
@@ -266,16 +319,16 @@ static void vgacon_scrolldelta(struct vc_data *c, int lines)
if (end > vgacon_scrollback.cnt)
end = vgacon_scrollback.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;
@@ -289,13 +342,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;
}

@@ -308,6 +361,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_switch_scrollback(...) do { } while (0)

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

--
2.1.4

2016-11-16 20:31:42

by Manuel Schölling

[permalink] [raw]
Subject: [PATCH v4.1 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 | 90 +++++++++++++++++++++---------------------
1 file changed, 46 insertions(+), 44 deletions(-)

diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index c22a562..1fee18f 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -162,31 +162,33 @@ 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 +196,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 +241,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 +289,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-16 21:34:37

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v4.1 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-rc5 next-20161116]
[cannot apply to v4.1]
[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-Add-persistent-scrollback-buffers-for-all-VGA-console/20161117-051658
config: x86_64-randconfig-x015-201646 (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_scrolldelta':
>> drivers/video/console/vgacon.c:332:14: error: 'vgacon_scrollback' undeclared (first use in this function)
if (start > vgacon_scrollback.cnt)
^~~~~~~~~~~~~~~~~
drivers/video/console/vgacon.c:332:14: note: each undeclared identifier is reported only once for each function it appears in

vim +/vgacon_scrollback +332 drivers/video/console/vgacon.c

25056ccd Manuel Sch?lling 2016-11-16 326 start = vgacon_scrollback_cur->cur + lines;
15bdab95 Antonino A. Daplas 2006-03-27 327 end = start + abs(lines);
15bdab95 Antonino A. Daplas 2006-03-27 328
15bdab95 Antonino A. Daplas 2006-03-27 329 if (start < 0)
15bdab95 Antonino A. Daplas 2006-03-27 330 start = 0;
15bdab95 Antonino A. Daplas 2006-03-27 331
18add6b3 Manuel Sch?lling 2016-11-16 @332 if (start > vgacon_scrollback.cnt)
18add6b3 Manuel Sch?lling 2016-11-16 333 start = vgacon_scrollback.cnt;
15bdab95 Antonino A. Daplas 2006-03-27 334
15bdab95 Antonino A. Daplas 2006-03-27 335 if (end < 0)

:::::: The code at line 332 was first introduced by commit
:::::: 18add6b31208f391323fcaa95f80c4f61d7bdd06 console: Move scrollback data into its own struct

:::::: TO: Manuel Sch?lling <[email protected]>
:::::: CC: 0day robot <[email protected]>

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


Attachments:
(No filename) (2.06 kB)
.config.gz (26.43 kB)
Download all attachments