d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add some locking to sc_cngetc().

Browse Source 
Keyboard input needs Giant locking, and that is not possible to do
correctly here.  Use mtx_trylock() and proceed unlocked as before if
we can't acquire Giant (non-recursively), except in kdb mode don't
even try to acquire Giant.  Everything here is a hack, but it often
works.  Even if mtx_trylock() succeeds, this might be a LOR.

Keyboard input also needs screen locking, to handle screen updates
and switches.  Add this, using the same simplistic screen locking
as for sc_cnputc().

Giant must be acquired before the screen lock, and the screen lock
must be dropped when calling the keyboard driver (else it would get a
harmless LOR if it tries to acquire Giant).  It was intended that sc
cn open/close hide the locking calls, and they do for i/o functions
functions except for this complication.

Non-console keyboard input is still only Giant-locked, with screen
locking in some called functions.  This is correct for the keyboard
parts only.

When Giant cannot be acquired properly, atkbd and kbdmux tend to race
and work (they assume that the caller acquired Giant properly and don't
try to acquire it again or check that it has been acquired, and the
races rarely matter), while ukbd tends to deadlock or panic (since it
does the opposite, and has other usb threads to deadlock with).

The keyboard (Giant) locking here does very little, but the screen
locking completes screen locking for console mode except for not
detecting or handling deadlock.
This commit is contained in:
Bruce Evans 2016-08-31 11:10:39 +00:00
parent 63dc81d861
commit a95582c6fd
2 changed files with 41 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
sys/dev/syscons/syscons.c
View File

@ -1649,10 +1649,32 @@ sc_cnterm(struct consdev *cp)
static void sccnclose(sc_softc_t *sc, struct sc_cnstate *sp);
static int sc_cngetc_locked(struct sc_cnstate *sp);
static void sccnkbdlock(sc_softc_t *sc, struct sc_cnstate *sp);
static void sccnkbdunlock(sc_softc_t *sc, struct sc_cnstate *sp);
static void sccnopen(sc_softc_t *sc, struct sc_cnstate *sp, int flags);
static void sccnscrlock(sc_softc_t *sc, struct sc_cnstate *sp);
static void sccnscrunlock(sc_softc_t *sc, struct sc_cnstate *sp);
static void
sccnkbdlock(sc_softc_t *sc, struct sc_cnstate *sp)
{
/*
* Locking method: hope for the best.
* The keyboard is supposed to be Giant locked. We can't handle that
* in general. The kdb_active case here is not safe, and we will
* proceed without the lock in all cases.
*/
sp->kbd_locked = !kdb_active && mtx_trylock(&Giant);
}
static void
sccnkbdunlock(sc_softc_t *sc, struct sc_cnstate *sp)
{
if (sp->kbd_locked)
mtx_unlock(&Giant);
sp->kbd_locked = FALSE;
}
static void
sccnscrlock(sc_softc_t *sc, struct sc_cnstate *sp)
{
@ -1674,11 +1696,14 @@ sccnopen(sc_softc_t *sc, struct sc_cnstate *sp, int flags)
sp->kbd_opened = FALSE;
sp->scr_opened = FALSE;
sp->kbd_locked = FALSE;
/* Opening the keyboard is optional. */
if (!(flags & 1) || sc->kbd == NULL)
goto over_keyboard;
sccnkbdlock(sc, sp);
/*
* Make sure the keyboard is accessible even when the kbd device
* driver is disabled.
@ -1726,6 +1751,7 @@ sccnclose(sc_softc_t *sc, struct sc_cnstate *sp)
kbdd_disable(sc->kbd);
sp->kbd_opened = FALSE;
sccnkbdunlock(sc, sp);
}
/*
@ -1751,6 +1777,7 @@ sc_cngrab(struct consdev *cp)
if (lev >= 0 && lev < 2) {
sccnopen(sc, &sc->grab_state[lev], 1 | 2);
sccnscrunlock(sc, &sc->grab_state[lev]);
sccnkbdunlock(sc, &sc->grab_state[lev]);
}
}
@ -1763,6 +1790,7 @@ sc_cnungrab(struct consdev *cp)
sc = sc_console->sc;
lev = atomic_load_acq_int(&sc->grab_level) - 1;
if (lev >= 0 && lev < 2) {
sccnkbdlock(sc, &sc->grab_state[lev]);
sccnscrlock(sc, &sc->grab_state[lev]);
sccnclose(sc, &sc->grab_state[lev]);
}
@ -1825,16 +1853,20 @@ sc_cnputc(struct consdev *cd, int c)
static int
sc_cngetc(struct consdev *cd)
{
struct sc_cnstate st;
int c, s;
/* assert(sc_console != NULL) */
sccnopen(sc_console->sc, &st, 1);
s = spltty(); /* block sckbdevent and scrn_timer while we poll */
if (sc_console->sc->kbd == NULL) {
if (!st.kbd_opened) {
splx(s);
return -1;
sccnclose(sc_console->sc, &st);
return -1; /* means no keyboard since we fudged the locking */
}
c = sc_cngetc_locked(NULL);
c = sc_cngetc_locked(&st);
splx(s);
sccnclose(sc_console->sc, &st);
return c;
}
@ -1858,7 +1890,7 @@ sc_cngetc_locked(struct sc_cnstate *sp)
if (fkeycp < fkey.len)
return fkey.str[fkeycp++];
c = scgetc(scp->sc, SCGETC_CN | SCGETC_NONBLOCK, NULL);
c = scgetc(scp->sc, SCGETC_CN | SCGETC_NONBLOCK, sp);
switch (KEYFLAGS(c)) {
case 0: /* normal char */
@ -3464,7 +3496,11 @@ scgetc(sc_softc_t *sc, u_int flags, struct sc_cnstate *sp)
scp = sc->cur_scp;
/* first see if there is something in the keyboard port */
for (;;) {
if (flags & SCGETC_CN)
sccnscrunlock(sc, sp);
c = kbdd_read_char(sc->kbd, !(flags & SCGETC_NONBLOCK));
if (flags & SCGETC_CN)
sccnscrlock(sc, sp);
if (c == ERRKEY) {
if (!(flags & SCGETC_CN))
sc_bell(scp, bios_value.bell_pitch, BELL_DURATION);

1
sys/dev/syscons/syscons.h
View File

@ -190,6 +190,7 @@ struct scr_stat;
struct tty;
struct sc_cnstate {
u_char kbd_locked;
u_char kbd_opened;
u_char scr_opened;
};