From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759220AbdAJQWc convert rfc822-to-8bit (ORCPT ); Tue, 10 Jan 2017 11:22:32 -0500 Received: from mailout3.samsung.com ([203.254.224.33]:43999 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758252AbdAJQW2 (ORCPT ); Tue, 10 Jan 2017 11:22:28 -0500 X-AuditID: b6c32a39-f793d6d000000fd7-ad-58750a415857 From: Bartlomiej Zolnierkiewicz To: Manuel =?ISO-8859-1?Q?Sch=F6lling?= 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:22:22 +0100 Message-id: <15146537.m1k7GYybWH@amdc3058> User-Agent: KMail/4.13.3 (Linux/3.13.0-96-generic; KDE/4.13.3; x86_64; ; ) In-reply-to: <20170105113320.18166-4-manuel.schoelling@gmx.de> MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset=iso-8859-1 X-Brightmail-Tracker: H4sIAAAAAAAAA02SbUhTYRTHeXbv7r2ulret7GAvxiCqSSuj4EnNXvDDJQrqgySB6cqbSk7X pqGRoBSZJlJWzIal5kRd4ltiWurUMlep2ItJpbaszLJ0c2iZZLldA7/9D+f3P+f5Hx6GkI2I vZmYuAReF6eOVVASsu7hRtWmPZLE0C2fJwicV1lO4b9Tr0lc+GGAwOeKKin8vcMgwteq7CJc dvMPha3ZdjF+eT+Pwu8mcmk8Uz8twu+bq2n8o99A4qbrNoR/l7Sj3Z5csaOO4G5/zae5pqkC kmswDtCc3XGUy2roRNzdUiVXY86gOGvuDMlZbpbT3HhzL8UVFtyhuMraXpJz1qw5uOSIJDCS j405zes2B0VIoktnpylta0TS0+EqOhV9O5iJPBhgt0FZiZMUtBf0DFZSmUjCyNh6BPfy39NC kS6C2WwT8d/RndM/38hDcL7xBhKKSQSWToObolh/uJJuRi69jN0LXY/TRS6IYF+IoN02Rrsa cjYMhqodbohk18HorUa3Wcoq4UmXwCxn90Fts8vswXiwQTBhn0UCsxR+XR10P5xgfeHTWI9Y 0Cp4lWolXcuALWbAkmGeMzNzxWqoaZmPEAzWJ62UoOXwraOWFvRKMM++FQnaMHeAGRDm3EVg fnB5HgqAhx3P55ctgbHJLLEwXwoXL8gEhIMfZaliQe+B7DYTKVyoHYHTYiMvIx/jggzGBRmM CzIUIMKMvHitXhPF6/2021V6tUafGBelOh6vqUHu/6fcUY8ed+9vQyyDFIuleDQhVCZWn9Yn a9oQMIRimXSYSQyVSSPVyWd4XXy4LjGW17ehlQypWCEtSgsIlbFR6gT+JM9red3/rojx8E5F Lceix9e+i+2Z2lBQ3NlkCqkYDvYXf/as2Bp+eDV92NTqZXRmrF8TdSkppa9vfPvJZ7vkXzzv tNjCy//cIDyrFq3iRwLfnPKKKN5ZCB8sOSnYZuw6kLKPmQyL8d2qHbIbPrYfSnGI0zSffgaf 9U3bnavKKvPJdj46Uf30bMh4vJxQkPpotZ+S0OnV/wBQbbtGewMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupkleLIzCtJLcpLzFFi42I5/e+xoK4DV2mEwa0bGhZz1q9hs/j/7RqL xcKHd5ktmhevZ7N4c3w6k8WUDR+YLFbO/ctmcaLvA6vF5V1z2Cxuf5rBbvF7x08mi/v7NrJb vL0zncVi79QHjBa/lh9ldOD3WPpxG7PHopfz2T32flvA4rFz1l12jw8f4zx6dp5h9Ni8Qstj 06pONo8TM36zeOyfu4bd4/2+q2weCxesZvNYv+Uqi8fnTXIBfFFuNhmpiSmpRQqpecn5KZl5 6bZKoSFuuhZKCnmJuam2ShG6viFBSgpliTmlQJ6RARpwcA5wD1bSt0twy1jx7ydbwcGEilPP NrA3ML4K6GLk5JAQMJE4N+kOO4QtJnHh3nq2LkYuDiGBWYwSK3//gXK+Mkp8fjMFrIpNwEpi YvsqRhBbRMBJ4uyxdiaQImaBC0wSl++cZgZJCAvESnTsOQtmswioSryetwfM5hXQkjh59h3Y IFEBL4kt+0CaOTk4BewkPn34BzZUSKBQ4t3sTlaIekGJH5PvsYDYzALaEk/eXWCFsHUker9/ Y57ACHQnQtksJGWzkJQtYGRexSiRWpBcUJyUnmuYl1quV5yYW1yal66XnJ+7iRGcDJ5J7WA8 uMv9EKMAB6MSD++DFyURQqyJZcWVuYcYJTiYlUR4n3GURgjxpiRWVqUW5ccXleakFh9iNAV6 cCKzlGhyPjBR5ZXEG5qYm5gbG1iYW1qaGCmJ8zbOfhYuJJCeWJKanZpakFoE08fEwSnVwMgS E+fJ2V+1saX/o9ScNC3pGaobZqg8bmUoedfJO/3F53reCueXP+0VZ/JePrvt5NlXO0t2Cnpx SJ28K7s9iTP3QfimNLuYrLOlrx8xnO7ZUDXh8Abdo6wh66PT1nQd5prUueKqPuvMVX/rb9vO eJIoKp609slJt3UKSZuvNjutC1XfKpa0S1SJpTgj0VCLuag4EQBvEF/lHAMAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170110162224epcas1p2095912fa6b73a1457fa86396149e78e9 X-Msg-Generator: CA X-Sender-IP: 203.254.230.26 X-Local-Sender: =?UTF-8?B?QmFydGxvbWllaiBab2xuaWVya2lld2ljehtTUlBPTC1LZXJu?= =?UTF-8?B?ZWwgKFRQKRvsgrzshLHsoITsnpAbU2VuaW9yIFNvZnR3YXJlIEVuZ2luZWVy?= X-Global-Sender: =?UTF-8?B?QmFydGxvbWllaiBab2xuaWVya2lld2ljehtTUlBPTC1LZXJu?= =?UTF-8?B?ZWwgKFRQKRtTYW1zdW5nIEVsZWN0cm9uaWNzG1NlbmlvciBTb2Z0d2FyZSBF?= =?UTF-8?B?bmdpbmVlcg==?= X-Sender-Code: =?UTF-8?B?QzEwG0VIURtDMTBDRDAyQ0QwMjczOTI=?= CMS-TYPE: 101P X-HopCount: 7 X-CMS-RootMailID: 20170110162224epcas1p2095912fa6b73a1457fa86396149e78e9 X-RootMTR: 20170110162224epcas1p2095912fa6b73a1457fa86396149e78e9 References: <20170105113320.18166-1-manuel.schoelling@gmx.de> <20170105113320.18166-4-manuel.schoelling@gmx.de> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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). 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 > Reviewed-by: Andrey Utkin > Tested-by: Andrey Utkin > Tested-by: Adam Borowski > --- > 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 */ > }