| Message ID | 20170110212838.16175-5-manuel.schoelling@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 10, 2017 at 10:28:38PM +0100, Manuel Schölling wrote: > The impact of the persistent scrollback feature on the code size is > rather small, so the config option is removed. The feature stays > disabled by default and can be enabled by using the boot command line > parameter 'vgacon.scrollback_persistent=1' or by setting > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y. > > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> > Suggested-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > +module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000); > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent scrollback for all vga consoles"); A command-line knob settable by the end-user is something more persistent than a config option. As you're going to extend this code beyond vgacon in the near future, perhaps it'd be better to have a shared setting for all console drivers? Meow!
Hi Manuel,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.10-rc3 next-20170111]
[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-consoles/20170111-203640
config: i386-randconfig-x018-201702 (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=i386
All error/warnings (new ones prefixed by >>):
In file included from include/linux/module.h:18:0,
from drivers/video/console/vgacon.c:36:
drivers/video/console/vgacon.c: In function '__check_scrollback_persistent':
>> drivers/video/console/vgacon.c:1423:43: error: 'scrollback_persistent' undeclared (first use in this function)
module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000);
^
include/linux/moduleparam.h:344:68: note: in definition of macro '__param_check'
static inline type __always_unused *__check_##name(void) { return(p); }
^
include/linux/moduleparam.h:146:2: note: in expansion of macro 'param_check_bool'
param_check_##type(name, &(value)); \
^~~~~~~~~~~~
>> drivers/video/console/vgacon.c:1423:1: note: in expansion of macro 'module_param_named'
module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000);
^~~~~~~~~~~~~~~~~~
drivers/video/console/vgacon.c:1423:43: note: each undeclared identifier is reported only once for each function it appears in
module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000);
^
include/linux/moduleparam.h:344:68: note: in definition of macro '__param_check'
static inline type __always_unused *__check_##name(void) { return(p); }
^
include/linux/moduleparam.h:146:2: note: in expansion of macro 'param_check_bool'
param_check_##type(name, &(value)); \
^~~~~~~~~~~~
>> drivers/video/console/vgacon.c:1423:1: note: in expansion of macro 'module_param_named'
module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000);
^~~~~~~~~~~~~~~~~~
drivers/video/console/vgacon.c: At top level:
>> drivers/video/console/vgacon.c:1423:43: error: 'scrollback_persistent' undeclared here (not in a function)
module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000);
^
include/linux/moduleparam.h:225:54: note: in definition of macro '__module_param_call'
VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } }
^~~
include/linux/moduleparam.h:147:2: note: in expansion of macro 'module_param_cb'
module_param_cb(name, ¶m_ops_##type, &value, perm); \
^~~~~~~~~~~~~~~
>> drivers/video/console/vgacon.c:1423:1: note: in expansion of macro 'module_param_named'
module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000);
^~~~~~~~~~~~~~~~~~
vim +/scrollback_persistent +1423 drivers/video/console/vgacon.c
1417 .con_build_attr = vgacon_build_attr,
1418 .con_invert_region = vgacon_invert_region,
1419 .con_flush_scrollback = vgacon_flush_scrollback,
1420 };
1421 EXPORT_SYMBOL(vga_con);
1422
> 1423 module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000);
1424 MODULE_PARM_DESC(scrollback_persistent, "Enable persistent scrollback for all vga consoles");
1425 MODULE_LICENSE("GPL");
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, 2017-01-10 at 23:58 +0100, Adam Borowski wrote: > On Tue, Jan 10, 2017 at 10:28:38PM +0100, Manuel Schölling wrote: > > The impact of the persistent scrollback feature on the code size is > > rather small, so the config option is removed. The feature stays > > disabled by default and can be enabled by using the boot command > > line > > parameter 'vgacon.scrollback_persistent=1' or by setting > > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y. > > > > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> > > Suggested-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > +module_param_named(scrollback_persistent, scrollback_persistent, > > bool, 0000); > > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent > > scrollback for all vga consoles"); > > A command-line knob settable by the end-user is something more > persistent > than a config option. As you're going to extend this code beyond > vgacon in > the near future, perhaps it'd be better to have a shared setting for > all > console drivers? Probably a good idea, but I'm struggling with the implementation a bit: I tried to run if (strstr(boot_command_line, "nopersistentscrollback")) {...} in vgacon_scrollback_startup() but I am getting WARNING: modpost: Found 2 section mismatch(es). when compiling. Probably because vgacon_scrollback_startup() is executed after init. I tried to find another way to implement a boot cmd line parameter but had no luck. If you/somebody could point me in the right direction, it would be very much appreciated. Thanks! 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
On Tue, 2017-01-10 at 23:58 +0100, Adam Borowski wrote: > On Tue, Jan 10, 2017 at 10:28:38PM +0100, Manuel Schölling wrote: > > The impact of the persistent scrollback feature on the code size is > > rather small, so the config option is removed. The feature stays > > disabled by default and can be enabled by using the boot command > > line > > parameter 'vgacon.scrollback_persistent=1' or by setting > > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y. > > > > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> > > Suggested-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > +module_param_named(scrollback_persistent, scrollback_persistent, > > bool, 0000); > > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent > > scrollback for all vga consoles"); > > A command-line knob settable by the end-user is something more > persistent > than a config option. As you're going to extend this code beyond > vgacon in > the near future, perhaps it'd be better to have a shared setting for > all > console drivers? According to the guys at #kernelnewbies on IRC everybody hates new command line options. I'd rather stick to the module parameter for now and maybe introduce a new cmd line option later, once this feature has been implemented in several console drivers. 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
Changes in v10:
- Fix compilation error if CONFIG_VGACON_SOFT_SCROLLBACK=n
Changes in v9:
- Make persistent scrollback feature a boot parameter
Changes in v8:
- Add Reviewed-by/Tested-By statements
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 (4):
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
console: Make persistent scrollback a boot parameter
drivers/tty/vt/vt.c | 9 +++
drivers/video/console/Kconfig | 27 ++++++-
drivers/video/console/vgacon.c | 164 ++++++++++++++++++++++++++++-------------
include/linux/console.h | 4 +
4 files changed, 149 insertions(+), 55 deletions(-)
On Fri, Jan 13, 2017 at 09:00:34PM +0100, Manuel Schölling wrote: > On Tue, 2017-01-10 at 23:58 +0100, Adam Borowski wrote: > > On Tue, Jan 10, 2017 at 10:28:38PM +0100, Manuel Schölling wrote: > > > The impact of the persistent scrollback feature on the code size is > > > rather small, so the config option is removed. The feature stays > > > disabled by default and can be enabled by using the boot command > > > line > > > parameter 'vgacon.scrollback_persistent=1' or by setting > > > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y. > > > > > > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> > > > Suggested-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > > +module_param_named(scrollback_persistent, scrollback_persistent, > > > bool, 0000); > > > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent > > > scrollback for all vga consoles"); > > > > A command-line knob settable by the end-user is something more > > persistent > > than a config option. As you're going to extend this code beyond > > vgacon in > > the near future, perhaps it'd be better to have a shared setting for > > all > > console drivers? > According to the guys at #kernelnewbies on IRC everybody hates new > command line options. That was me, you can use my name here :) > I'd rather stick to the module parameter for now and maybe introduce a > new cmd line option later, once this feature has been implemented in > several console drivers. Yes, that should be fine. thanks, greg k-h -- 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
On Fri, Jan 13, 2017 at 09:07:54PM +0100, Manuel Schölling wrote: > Changes in v10: > - Fix compilation error if CONFIG_VGACON_SOFT_SCROLLBACK=n All now applied to my tty-testing branch, let's see what the 0-day bot has to say about this :) thanks for the persistence. greg k-h -- 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
On Tue, Jan 10, 2017 at 4:28 PM, Manuel Schölling <manuel.schoelling@gmx.de> wrote: > The impact of the persistent scrollback feature on the code size is > rather small, so the config option is removed. The feature stays > disabled by default and can be enabled by using the boot command line > parameter 'vgacon.scrollback_persistent=1' or by setting > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y. > > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> > Suggested-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- [...] > +module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000); > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent scrollback for all vga consoles"); Since this hasn't got widespread deployment yet and only exists in Greg's tree, can we please fix the above to use setup_param or similar, since there is nothing modular about this code at all. Thanks. Paul. -- 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
Hi Paul, On Thu, 2017-02-02 at 15:07 -0500, Paul Gortmaker wrote: > On Tue, Jan 10, 2017 at 4:28 PM, Manuel Schölling > <manuel.schoelling@gmx.de> wrote: > > The impact of the persistent scrollback feature on the code size is > > rather small, so the config option is removed. The feature stays > > disabled by default and can be enabled by using the boot command > > line > > parameter 'vgacon.scrollback_persistent=1' or by setting > > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y. > > > > Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> > > Suggested-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > --- > > [...] > > > +module_param_named(scrollback_persistent, scrollback_persistent, > > bool, 0000); > > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent > > scrollback for all vga consoles"); > > Since this hasn't got widespread deployment yet and only exists > in Greg's tree, can we please fix the above to use setup_param or > similar, since there is nothing modular about this code at all. Not sure what you mean here. If this is not the right may to declare it I'd be more than happy to change this. But I could not find any function/macro named setup_param [1]. It would be great if you could give me a hint what function to use here! Have a great weekend! Manuel [1] http://lxr.free-electrons.com/ident?i=setup_param -- 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
On Fri, Feb 03, 2017 at 05:04:15PM +0100, Manuel Schölling wrote: > On Thu, 2017-02-02 at 15:07 -0500, Paul Gortmaker wrote: > > On Tue, Jan 10, 2017 at 4:28 PM, Manuel Schölling > > <manuel.schoelling@gmx.de> wrote: > > > The impact of the persistent scrollback feature on the code size is > > > rather small, so the config option is removed. The feature stays > > > disabled by default and can be enabled by using the boot command > > > line > > > parameter 'vgacon.scrollback_persistent=1' or by setting > > > VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y. > > > > [...] > > > > > +module_param_named(scrollback_persistent, scrollback_persistent, > > > bool, 0000); > > > +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent > > > scrollback for all vga consoles"); > > > > Since this hasn't got widespread deployment yet and only exists > > in Greg's tree, can we please fix the above to use setup_param or > > similar, since there is nothing modular about this code at all. > Not sure what you mean here. > If this is not the right may to declare it I'd be more than happy to > change this. But I could not find any function/macro named setup_param > [1]. > It would be great if you could give me a hint what function to use > here! > > [1] http://lxr.free-electrons.com/ident?i=setup_param That shows only exact matches. You want "git grep setup_param", which shows __setup_param() plus some unrelated stuff. I see only four uses in the kernel, but that's enough to see how to use it. Meow!
diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig index f500e58f7636..5b71bd905a60 100644 --- a/drivers/video/console/Kconfig +++ b/drivers/video/console/Kconfig @@ -47,14 +47,16 @@ config VGACON_SOFT_SCROLLBACK_SIZE 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" +config VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT + bool "Persistent Scrollback History for each console by default" 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. + Say Y here if the scrollback history should persist by default when + switching between consoles. Otherwise, the scrollback history will be + flushed each time the console is switched. This feature can also be + enabled using the boot command line parameter + 'vgacon.scrollback_persistent=1'. 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) diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c index ca23d222e029..45a76972495b 100644 --- a/drivers/video/console/vgacon.c +++ b/drivers/video/console/vgacon.c @@ -174,11 +174,9 @@ struct vgacon_scrollback_info { }; 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 bool scrollback_persistent = \ + IS_ENABLED(CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT); static void vgacon_scrollback_reset(int vc_num, size_t reset_size) { @@ -213,20 +211,19 @@ static void vgacon_scrollback_init(int vc_num) static void vgacon_scrollback_switch(int vc_num) { -#ifndef CONFIG_VGACON_SOFT_SCROLLBACK_PERSISTENT - vc_num = 0; -#endif + if (!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; + if (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 + vgacon_scrollback_reset(vc_num, size); + } } } @@ -1423,4 +1420,6 @@ const struct consw vga_con = { }; EXPORT_SYMBOL(vga_con); +module_param_named(scrollback_persistent, scrollback_persistent, bool, 0000); +MODULE_PARM_DESC(scrollback_persistent, "Enable persistent scrollback for all vga consoles"); MODULE_LICENSE("GPL");
The impact of the persistent scrollback feature on the code size is rather small, so the config option is removed. The feature stays disabled by default and can be enabled by using the boot command line parameter 'vgacon.scrollback_persistent=1' or by setting VGACON_SOFT_SCROLLBACK_PERSISTENT_ENABLE_BY_DEFAULT=y. Signed-off-by: Manuel Schölling <manuel.schoelling@gmx.de> Suggested-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- drivers/video/console/Kconfig | 12 +++++++----- drivers/video/console/vgacon.c | 25 ++++++++++++------------- 2 files changed, 19 insertions(+), 18 deletions(-)