Date: Thu, 1 Sep 2016 19:18:27 +0000 (UTC) From: Bruce Evans <bde@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r305231 - head/sys/dev/syscons Message-ID: <201609011918.u81JIRME017756@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bde Date: Thu Sep 1 19:18:26 2016 New Revision: 305231 URL: https://svnweb.freebsd.org/changeset/base/305231 Log: The log message for the previous commit didn't mention the most the important detail that sc_cngetc() now opens and closes the keyboard on every call again. This was moved from sc_cngetc() to scn_cngrab/ ungrab() in r228644, but the change wasn't quite complete. After fixes for nesting in kbdd_poll() in ukbd and kbdmux, these opens and closes should have no significant effect if done while grabbed. They fix unusual cases when cngetc() is called while not grabbed. This commit is the main fix for screen locking in sc_cnputc(): detect deadlock or likely-deadlock and handle it by buffering the output atomically and printing it later if the deadlock condition clears (and sc_cnputc() is called). The most common deadlock is when the screen lock is held by ourself. Then it would be safe to acquire the lock recursively if the console driver is calling printf() in a safe context, but we don't know when that is. It is not safe to ignore the lock even in kdb or panic mode. But ignore it in panic mode. The only other known case of deadlock is when another thread holds the lock but is running on a stopped CPU. Detect that case approximately by using trylock and retrying for 1000 usec. On a 4 GHz CPU, 100 usec is almost long enough -- screen switches take slightly longer than that. Not retrying at all is good enough except for stress tests, and planned future versions will extend the timeout so that the stress tests work better. To see the behaviour when deadlock is detected, single step through sctty_outwakeup() (or sc_puts() to start with deadlock). Another (serial) console is needed to the buffered-only output, but the keyboard works in this context to continue or step out of the deadlocked region. The buffer is not large enough to hold all the output for this. Modified: head/sys/dev/syscons/syscons.c head/sys/dev/syscons/syscons.h Modified: head/sys/dev/syscons/syscons.c ============================================================================== --- head/sys/dev/syscons/syscons.c Thu Sep 1 19:11:50 2016 (r305230) +++ head/sys/dev/syscons/syscons.c Thu Sep 1 19:18:26 2016 (r305231) @@ -1678,13 +1678,39 @@ sccnkbdunlock(sc_softc_t *sc, struct sc_ static void sccnscrlock(sc_softc_t *sc, struct sc_cnstate *sp) { - SC_VIDEO_LOCK(sc); + int retries; + + /** + * Locking method: + * - if kdb_active and video_mtx is not owned by anyone, then lock + * by kdb remaining active + * - if !kdb_active, try to acquire video_mtx without blocking or + * recursing; if we get it then it works normally. + * Note that video_mtx is especially unusable if we already own it, + * since then it is protecting something and syscons is not reentrant + * enough to ignore the protection even in the kdb_active case. + */ + if (kdb_active) { + sp->kdb_locked = sc->video_mtx.mtx_lock == MTX_UNOWNED || panicstr; + sp->mtx_locked = FALSE; + } else { + sp->kdb_locked = FALSE; + for (retries = 0; retries < 1000; retries++) { + sp->mtx_locked = mtx_trylock_spin_flags(&sc->video_mtx, + MTX_QUIET) != 0 || panicstr; + if (sp->mtx_locked) + break; + DELAY(1); + } + } } static void sccnscrunlock(sc_softc_t *sc, struct sc_cnstate *sp) { - SC_VIDEO_UNLOCK(sc); + if (sp->mtx_locked) + mtx_unlock_spin(&sc->video_mtx); + sp->mtx_locked = sp->kdb_locked = FALSE; } static void @@ -1721,6 +1747,8 @@ over_keyboard: ; /* The screen is opened iff locking it succeeds. */ sccnscrlock(sc, sp); + if (!sp->kdb_locked && !sp->mtx_locked) + return; sp->scr_opened = TRUE; /* The screen switch is optional. */ @@ -1797,6 +1825,10 @@ sc_cnungrab(struct consdev *cp) atomic_add_int(&sc->grab_level, -1); } +static char sc_cnputc_log[0x1000]; +static u_int sc_cnputc_loghead; +static u_int sc_cnputc_logtail; + static void sc_cnputc(struct consdev *cd, int c) { @@ -1808,12 +1840,28 @@ sc_cnputc(struct consdev *cd, int c) struct tty *tp; #endif #endif /* !SC_NO_HISTORY */ + u_int head; int s; /* assert(sc_console != NULL) */ sccnopen(scp->sc, &st, 0); + /* + * Log the output. + * + * In the unlocked case, the logging is intentionally only + * perfectly atomic for the indexes. + */ + head = atomic_fetchadd_int(&sc_cnputc_loghead, 1); + sc_cnputc_log[head % sizeof(sc_cnputc_log)] = c; + + /* + * If we couldn't open, return to defer output. + */ + if (!st.scr_opened) + return; + #ifndef SC_NO_HISTORY if (scp == scp->sc->cur_scp && scp->status & SLKED) { scp->status &= ~SLKED; @@ -1841,8 +1889,14 @@ sc_cnputc(struct consdev *cd, int c) } #endif /* !SC_NO_HISTORY */ - buf[0] = c; - sc_puts(scp, buf, 1, 1); + /* Play any output still in the log (our char may already be done). */ + while (sc_cnputc_logtail != atomic_load_acq_int(&sc_cnputc_loghead)) { + buf[0] = sc_cnputc_log[sc_cnputc_logtail++ % sizeof(sc_cnputc_log)]; + if (atomic_load_acq_int(&sc_cnputc_loghead) - sc_cnputc_logtail >= + sizeof(sc_cnputc_log)) + continue; + sc_puts(scp, buf, 1, 1); + } s = spltty(); /* block sckbdevent and scrn_timer */ sccnupdate(scp); @@ -1883,9 +1937,11 @@ sc_cngetc_locked(struct sc_cnstate *sp) * Stop the screen saver and update the screen if necessary. * What if we have been running in the screen saver code... XXX */ - sc_touch_scrn_saver(); + if (sp->scr_opened) + sc_touch_scrn_saver(); scp = sc_console->sc->cur_scp; /* XXX */ - sccnupdate(scp); + if (sp->scr_opened) + sccnupdate(scp); if (fkeycp < fkey.len) return fkey.str[fkeycp++]; Modified: head/sys/dev/syscons/syscons.h ============================================================================== --- head/sys/dev/syscons/syscons.h Thu Sep 1 19:11:50 2016 (r305230) +++ head/sys/dev/syscons/syscons.h Thu Sep 1 19:18:26 2016 (r305231) @@ -191,6 +191,8 @@ struct tty; struct sc_cnstate { u_char kbd_locked; + u_char kdb_locked; + u_char mtx_locked; u_char kbd_opened; u_char scr_opened; };
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201609011918.u81JIRME017756>