diff mbox

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

Message ID 1479679088-3015-3-git-send-email-manuel.schoelling@gmx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Manuel Schölling Nov. 20, 2016, 9:58 p.m. UTC
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>
---
 drivers/video/console/Kconfig  |  25 +++++++-
 drivers/video/console/vgacon.c | 131 +++++++++++++++++++++++++++--------------
 2 files changed, 108 insertions(+), 48 deletions(-)

Comments

kernel test robot Nov. 20, 2016, 10:23 p.m. UTC | #1
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
Adam Borowski Nov. 21, 2016, 8:17 p.m. UTC | #2
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 <manuel.schoelling@gmx.de>
> ---

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
Manuel Schölling Nov. 22, 2016, 4:56 p.m. UTC | #3
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


--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Borowski Nov. 23, 2016, 5:33 p.m. UTC | #4
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!
Manuel Schölling Nov. 23, 2016, 5:33 p.m. UTC | #5
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 <manuel.schoelling@gmx.de>
> > ---

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


--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Manuel Schölling Nov. 27, 2016, 4:51 p.m. UTC | #6
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 (kilobyte@angband.pl).
  - 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(-)
Andrey Utkin Nov. 27, 2016, 9:37 p.m. UTC | #7
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).
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Borowski Nov. 27, 2016, 11:15 p.m. UTC | #8
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!
Jakub Wilk Nov. 27, 2016, 11:30 p.m. UTC | #9
* Adam Borowski <kilobyte@angband.pl>, 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.
Adam Borowski Nov. 27, 2016, 11:53 p.m. UTC | #10
\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 <kilobyte@angband.pl>


Meow!
Andrey Utkin Nov. 28, 2016, 12:02 a.m. UTC | #11
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Manuel Schölling Nov. 28, 2016, 9:23 p.m. UTC | #12
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 <kilobyte@angband.pl>
Thanks, Adam, for spending all this time testing the patches!


--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Manuel Schölling Nov. 28, 2016, 9:28 p.m. UTC | #13
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

--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrey Utkin Nov. 29, 2016, 10:01 a.m. UTC | #14
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Adam Borowski Nov. 29, 2016, 10:44 a.m. UTC | #15
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".
Manuel Schölling Nov. 29, 2016, 4:35 p.m. UTC | #16
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


--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Manuel Schölling Dec. 1, 2016, 9:03 p.m. UTC | #17
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


--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andrey Utkin Dec. 1, 2016, 9:31 p.m. UTC | #18
On Thu, Dec 01, 2016 at 10:03:23PM +0100, Manuel Schölling wrote:
> 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

Wow, what a big undertaking.

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

I believe you that this works and you must know better than me whether
this solution is correct. Besides that, I'd suggest updating that large
block comment before the updated write_all() call to describe the new
action you're doing. Please CC me in your discussion with util-linux
maintainers.

Also I'd suggest coming back in a while, to set this feature enabled by
default. I wonder how many years to wait gracefully until "stable"
distros update util-linux to ensure secure scrollback wiping. 3? 5?

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

Thank you very much for your time and effort in bringing this useful
feature!

I give all sorts of my approval on this patchset:
Reviewed-by: Andrey Utkin <andrey_utkin@fastmail.com>
Tested-by: Andrey Utkin <andrey_utkin@fastmail.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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 */
 }