Date: Wed, 16 May 2018 09:01:02 +0000 (UTC) From: =?UTF-8?Q?Jean-S=c3=a9bastien_P=c3=a9dron?= <dumbbell@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r333669 - in head/sys: dev/vt kern sys teken Message-ID: <201805160901.w4G912FD056132@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: dumbbell Date: Wed May 16 09:01:02 2018 New Revision: 333669 URL: https://svnweb.freebsd.org/changeset/base/333669 Log: teken, vt(4): New callbacks to lock the terminal once ... to process input, instead of inside each smaller operations such as appending a character or moving the cursor forward. In other words, before we were doing (oversimplified): teken_input() <for each input character> vtterm_putchar() VTBUF_LOCK() VTBUF_UNLOCK() vtterm_cursor_position() VTBUF_LOCK() VTBUF_UNLOCK() Now, we are doing: vtterm_pre_input() VTBUF_LOCK() teken_input() <for each input character> vtterm_putchar() vtterm_cursor_position() vtterm_post_input() VTBUF_UNLOCK() The situation was even worse when the vtterm_copy() and vtterm_fill() callbacks were involved. The new callbacks are: * struct terminal_class->tc_pre_input() * struct terminal_class->tc_post_input() They are called in teken_input(), surrounding the while() loop. The goal is to improve input processing speed of vt(4). As a benchmark, here is the time taken to write a text file of 360 000 lines (26 MiB) on `ttyv0`: * vt(4), unmodified: 1500 ms * vt(4), with this patch: 1200 ms * syscons(4): 700 ms This is on a Haswell laptop with a GENERIC-NODEBUG kernel. At the same time, the locking is changed in the vt_flush() function which is responsible to draw the text on screen. So instead of (indirectly) using VTBUF_LOCK() just to read and reset the dirty area of the internal buffer, the lock is held for about the entire function, including the drawing part. The change is mostly visible while content is scrolling fast: before, lines could appear garbled while scrolling because the internal buffer was accessed without locks (once the scrolling was finished, the output was correct). Now, the scrolling appears correct. In the end, the locking model is closer to what syscons(4) does. Differential Revision: https://reviews.freebsd.org/D15302 Modified: head/sys/dev/vt/vt.h head/sys/dev/vt/vt_buf.c head/sys/dev/vt/vt_core.c head/sys/kern/subr_terminal.c head/sys/sys/terminal.h head/sys/teken/teken.c head/sys/teken/teken.h Modified: head/sys/dev/vt/vt.h ============================================================================== --- head/sys/dev/vt/vt.h Wed May 16 08:43:08 2018 (r333668) +++ head/sys/dev/vt/vt.h Wed May 16 09:01:02 2018 (r333669) @@ -213,8 +213,10 @@ struct vt_buf { #define VBF_DEFAULT_HISTORY_SIZE 500 #endif +void vtbuf_lock(struct vt_buf *); +void vtbuf_unlock(struct vt_buf *); void vtbuf_copy(struct vt_buf *, const term_rect_t *, const term_pos_t *); -void vtbuf_fill_locked(struct vt_buf *, const term_rect_t *, term_char_t); +void vtbuf_fill(struct vt_buf *, const term_rect_t *, term_char_t); void vtbuf_init_early(struct vt_buf *); void vtbuf_init(struct vt_buf *, const term_pos_t *); void vtbuf_grow(struct vt_buf *, const term_pos_t *, unsigned int); Modified: head/sys/dev/vt/vt_buf.c ============================================================================== --- head/sys/dev/vt/vt_buf.c Wed May 16 08:43:08 2018 (r333668) +++ head/sys/dev/vt/vt_buf.c Wed May 16 09:01:02 2018 (r333669) @@ -253,10 +253,24 @@ vtbuf_iscursor(const struct vt_buf *vb, int row, int c return (0); } -static inline void -vtbuf_dirty_locked(struct vt_buf *vb, const term_rect_t *area) +void +vtbuf_lock(struct vt_buf *vb) { + VTBUF_LOCK(vb); +} + +void +vtbuf_unlock(struct vt_buf *vb) +{ + + VTBUF_UNLOCK(vb); +} + +void +vtbuf_dirty(struct vt_buf *vb, const term_rect_t *area) +{ + if (vb->vb_dirtyrect.tr_begin.tp_row > area->tr_begin.tp_row) vb->vb_dirtyrect.tr_begin.tp_row = area->tr_begin.tp_row; if (vb->vb_dirtyrect.tr_begin.tp_col > area->tr_begin.tp_col) @@ -267,24 +281,15 @@ vtbuf_dirty_locked(struct vt_buf *vb, const term_rect_ vb->vb_dirtyrect.tr_end.tp_col = area->tr_end.tp_col; } -void -vtbuf_dirty(struct vt_buf *vb, const term_rect_t *area) -{ - - VTBUF_LOCK(vb); - vtbuf_dirty_locked(vb, area); - VTBUF_UNLOCK(vb); -} - static inline void -vtbuf_dirty_cell_locked(struct vt_buf *vb, const term_pos_t *p) +vtbuf_dirty_cell(struct vt_buf *vb, const term_pos_t *p) { term_rect_t area; area.tr_begin = *p; area.tr_end.tp_row = p->tp_row + 1; area.tr_end.tp_col = p->tp_col + 1; - vtbuf_dirty_locked(vb, &area); + vtbuf_dirty(vb, &area); } static void @@ -299,10 +304,8 @@ void vtbuf_undirty(struct vt_buf *vb, term_rect_t *r) { - VTBUF_LOCK(vb); *r = vb->vb_dirtyrect; vtbuf_make_undirty(vb); - VTBUF_UNLOCK(vb); } void @@ -366,7 +369,7 @@ vtbuf_copy(struct vt_buf *vb, const term_rect_t *r, co } static void -vtbuf_fill(struct vt_buf *vb, const term_rect_t *r, term_char_t c) +vtbuf_do_fill(struct vt_buf *vb, const term_rect_t *r, term_char_t c) { unsigned int pr, pc; term_char_t *row; @@ -381,26 +384,25 @@ vtbuf_fill(struct vt_buf *vb, const term_rect_t *r, te } void -vtbuf_fill_locked(struct vt_buf *vb, const term_rect_t *r, term_char_t c) +vtbuf_fill(struct vt_buf *vb, const term_rect_t *r, term_char_t c) { + KASSERT(r->tr_begin.tp_row < vb->vb_scr_size.tp_row, - ("vtbuf_fill_locked begin.tp_row %d must be < screen height %d", + ("vtbuf_fill begin.tp_row %d must be < screen height %d", r->tr_begin.tp_row, vb->vb_scr_size.tp_row)); KASSERT(r->tr_begin.tp_col < vb->vb_scr_size.tp_col, - ("vtbuf_fill_locked begin.tp_col %d must be < screen width %d", + ("vtbuf_fill begin.tp_col %d must be < screen width %d", r->tr_begin.tp_col, vb->vb_scr_size.tp_col)); KASSERT(r->tr_end.tp_row <= vb->vb_scr_size.tp_row, - ("vtbuf_fill_locked end.tp_row %d must be <= screen height %d", + ("vtbuf_fill end.tp_row %d must be <= screen height %d", r->tr_end.tp_row, vb->vb_scr_size.tp_row)); KASSERT(r->tr_end.tp_col <= vb->vb_scr_size.tp_col, - ("vtbuf_fill_locked end.tp_col %d must be <= screen width %d", + ("vtbuf_fill end.tp_col %d must be <= screen width %d", r->tr_end.tp_col, vb->vb_scr_size.tp_col)); - VTBUF_LOCK(vb); - vtbuf_fill(vb, r, c); - vtbuf_dirty_locked(vb, r); - VTBUF_UNLOCK(vb); + vtbuf_do_fill(vb, r, c); + vtbuf_dirty(vb, r); } static void @@ -431,7 +433,7 @@ vtbuf_init_early(struct vt_buf *vb) rect.tr_begin.tp_row = rect.tr_begin.tp_col = 0; rect.tr_end.tp_col = vb->vb_scr_size.tp_col; rect.tr_end.tp_row = vb->vb_history_size; - vtbuf_fill(vb, &rect, VTBUF_SPACE_CHAR(TERMINAL_NORM_ATTR)); + vtbuf_do_fill(vb, &rect, VTBUF_SPACE_CHAR(TERMINAL_NORM_ATTR)); vtbuf_make_undirty(vb); if ((vb->vb_flags & VBF_MTX_INIT) == 0) { mtx_init(&vb->vb_lock, "vtbuf", NULL, MTX_SPIN); @@ -645,23 +647,18 @@ vtbuf_putchar(struct vt_buf *vb, const term_pos_t *p, row = vb->vb_rows[(vb->vb_curroffset + p->tp_row) % VTBUF_MAX_HEIGHT(vb)]; if (row[p->tp_col] != c) { - VTBUF_LOCK(vb); row[p->tp_col] = c; - vtbuf_dirty_cell_locked(vb, p); - VTBUF_UNLOCK(vb); + vtbuf_dirty_cell(vb, p); } } void vtbuf_cursor_position(struct vt_buf *vb, const term_pos_t *p) { - if (vb->vb_flags & VBF_CURSOR) { - VTBUF_LOCK(vb); - vtbuf_dirty_cell_locked(vb, &vb->vb_cursor); + vtbuf_dirty_cell(vb, &vb->vb_cursor); vb->vb_cursor = *p; - vtbuf_dirty_cell_locked(vb, &vb->vb_cursor); - VTBUF_UNLOCK(vb); + vtbuf_dirty_cell(vb, &vb->vb_cursor); } else { vb->vb_cursor = *p; } @@ -687,7 +684,9 @@ vtbuf_flush_mark(struct vt_buf *vb) area.tr_end.tp_col = vb->vb_scr_size.tp_col; area.tr_end.tp_row = MAX(s, e) + 1; + VTBUF_LOCK(vb); vtbuf_dirty(vb, &area); + VTBUF_UNLOCK(vb); } } @@ -823,7 +822,6 @@ vtbuf_cursor_visibility(struct vt_buf *vb, int yes) { int oflags, nflags; - VTBUF_LOCK(vb); oflags = vb->vb_flags; if (yes) vb->vb_flags |= VBF_CURSOR; @@ -832,8 +830,7 @@ vtbuf_cursor_visibility(struct vt_buf *vb, int yes) nflags = vb->vb_flags; if (oflags != nflags) - vtbuf_dirty_cell_locked(vb, &vb->vb_cursor); - VTBUF_UNLOCK(vb); + vtbuf_dirty_cell(vb, &vb->vb_cursor); } void @@ -850,7 +847,6 @@ vtbuf_scroll_mode(struct vt_buf *vb, int yes) nflags = vb->vb_flags; if (oflags != nflags) - vtbuf_dirty_cell_locked(vb, &vb->vb_cursor); + vtbuf_dirty_cell(vb, &vb->vb_cursor); VTBUF_UNLOCK(vb); } - Modified: head/sys/dev/vt/vt_core.c ============================================================================== --- head/sys/dev/vt/vt_core.c Wed May 16 08:43:08 2018 (r333668) +++ head/sys/dev/vt/vt_core.c Wed May 16 09:01:02 2018 (r333669) @@ -66,6 +66,8 @@ static tc_cursor_t vtterm_cursor; static tc_putchar_t vtterm_putchar; static tc_fill_t vtterm_fill; static tc_copy_t vtterm_copy; +static tc_pre_input_t vtterm_pre_input; +static tc_post_input_t vtterm_post_input; static tc_param_t vtterm_param; static tc_done_t vtterm_done; @@ -85,6 +87,8 @@ const struct terminal_class vt_termclass = { .tc_putchar = vtterm_putchar, .tc_fill = vtterm_fill, .tc_copy = vtterm_copy, + .tc_pre_input = vtterm_pre_input, + .tc_post_input = vtterm_post_input, .tc_param = vtterm_param, .tc_done = vtterm_done, @@ -1053,7 +1057,7 @@ vtterm_fill(struct terminal *tm, const term_rect_t *r, { struct vt_window *vw = tm->tm_softc; - vtbuf_fill_locked(&vw->vw_buf, r, c); + vtbuf_fill(&vw->vw_buf, r, c); vt_resume_flush_timer(vw->vw_device, 0); } @@ -1142,7 +1146,7 @@ vt_is_cursor_in_area(const struct vt_device *vd, const } static void -vt_mark_mouse_position_as_dirty(struct vt_device *vd) +vt_mark_mouse_position_as_dirty(struct vt_device *vd, int locked) { term_rect_t area; struct vt_window *vw; @@ -1175,7 +1179,11 @@ vt_mark_mouse_position_as_dirty(struct vt_device *vd) area.tr_end.tp_row = y + 2; } + if (!locked) + vtbuf_lock(&vw->vw_buf); vtbuf_dirty(&vw->vw_buf, &area); + if (!locked) + vtbuf_unlock(&vw->vw_buf); } #endif @@ -1230,6 +1238,8 @@ vt_flush(struct vt_device *vd) if (((vd->vd_flags & VDF_TEXTMODE) == 0) && (vf == NULL)) return (0); + vtbuf_lock(&vw->vw_buf); + #ifndef SC_NO_CUTPASTE cursor_was_shown = vd->vd_mshown; cursor_moved = (vd->vd_mx != vd->vd_mx_drawn || @@ -1250,7 +1260,7 @@ vt_flush(struct vt_device *vd) */ if (cursor_was_shown != vd->vd_mshown || (vd->vd_mshown && cursor_moved)) - vt_mark_mouse_position_as_dirty(vd); + vt_mark_mouse_position_as_dirty(vd, true); /* * Save position of the mouse cursor. It's used by backends to @@ -1265,7 +1275,7 @@ vt_flush(struct vt_device *vd) * mark the new position as dirty. */ if (vd->vd_mshown && cursor_moved) - vt_mark_mouse_position_as_dirty(vd); + vt_mark_mouse_position_as_dirty(vd, true); #endif vtbuf_undirty(&vw->vw_buf, &tarea); @@ -1282,9 +1292,11 @@ vt_flush(struct vt_device *vd) if (tarea.tr_begin.tp_col < tarea.tr_end.tp_col) { vd->vd_driver->vd_bitblt_text(vd, vw, &tarea); + vtbuf_unlock(&vw->vw_buf); return (1); } + vtbuf_unlock(&vw->vw_buf); return (0); } @@ -1306,6 +1318,23 @@ vt_timer(void *arg) } static void +vtterm_pre_input(struct terminal *tm) +{ + struct vt_window *vw = tm->tm_softc; + + vtbuf_lock(&vw->vw_buf); +} + +static void +vtterm_post_input(struct terminal *tm) +{ + struct vt_window *vw = tm->tm_softc; + + vtbuf_unlock(&vw->vw_buf); + vt_resume_flush_timer(vw->vw_device, 0); +} + +static void vtterm_done(struct terminal *tm) { struct vt_window *vw = tm->tm_softc; @@ -2000,7 +2029,7 @@ vt_mouse_state(int show) } /* Mark mouse position as dirty. */ - vt_mark_mouse_position_as_dirty(vd); + vt_mark_mouse_position_as_dirty(vd, false); vt_resume_flush_timer(vw->vw_device, 0); } #endif Modified: head/sys/kern/subr_terminal.c ============================================================================== --- head/sys/kern/subr_terminal.c Wed May 16 08:43:08 2018 (r333668) +++ head/sys/kern/subr_terminal.c Wed May 16 09:01:02 2018 (r333669) @@ -106,6 +106,8 @@ static tf_cursor_t termteken_cursor; static tf_putchar_t termteken_putchar; static tf_fill_t termteken_fill; static tf_copy_t termteken_copy; +static tf_pre_input_t termteken_pre_input; +static tf_post_input_t termteken_post_input; static tf_param_t termteken_param; static tf_respond_t termteken_respond; @@ -115,6 +117,8 @@ static teken_funcs_t terminal_drawmethods = { .tf_putchar = termteken_putchar, .tf_fill = termteken_fill, .tf_copy = termteken_copy, + .tf_pre_input = termteken_pre_input, + .tf_post_input = termteken_post_input, .tf_param = termteken_param, .tf_respond = termteken_respond, }; @@ -624,6 +628,22 @@ termteken_copy(void *softc, const teken_rect_t *r, con struct terminal *tm = softc; tm->tm_class->tc_copy(tm, r, p); +} + +static void +termteken_pre_input(void *softc) +{ + struct terminal *tm = softc; + + tm->tm_class->tc_pre_input(tm); +} + +static void +termteken_post_input(void *softc) +{ + struct terminal *tm = softc; + + tm->tm_class->tc_post_input(tm); } static void Modified: head/sys/sys/terminal.h ============================================================================== --- head/sys/sys/terminal.h Wed May 16 08:43:08 2018 (r333668) +++ head/sys/sys/terminal.h Wed May 16 09:01:02 2018 (r333669) @@ -151,6 +151,8 @@ typedef void tc_fill_t(struct terminal *tm, const term term_char_t c); typedef void tc_copy_t(struct terminal *tm, const term_rect_t *r, const term_pos_t *p); +typedef void tc_pre_input_t(struct terminal *tm); +typedef void tc_post_input_t(struct terminal *tm); typedef void tc_param_t(struct terminal *tm, int cmd, unsigned int arg); typedef void tc_done_t(struct terminal *tm); @@ -173,6 +175,8 @@ struct terminal_class { tc_putchar_t *tc_putchar; tc_fill_t *tc_fill; tc_copy_t *tc_copy; + tc_pre_input_t *tc_pre_input; + tc_post_input_t *tc_post_input; tc_param_t *tc_param; tc_done_t *tc_done; Modified: head/sys/teken/teken.c ============================================================================== --- head/sys/teken/teken.c Wed May 16 08:43:08 2018 (r333668) +++ head/sys/teken/teken.c Wed May 16 09:01:02 2018 (r333669) @@ -133,6 +133,22 @@ teken_funcs_copy(const teken_t *t, const teken_rect_t } static inline void +teken_funcs_pre_input(const teken_t *t) +{ + + teken_assert(t->t_funcs->tf_pre_input != NULL); + t->t_funcs->tf_pre_input(t->t_softc); +} + +static inline void +teken_funcs_post_input(const teken_t *t) +{ + + teken_assert(t->t_funcs->tf_post_input != NULL); + t->t_funcs->tf_post_input(t->t_softc); +} + +static inline void teken_funcs_param(const teken_t *t, int cmd, unsigned int value) { @@ -292,8 +308,10 @@ teken_input(teken_t *t, const void *buf, size_t len) { const char *c = buf; + teken_funcs_pre_input(t); while (len-- > 0) teken_input_byte(t, *c++); + teken_funcs_post_input(t); } const teken_pos_t * Modified: head/sys/teken/teken.h ============================================================================== --- head/sys/teken/teken.h Wed May 16 08:43:08 2018 (r333668) +++ head/sys/teken/teken.h Wed May 16 09:01:02 2018 (r333669) @@ -93,6 +93,8 @@ typedef void tf_putchar_t(void *, const teken_pos_t *, typedef void tf_fill_t(void *, const teken_rect_t *, teken_char_t, const teken_attr_t *); typedef void tf_copy_t(void *, const teken_rect_t *, const teken_pos_t *); +typedef void tf_pre_input_t(void *); +typedef void tf_post_input_t(void *); typedef void tf_param_t(void *, int, unsigned int); #define TP_SHOWCURSOR 0 #define TP_KEYPADAPP 1 @@ -114,6 +116,8 @@ typedef struct { tf_putchar_t *tf_putchar; tf_fill_t *tf_fill; tf_copy_t *tf_copy; + tf_pre_input_t *tf_pre_input; + tf_post_input_t *tf_post_input; tf_param_t *tf_param; tf_respond_t *tf_respond; } teken_funcs_t;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201805160901.w4G912FD056132>