From ae1ca6fd73256507ef9094b93fec5b9760349c20 Mon Sep 17 00:00:00 2001 From: scottl Date: Wed, 13 Sep 2006 15:48:15 +0000 Subject: [PATCH] Introduce a spinlock for synchronizing access to the video output hardware in syscons. This replaces a simple access semaphore that was assumed to be protected by Giant but often was not. If two threads that were otherwise SMP-safe called printf at the same time, there was a high likelyhood that the semaphore would get corrupted and result in a permanently frozen video console. This is similar to what is already done in the serial console drivers. --- sys/dev/syscons/scmouse.c | 8 ++++---- sys/dev/syscons/syscons.c | 31 +++++++++++++++++-------------- sys/dev/syscons/syscons.h | 18 +++++++++++++++++- sys/kern/subr_witness.c | 1 + 4 files changed, 39 insertions(+), 19 deletions(-) diff --git a/sys/dev/syscons/scmouse.c b/sys/dev/syscons/scmouse.c index 73455f315884..54ca2ec49aa8 100644 --- a/sys/dev/syscons/scmouse.c +++ b/sys/dev/syscons/scmouse.c @@ -178,13 +178,13 @@ sc_draw_mouse_image(scr_stat *scp) if (ISGRAPHSC(scp)) return; - ++scp->sc->videoio_in_progress; + SC_VIDEO_LOCK(scp->sc); (*scp->rndr->draw_mouse)(scp, scp->mouse_xpos, scp->mouse_ypos, TRUE); scp->mouse_oldpos = scp->mouse_pos; scp->mouse_oldxpos = scp->mouse_xpos; scp->mouse_oldypos = scp->mouse_ypos; scp->status |= MOUSE_VISIBLE; - --scp->sc->videoio_in_progress; + SC_VIDEO_UNLOCK(scp->sc); } void @@ -196,7 +196,7 @@ sc_remove_mouse_image(scr_stat *scp) if (ISGRAPHSC(scp)) return; - ++scp->sc->videoio_in_progress; + SC_VIDEO_LOCK(scp->sc); (*scp->rndr->draw_mouse)(scp, (scp->mouse_oldpos%scp->xsize + scp->xoff) * scp->font_width, @@ -217,7 +217,7 @@ sc_remove_mouse_image(scr_stat *scp) } #endif /* PC98 */ scp->status &= ~MOUSE_VISIBLE; - --scp->sc->videoio_in_progress; + SC_VIDEO_UNLOCK(scp->sc); } int diff --git a/sys/dev/syscons/syscons.c b/sys/dev/syscons/syscons.c index c42fcf8102db..08f8e901ed1e 100644 --- a/sys/dev/syscons/syscons.c +++ b/sys/dev/syscons/syscons.c @@ -1578,7 +1578,7 @@ sccnupdate(scr_stat *scp) { /* this is a cut-down version of scrn_timer()... */ - if (scp->sc->font_loading_in_progress || scp->sc->videoio_in_progress) + if (scp->sc->font_loading_in_progress) return; if (debugger > 0 || panicstr || shutdown_in_progress) { @@ -1628,7 +1628,7 @@ scrn_timer(void *arg) return; /* don't do anything when we are performing some I/O operations */ - if (sc->font_loading_in_progress || sc->videoio_in_progress) { + if (sc->font_loading_in_progress) { if (again) timeout(scrn_timer, sc, hz / 10); return; @@ -1722,7 +1722,7 @@ scrn_update(scr_stat *scp, int show_cursor) /* assert(scp == scp->sc->cur_scp) */ - ++scp->sc->videoio_in_progress; + SC_VIDEO_LOCK(scp->sc); #ifndef SC_NO_CUTPASTE /* remove the previous mouse pointer image if necessary */ @@ -1792,7 +1792,7 @@ scrn_update(scr_stat *scp, int show_cursor) if (!show_cursor) { scp->end = 0; scp->start = scp->xsize*scp->ysize - 1; - --scp->sc->videoio_in_progress; + SC_VIDEO_UNLOCK(scp->sc); return; } @@ -1831,7 +1831,7 @@ scrn_update(scr_stat *scp, int show_cursor) scp->end = 0; scp->start = scp->xsize*scp->ysize - 1; - --scp->sc->videoio_in_progress; + SC_VIDEO_UNLOCK(scp->sc); } #ifdef DEV_SPLASH @@ -2094,7 +2094,7 @@ sc_switch_scr(sc_softc_t *sc, u_int next_scr) /* delay switch if the screen is blanked or being updated */ if ((sc->flags & SC_SCRN_BLANKED) || sc->write_in_progress - || sc->blink_in_progress || sc->videoio_in_progress) { + || sc->blink_in_progress) { sc->delayed_next_scr = next_scr + 1; sc_touch_scrn_saver(); DPRINTF(5, ("switch delayed\n")); @@ -2452,23 +2452,23 @@ void sc_draw_cursor_image(scr_stat *scp) { /* assert(scp == scp->sc->cur_scp); */ - ++scp->sc->videoio_in_progress; + SC_VIDEO_LOCK(scp->sc); (*scp->rndr->draw_cursor)(scp, scp->cursor_pos, scp->curs_attr.flags & CONS_BLINK_CURSOR, TRUE, sc_inside_cutmark(scp, scp->cursor_pos)); scp->cursor_oldpos = scp->cursor_pos; - --scp->sc->videoio_in_progress; + SC_VIDEO_UNLOCK(scp->sc); } void sc_remove_cursor_image(scr_stat *scp) { /* assert(scp == scp->sc->cur_scp); */ - ++scp->sc->videoio_in_progress; + SC_VIDEO_LOCK(scp->sc); (*scp->rndr->draw_cursor)(scp, scp->cursor_oldpos, scp->curs_attr.flags & CONS_BLINK_CURSOR, FALSE, sc_inside_cutmark(scp, scp->cursor_oldpos)); - --scp->sc->videoio_in_progress; + SC_VIDEO_UNLOCK(scp->sc); } static void @@ -2499,10 +2499,10 @@ sc_set_cursor_image(scr_stat *scp) } /* assert(scp == scp->sc->cur_scp); */ - ++scp->sc->videoio_in_progress; + SC_VIDEO_LOCK(scp->sc); (*scp->rndr->set_cursor)(scp, scp->curs_attr.base, scp->curs_attr.height, scp->curs_attr.flags & CONS_BLINK_CURSOR); - --scp->sc->videoio_in_progress; + SC_VIDEO_UNLOCK(scp->sc); } static void @@ -2606,6 +2606,9 @@ scinit(int unit, int flags) * disappeared... */ sc = sc_get_softc(unit, flags & SC_KERNEL_CONSOLE); + if ((sc->flags & SC_INIT_DONE) == 0) + SC_VIDEO_LOCKINIT(sc); + adp = NULL; if (sc->adapter >= 0) { vid_release(sc->adp, (void *)&sc->adapter); @@ -3458,9 +3461,9 @@ set_mode(scr_stat *scp) void sc_set_border(scr_stat *scp, int color) { - ++scp->sc->videoio_in_progress; + SC_VIDEO_LOCK(scp->sc); (*scp->rndr->draw_border)(scp, color); - --scp->sc->videoio_in_progress; + SC_VIDEO_UNLOCK(scp->sc); } #ifndef SC_NO_FONT_LOADING diff --git a/sys/dev/syscons/syscons.h b/sys/dev/syscons/syscons.h index 1cd33b251a05..089adf2b1fb1 100644 --- a/sys/dev/syscons/syscons.h +++ b/sys/dev/syscons/syscons.h @@ -34,6 +34,9 @@ #ifndef _DEV_SYSCONS_SYSCONS_H_ #define _DEV_SYSCONS_SYSCONS_H_ +#include +#include + /* machine-dependent part of the header */ #ifdef PC98 @@ -225,9 +228,9 @@ typedef struct sc_softc { char font_loading_in_progress; char switch_in_progress; - char videoio_in_progress; char write_in_progress; char blink_in_progress; + struct mtx video_mtx; long scrn_time_stamp; @@ -532,6 +535,19 @@ typedef struct { #define kbd_poll(kbd, on) \ (*kbdsw[(kbd)->kb_index]->poll)((kbd), (on)) +#define SC_VIDEO_LOCKINIT(sc) \ + mtx_init(&(sc)->video_mtx, "syscons video lock", NULL,MTX_SPIN); +#define SC_VIDEO_LOCK(sc) \ + do { \ + if (!cold) \ + mtx_lock_spin(&(sc)->video_mtx); \ + } while(0) +#define SC_VIDEO_UNLOCK(sc) \ + do { \ + if (!cold) \ + mtx_unlock_spin(&(sc)->video_mtx); \ + } while(0) + /* syscons.c */ extern int (*sc_user_ioctl)(struct cdev *dev, u_long cmd, caddr_t data, int flag, struct thread *td); diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c index b30f61aa8157..4018d3f7e1ec 100644 --- a/sys/kern/subr_witness.c +++ b/sys/kern/subr_witness.c @@ -393,6 +393,7 @@ static struct witness_order_list_entry order_lists[] = { { "td_contested", &lock_class_mtx_spin }, { "callout", &lock_class_mtx_spin }, { "entropy harvest mutex", &lock_class_mtx_spin }, + { "syscons video lock", &lock_class_mtx_spin }, /* * leaf locks */