[PATCH RESEND] vgacon: remember scrollback buffer on console switch - Kernel

This is a discussion on [PATCH RESEND] vgacon: remember scrollback buffer on console switch - Kernel ; Add support for persistent console history, surviving console switches. It allocates new scrollback buffer only when user switches console for the first time. Signed-off-by: Marcin Slusarz Cc: Antonino Daplas Cc: Krzysztof Helt Cc: Andrew Morton Cc: linux-fbdev-devel@lists.sourceforge.net --- drivers/video/console/Kconfig | ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

  1. [PATCH RESEND] vgacon: remember scrollback buffer on console switch

    Add support for persistent console history, surviving
    console switches. It allocates new scrollback buffer only when
    user switches console for the first time.

    Signed-off-by: Marcin Slusarz
    Cc: Antonino Daplas
    Cc: Krzysztof Helt
    Cc: Andrew Morton
    Cc: linux-fbdev-devel@lists.sourceforge.net
    ---
    drivers/video/console/Kconfig | 11 ++++++
    drivers/video/console/vgacon.c | 75 +++++++++++++++++++++++++++++++++++++--
    2 files changed, 82 insertions(+), 4 deletions(-)

    diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
    index 2f50a80..09e3d98 100644
    --- a/drivers/video/console/Kconfig
    +++ b/drivers/video/console/Kconfig
    @@ -43,6 +43,17 @@ config VGACON_SOFT_SCROLLBACK_SIZE
    buffer. Each 64KB will give you approximately 16 80x25
    screenfuls of scrollback buffer

    +config VGACON_REMEMBER_SCROLLBACK
    + bool "Remember scrollback buffer on console switch"
    + depends on VGACON_SOFT_SCROLLBACK
    + default y
    + help
    + Say 'Y' here if you want the scrollback buffer to be remembered
    + on console switch and restored when you switch back.
    +
    + Note: every VGA console will use its own buffer, but it will be
    + allocated only when you switch to this console for the first time.
    +
    config MDA_CONSOLE
    depends on !M68K && !PARISC && ISA
    tristate "MDA text console (dual-headed) (EXPERIMENTAL)"
    diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
    index 448d209..15ee7e1 100644
    --- a/drivers/video/console/vgacon.c
    +++ b/drivers/video/console/vgacon.c
    @@ -174,9 +174,11 @@ static int vgacon_scrollback_cur;
    static int vgacon_scrollback_save;
    static int vgacon_scrollback_restore;

    +#define SCROLLBACK_SIZE (CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024)
    +
    static void vgacon_scrollback_init(int pitch)
    {
    - int rows = CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024/pitch;
    + int rows = SCROLLBACK_SIZE / pitch;

    if (vgacon_scrollback) {
    vgacon_scrollback_cnt = 0;
    @@ -187,15 +189,76 @@ static void vgacon_scrollback_init(int pitch)
    }
    }

    +#ifdef CONFIG_VGACON_REMEMBER_SCROLLBACK
    +static struct vgacon_scrollback_info {
    + void *data;
    + int cnt;
    + int tail;
    + int cur;
    + int rows;
    + int size;
    +} vgacon_scrollbacks[MAX_NR_CONSOLES];
    +static int vgacon_last_vc_num;
    +
    +static void vgacon_switch_scrollback(struct vc_data *c)
    +{
    + int num = c->vc_num;
    + struct vgacon_scrollback_info *old_scrollback =
    + &vgacon_scrollbacks[vgacon_last_vc_num];
    + struct vgacon_scrollback_info *new_scrollback =
    + &vgacon_scrollbacks[num];
    +
    + old_scrollback->cnt = vgacon_scrollback_cnt;
    + old_scrollback->tail = vgacon_scrollback_tail;
    + old_scrollback->cur = vgacon_scrollback_cur;
    + old_scrollback->rows = vgacon_scrollback_rows;
    + old_scrollback->size = vgacon_scrollback_size;
    +
    + if (!new_scrollback->data) {
    + int rows = SCROLLBACK_SIZE / c->vc_size_row;
    +
    + new_scrollback->data = kmalloc(SCROLLBACK_SIZE, GFP_KERNEL);
    + new_scrollback->cnt = 0;
    + new_scrollback->tail = 0;
    + new_scrollback->cur = 0;
    + new_scrollback->rows = rows - 1;
    + new_scrollback->size = rows * c->vc_size_row;
    +
    + if (!new_scrollback->data) {
    + printk(KERN_WARNING "VGAcon: failed to allocate memory for scrollback of console %d, using scrollback of console %d.\n",
    + num, vgacon_last_vc_num);
    + new_scrollback->data = old_scrollback->data;
    + old_scrollback->data = NULL;
    + }
    + }
    +
    + vgacon_scrollback = new_scrollback->data;
    + vgacon_scrollback_cnt = new_scrollback->cnt;
    + vgacon_scrollback_tail = new_scrollback->tail;
    + vgacon_scrollback_cur = new_scrollback->cur;
    + vgacon_scrollback_rows = new_scrollback->rows;
    + vgacon_scrollback_size = new_scrollback->size;
    +
    + vgacon_last_vc_num = num;
    +}
    +#else
    +static inline void vgacon_switch_scrollback(struct vc_data *c)
    +{
    + vgacon_scrollback_init(c->vc_size_row);
    +}
    +#endif
    /*
    * Called only duing init so call of alloc_bootmen is ok.
    * Marked __init_refok to silence modpost.
    */
    static void __init_refok vgacon_scrollback_startup(void)
    {
    - vgacon_scrollback = alloc_bootmem(CONFIG_VGACON_SOFT_SCROLLBACK_SIZE
    - * 1024);
    + vgacon_scrollback = alloc_bootmem(SCROLLBACK_SIZE);
    vgacon_scrollback_init(vga_video_num_columns * 2);
    +#ifdef CONFIG_VGACON_REMEMBER_SCROLLBACK
    + vgacon_scrollbacks[0].data = vgacon_scrollback;
    + vgacon_last_vc_num = 0;
    +#endif
    }

    static void vgacon_scrollback_update(struct vc_data *c, int t, int count)
    @@ -317,6 +380,10 @@ static int vgacon_scrolldelta(struct vc_data *c, int lines)
    #define vgacon_scrollback_init(...) do { } while (0)
    #define vgacon_scrollback_update(...) do { } while (0)

    +static inline void vgacon_switch_scrollback(struct vc_data *c)
    +{
    +}
    +
    static void vgacon_restore_screen(struct vc_data *c)
    {
    if (c->vc_origin != c->vc_visible_origin)
    @@ -823,7 +890,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);
    return 0; /* Redrawing not needed */
    }

    --
    1.5.6.4

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

    On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz wrote:

    > Add support for persistent console history, surviving
    > console switches. It allocates new scrollback buffer only when
    > user switches console for the first time.
    >
    > Signed-off-by: Marcin Slusarz
    > Cc: Antonino Daplas
    > Cc: Krzysztof Helt
    > Cc: Andrew Morton
    > Cc: linux-fbdev-devel@lists.sourceforge.net
    > ---
    > drivers/video/console/Kconfig | 11 ++++++
    > drivers/video/console/vgacon.c | 75 +++++++++++++++++++++++++++++++++++++--
    > 2 files changed, 82 insertions(+), 4 deletions(-)
    >
    > diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
    > index 2f50a80..09e3d98 100644
    > --- a/drivers/video/console/Kconfig
    > +++ b/drivers/video/console/Kconfig
    > @@ -43,6 +43,17 @@ config VGACON_SOFT_SCROLLBACK_SIZE
    > buffer. Each 64KB will give you approximately 16 80x25
    > screenfuls of scrollback buffer
    >
    > +config VGACON_REMEMBER_SCROLLBACK
    > + bool "Remember scrollback buffer on console switch"
    > + depends on VGACON_SOFT_SCROLLBACK
    > + default y
    > + help
    > + Say 'Y' here if you want the scrollback buffer to be remembered
    > + on console switch and restored when you switch back.
    > +
    > + Note: every VGA console will use its own buffer, but it will be
    > + allocated only when you switch to this console for the first time.


    I'd question the value in adding the config option. Why not make the
    feature unconditionally present?

    >
    > +#define SCROLLBACK_SIZE (CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024)
    > +
    > ...
    >
    > +#ifdef CONFIG_VGACON_REMEMBER_SCROLLBACK
    > +static struct vgacon_scrollback_info {
    > + void *data;
    > + int cnt;
    > + int tail;
    > + int cur;
    > + int rows;
    > + int size;
    > +} vgacon_scrollbacks[MAX_NR_CONSOLES];


    Perhaps you were concerned about memory consumption?

    If so, it would be much much better to make this feature switchable at
    runtime (module parameter/boot option or a /proc or /sys knob).

    > +static int vgacon_last_vc_num;


    We have lots of global state here with no apparent locking protecting
    it. Possibly there's some higher-level lock which provides
    seralisation? If so, the addition of a comment explaining all
    this would be good.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

    On Sat, Oct 25, 2008 at 01:46:15PM -0700, Andrew Morton wrote:
    > On Sat, 25 Oct 2008 21:58:19 +0200 Marcin Slusarz wrote:
    >
    > > Add support for persistent console history, surviving
    > > console switches. It allocates new scrollback buffer only when
    > > user switches console for the first time.
    > >
    > > Signed-off-by: Marcin Slusarz
    > > Cc: Antonino Daplas
    > > Cc: Krzysztof Helt
    > > Cc: Andrew Morton
    > > Cc: linux-fbdev-devel@lists.sourceforge.net
    > > ---
    > > drivers/video/console/Kconfig | 11 ++++++
    > > drivers/video/console/vgacon.c | 75 +++++++++++++++++++++++++++++++++++++--
    > > 2 files changed, 82 insertions(+), 4 deletions(-)
    > >
    > > diff --git a/drivers/video/console/Kconfig b/drivers/video/console/Kconfig
    > > index 2f50a80..09e3d98 100644
    > > --- a/drivers/video/console/Kconfig
    > > +++ b/drivers/video/console/Kconfig
    > > @@ -43,6 +43,17 @@ config VGACON_SOFT_SCROLLBACK_SIZE
    > > buffer. Each 64KB will give you approximately 16 80x25
    > > screenfuls of scrollback buffer
    > >
    > > +config VGACON_REMEMBER_SCROLLBACK
    > > + bool "Remember scrollback buffer on console switch"
    > > + depends on VGACON_SOFT_SCROLLBACK
    > > + default y
    > > + help
    > > + Say 'Y' here if you want the scrollback buffer to be remembered
    > > + on console switch and restored when you switch back.
    > > +
    > > + Note: every VGA console will use its own buffer, but it will be
    > > + allocated only when you switch to this console for the first time.

    >
    > I'd question the value in adding the config option. Why not make the
    > feature unconditionally present?
    >
    > >
    > > +#define SCROLLBACK_SIZE (CONFIG_VGACON_SOFT_SCROLLBACK_SIZE * 1024)
    > > +
    > > ...
    > >
    > > +#ifdef CONFIG_VGACON_REMEMBER_SCROLLBACK
    > > +static struct vgacon_scrollback_info {
    > > + void *data;
    > > + int cnt;
    > > + int tail;
    > > + int cur;
    > > + int rows;
    > > + int size;
    > > +} vgacon_scrollbacks[MAX_NR_CONSOLES];

    >
    > Perhaps you were concerned about memory consumption?


    Yes. I could imagine scenario where this memory would be considered "wasted".

    > If so, it would be much much better to make this feature switchable at
    > runtime (module parameter/boot option or a /proc or /sys knob).


    /sys knob seems to be the most flexible option.

    /sys/class/vtconsole/vtconX/persistent_history? 0/1

    > > +static int vgacon_last_vc_num;

    >
    > We have lots of global state here with no apparent locking protecting
    > it. Possibly there's some higher-level lock which provides
    > seralisation? If so, the addition of a comment explaining all
    > this would be good.


    I checked it and this code is called under console_sem.

    vgacon_switch_scrollback <- vgacon_switch <- con_switch <- redraw_screen <- switch_screen <- complete_change_console <-
    1) vt_ioctl (calls acquire_console_sem before complete_change_console)
    2) change_console <- console_callback (calls acquire_console_sem before change_console)

    Thanks for a review!

    PS: why DECLARE_MUTEX _defines_ _semaphore_? there are only 8 uses of this
    macro so it's not a big problem to rename it to e.g. DEFINE_SEMAPHORE (I can
    provide a patch)

    Marcin
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

    On Sun, 26 Oct 2008 00:43:01 +0200 Marcin Slusarz wrote:

    > PS: why DECLARE_MUTEX _defines_ _semaphore_?


    The kernel gets definition-vs-declaration confused in several places.

    > there are only 8 uses of this
    > macro so it's not a big problem to rename it to e.g. DEFINE_SEMAPHORE (I can
    > provide a patch)


    I'd say that s/declare/define/ at such a late stage in the semaphore's
    lifetime would be of dubious value. But getting "MUTEX" out of that
    macro's name would be a very good thing - it's a bad overlap with struct
    mutex. Send patch


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH RESEND] vgacon: remember scrollback buffer on console switch

    Hi!

    > > If so, it would be much much better to make this feature switchable at
    > > runtime (module parameter/boot option or a /proc or /sys knob).

    >
    > /sys knob seems to be the most flexible option.
    >
    > /sys/class/vtconsole/vtconX/persistent_history? 0/1


    0/number-of-lines-to-remember?
    Pavel

    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pav...rses/blog.html
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread