From: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
To: "Manuel Schölling" <manuel.schoelling@gmx.de>
Cc: gregkh@linuxfoundation.org, jslaby@suse.com, kilobyte@angband.pl,
lkml14@scotdoyle.com, rdunlap@infradead.org, shorne@gmail.com,
andrey_utkin@fastmail.com, akpm@linux-foundation.org,
paul.burton@imgtec.com, daniel.vetter@ffwll.ch, tj@kernel.org,
hdegoede@redhat.com, linux-kernel@vger.kernel.org,
linux-fbdev@vger.kernel.org
Subject: Re: [PATCH v8 3/3] console: Add persistent scrollback buffers for all VGA consoles
Date: Tue, 10 Jan 2017 17:44:33 +0100 [thread overview]
Message-ID: <4640722.jsFVORW8WM@amdc3058> (raw)
In-Reply-To: <15146537.m1k7GYybWH@amdc3058>
On Tuesday, January 10, 2017 05:22:22 PM Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> The patchset generally looks fine to me but I have a question
> regarding new VGACON_SOFT_SCROLLBACK_PERSISTENT config option.
>
> Since the code size impact to support the persistent scrollback
> feature is minimal wouldn't it be better to always include it?
>
> The feature would be disabled by default and could be enabled
> by using the new kernel command line parameter (you may also add
> a new config option for enabling it by default if desired).
Something like:
#ifdef VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT
static bool scrollback_persistent = 1;
#else
static bool scrollback_persistent;
#endif
module_param_named(scrollback_persistent, scrollback_persistent, bool, 0);
MODULE_PARM_DESC(scrollback_persistent, "Enable persistent scrollback feature");
and then use scrollback_persistent variable in vgacon_scrollback_switch()
to control the actual behavior instead of ifdefs.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
> On Thursday, January 05, 2017 12:33:20 PM 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 <manuel.schoelling@gmx.de>
> > Reviewed-by: Andrey Utkin <andrey_utkin@fastmail.com>
> > Tested-by: Andrey Utkin <andrey_utkin@fastmail.com>
> > Tested-by: Adam Borowski <kilobyte@angband.pl>
> > ---
> > 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 c3f1fb9ee820..f500e58f7636 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 9a7c2bbc5326..ca23d222e029 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 */
> > }
next prev parent reply other threads:[~2017-01-10 16:44 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-04 10:53 [PATCH v7 0/3] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
2016-12-04 10:53 ` [PATCH v7 1/3] console: Move scrollback data into its own struct Manuel Schölling
2016-12-04 10:53 ` [PATCH v7 2/3] console: Add callback to flush scrollback buffer to consw struct Manuel Schölling
2016-12-04 10:53 ` [PATCH v7 3/3] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
2016-12-06 10:02 ` [PATCH v7 0/3] " Greg KH
2016-12-06 16:32 ` Manuel Schölling
2016-12-21 16:37 ` [PATCH RESEND " Manuel Schölling
2016-12-21 16:37 ` [PATCH RESEND v7 1/3] console: Move scrollback data into its own struct Manuel Schölling
2016-12-21 16:37 ` [PATCH RESEND v7 2/3] console: Add callback to flush scrollback buffer to consw struct Manuel Schölling
2016-12-21 16:37 ` [PATCH RESEND v7 3/3] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
2017-01-04 13:54 ` [PATCH RESEND v7 0/3] " Andrey Utkin
2017-01-05 11:33 ` [PATCH v8 " Manuel Schölling
2017-01-05 11:33 ` [PATCH v8 1/3] console: Move scrollback data into its own struct Manuel Schölling
2017-01-05 11:33 ` [PATCH v8 2/3] console: Add callback to flush scrollback buffer to consw struct Manuel Schölling
2017-01-05 11:33 ` [PATCH v8 3/3] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
[not found] ` <CGME20170110162224epcas1p2095912fa6b73a1457fa86396149e78e9@epcas1p2.samsung.com>
2017-01-10 16:22 ` Bartlomiej Zolnierkiewicz
[not found] ` <CGME20170110164435epcas1p314efb9c6ea3ffb971fce06f3bfaec736@epcas1p3.samsung.com>
2017-01-10 16:44 ` Bartlomiej Zolnierkiewicz [this message]
2017-01-10 17:31 ` Manuel Schölling
2017-01-10 21:28 ` [PATCH v9 0/4] " Manuel Schölling
2017-01-10 21:28 ` [PATCH v9 1/4] console: Move scrollback data into its own struct Manuel Schölling
2017-01-10 21:28 ` [PATCH v9 2/4] console: Add callback to flush scrollback buffer to consw struct Manuel Schölling
2017-01-10 21:28 ` [PATCH v9 3/4] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
2017-01-10 21:28 ` [PATCH v9 4/4] console: Make persistent scrollback a boot parameter Manuel Schölling
2017-01-10 22:58 ` Adam Borowski
2017-01-11 21:41 ` Manuel Schölling
2017-01-13 20:00 ` Manuel Schölling
2017-01-14 7:26 ` Greg KH
2017-01-11 13:32 ` kbuild test robot
2017-01-13 20:07 ` [PATCH v10 0/4] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
2017-01-25 10:55 ` Greg KH
2017-01-13 20:07 ` [PATCH v10 1/4] console: Move scrollback data into its own struct Manuel Schölling
2017-01-13 20:07 ` [PATCH v10 2/4] console: Add callback to flush scrollback buffer to consw struct Manuel Schölling
2017-01-13 20:07 ` [PATCH v10 3/4] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
2017-01-19 13:23 ` Greg KH
2017-01-19 16:12 ` Manuel Schölling
2017-01-19 16:33 ` Greg KH
2017-01-19 23:04 ` Adam Borowski
2017-01-20 13:16 ` Adam Borowski
2017-01-20 13:31 ` Greg KH
2017-01-20 14:27 ` Adam Borowski
2017-01-19 17:20 ` Adam Borowski
2017-01-13 20:07 ` [PATCH v10 4/4] console: Make persistent scrollback a boot parameter Manuel Schölling
2017-02-02 20:07 ` [PATCH v9 " Paul Gortmaker
2017-02-03 16:04 ` Manuel Schölling
2017-02-03 16:45 ` Adam Borowski
2017-01-02 14:27 ` [PATCH RESEND v7 0/3] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
2017-01-02 14:27 ` [PATCH RESEND v7 1/3] console: Move scrollback data into its own struct Manuel Schölling
2017-01-02 14:27 ` [PATCH RESEND v7 2/3] console: Add callback to flush scrollback buffer to consw struct Manuel Schölling
2017-01-02 14:27 ` [PATCH RESEND v7 3/3] console: Add persistent scrollback buffers for all VGA consoles Manuel Schölling
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4640722.jsFVORW8WM@amdc3058 \
--to=b.zolnierkie@samsung.com \
--cc=akpm@linux-foundation.org \
--cc=andrey_utkin@fastmail.com \
--cc=daniel.vetter@ffwll.ch \
--cc=gregkh@linuxfoundation.org \
--cc=hdegoede@redhat.com \
--cc=jslaby@suse.com \
--cc=kilobyte@angband.pl \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkml14@scotdoyle.com \
--cc=manuel.schoelling@gmx.de \
--cc=paul.burton@imgtec.com \
--cc=rdunlap@infradead.org \
--cc=shorne@gmail.com \
--cc=tj@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).