kernel - Fix syscons deadlock during panic
authorMatthew Dillon <dillon@apollo.backplane.com>
Fri, 26 Jun 2015 21:05:40 +0000 (14:05 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Fri, 26 Jun 2015 21:49:30 +0000 (14:49 -0700)
* The system console can deadlock during a panic if a cpu is stopped
  while holding syscons_mtx.

* The new asynchronous update thread makes this problem more likely to occur.

* Fix the problem by recoding syscons_lock() to detect the panic or shutdown
  condition and loop for up to 0.5 seconds trying to get the syscons_mtx.
  If it is unable to acquire it, it reinitializes the mutex.

* We still have issues with VT switching away from X.

sys/dev/misc/syscons/sckmsrndr.c
sys/dev/misc/syscons/scterm-dumb.c
sys/dev/misc/syscons/scterm-sc.c
sys/dev/misc/syscons/scterm.c
sys/dev/misc/syscons/sctermvar.h
sys/dev/misc/syscons/scvgarndr.c
sys/dev/misc/syscons/syscons.c

index dfd60b9..2492af6 100644 (file)
@@ -48,7 +48,9 @@
 static vr_draw_t               kms_draw;
 static vr_draw_cursor_t                kms_cursor;
 static vr_blink_cursor_t       kms_blink;
+#ifndef SC_NO_CUTPASTE
 static vr_draw_mouse_t         kms_mouse;
+#endif
 
 static void                    kms_nop(scr_stat *scp, ...);
 
@@ -61,7 +63,7 @@ static sc_rndr_sw_t kmsrndrsw = {
 #ifndef SC_NO_CUTPASTE
        kms_mouse,
 #else
-       (vr_draw_mouse_t *)kms_nop;
+       (vr_draw_mouse_t *)kms_nop,
 #endif
 };
 RENDERER(kms, V_INFO_MM_TEXT, kmsrndrsw, kms_set);
index b6b3fdc..ee55d0b 100644 (file)
@@ -95,11 +95,11 @@ static void
 dumb_puts(scr_stat *scp, u_char *buf, int len)
 {
        while (len > 0) {
-               ++scp->sc->write_in_progress;
+               atomic_add_char(&scp->sc->write_in_progress, 1);
                sc_term_gen_print(scp, &buf, &len, SC_NORM_ATTR << 8);
                sc_term_gen_scroll(scp, scp->sc->scr_map[0x20],
                                   SC_NORM_ATTR << 8);
-               --scp->sc->write_in_progress;
+               atomic_add_char(&scp->sc->write_in_progress, -1);
        }
 }
 
index 8737970..0cad0b0 100644 (file)
@@ -647,7 +647,7 @@ scterm_puts(scr_stat *scp, u_char *buf, int len)
 
        tcp = scp->ts;
 outloop:
-       scp->sc->write_in_progress++;
+       atomic_add_char(&scp->sc->write_in_progress, 1);
 
        if (tcp->esc) {
                scterm_scan_esc(scp, tcp, *buf);
@@ -669,7 +669,7 @@ outloop:
 
        sc_term_gen_scroll(scp, scp->sc->scr_map[0x20], tcp->cur_attr);
 
-       scp->sc->write_in_progress--;
+       atomic_add_char(&scp->sc->write_in_progress, -1);
        if (len)
                goto outloop;
 }
index 98720ba..6244925 100644 (file)
@@ -41,6 +41,10 @@ SET_DECLARE(scterm_set, sc_term_sw_t);
 
 /* exported subroutines */
 
+/*
+ * Move the cursor, make sure reentrancy doesn't implode fields
+ * on panic.
+ */
 void
 sc_move_cursor(scr_stat *scp, int x, int y)
 {
@@ -54,7 +58,7 @@ sc_move_cursor(scr_stat *scp, int x, int y)
                y = scp->ysize - 1;
        scp->xpos = x;
        scp->ypos = y;
-       scp->cursor_pos = scp->ypos*scp->xsize + scp->xpos;
+       scp->cursor_pos = scp->ypos*scp->xsize + x;
 }
 
 void
index 84dce98..7700168 100644 (file)
@@ -265,20 +265,24 @@ sc_term_clr_eol(scr_stat *scp, int n, int ch, int attr)
 static __inline void
 sc_term_tab(scr_stat *scp, int n)
 {
+       int ypos;
        int i;
 
        if (n < 1)
                n = 1;
        i = (scp->xpos & ~7) + 8*n;
        if (i >= scp->xsize) {
-               if (scp->ypos >= scp->ysize - 1) {
+               ypos = scp->ypos;
+               if (ypos >= scp->ysize - 1) {
                        scp->xpos = 0;
-                       scp->ypos++;
+                       scp->ypos = ypos + 1;
                        scp->cursor_pos = scp->ypos*scp->xsize;
-               } else
-                       sc_move_cursor(scp, 0, scp->ypos + 1);
-       } else
+               } else {
+                       sc_move_cursor(scp, 0, ypos + 1);
+               }
+       } else {
                sc_move_cursor(scp, i, scp->ypos);
+       }
 }
 
 static __inline void
@@ -413,19 +417,31 @@ sc_term_gen_print(scr_stat *scp, u_char **buf, int *len, int attr)
        *len = l;
 }
 
+/*
+ * Handle scrolling, take care to ensure that we don't implode the
+ * fields if we happen to be multi-entrant during a panic.
+ */
 static __inline void
 sc_term_gen_scroll(scr_stat *scp, int ch, int attr)
 {
+       int pos;
+       int ypos;
+
+       pos = scp->cursor_pos;
+       cpu_ccfence();
        /* do we have to scroll ?? */
-       if (scp->cursor_pos >= scp->ysize*scp->xsize) {
+       if (pos >= scp->ysize*scp->xsize) {
                sc_remove_cutmarking(scp);              /* XXX */
 #ifndef SC_NO_HISTORY
                if (scp->history != NULL)
                        sc_hist_save_one_line(scp, 0);  /* XXX */
 #endif
                sc_vtb_delete(&scp->vtb, 0, scp->xsize, ch, attr);
-               scp->cursor_pos -= scp->xsize;
-               scp->ypos--;
+               scp->cursor_pos = pos - scp->xsize;
+               ypos = scp->ypos - 1;
+               if (ypos <= 0)
+                       ypos = 0;
+               scp->ypos = ypos;
                mark_all(scp);
        }
 }
index 8e98e34..0e480dc 100644 (file)
@@ -1,6 +1,4 @@
 /*-
- * (MPSAFE)
- *
  * Copyright (c) 1999 Kazutaka YOKOTA <yokota@zodiac.mech.utsunomiya-u.ac.jp>
  * All rights reserved.
  *
index 256c183..8042c51 100644 (file)
@@ -204,11 +204,26 @@ static int sc_allocate_keyboard(sc_softc_t *sc, int unit);
  *
  * We use mutex spinlocks here in order to allow reentrancy which should
  * avoid issues during panics.
+ *
+ * A panic can stop a cpu while it is holding the syscons_mtx.  If this
+ * occurs we try for up to 1/2 a second and then blow the spinlock away.
+ * The asynchronous update thread makes this happen more often.
  */
 static void
 syscons_lock(void)
 {
-       mtx_spinlock(&syscons_mtx);
+       if (panicstr || shutdown_in_progress) {
+               int retries = 500000;
+               while (mtx_spinlock_try(&syscons_mtx)) {
+                       tsc_delay(1000);        /* 1uS */
+                       if (--retries == 0) {
+                               mtx_init(&syscons_mtx, "syscons");
+                               retries = 500000;
+                       }
+               }
+       } else {
+               mtx_spinlock(&syscons_mtx);
+       }
 }
 
 /*
@@ -1831,8 +1846,14 @@ sccnupdate(scr_stat *scp)
 {
     /* this is a cut-down version of scrn_timer()... */
 
+    /*
+     * Don't update if videoio is in progress.  However, if we are paniced
+     * it is possible that the variable might be stuck, so ignore it in that
+     * case.
+     */
     if (scp->sc->font_loading_in_progress || scp->sc->videoio_in_progress) {
-       return;
+       if (panicstr == NULL && shutdown_in_progress == 0)
+               return;
     }
 
     if (debugger > 0 || panicstr || shutdown_in_progress) {
@@ -1998,11 +2019,16 @@ scrn_update(scr_stat *scp, int show_cursor, int flags)
 {
     int start;
     int end;
+    int cpos;
+    int copos;
     int s;
     int e;
 
     /*
-     * Try to run large screen updates as an async operation.
+     * Try to run large screen updates as an async operation, which will
+     * call this function again from a thread with BULKUNLOCK set, allowing
+     * interrupts during the update which tends to make the system behave
+     * better (no sound glitches, etc).
      */
     if ((flags & SCRN_ASYNCOK) && scp->asynctd &&
        (scp->end - scp->start) > 16 &&
@@ -2020,9 +2046,11 @@ scrn_update(scr_stat *scp, int show_cursor, int flags)
      */
 
     /* assert(scp == scp->sc->cur_scp) */
-
     atomic_add_int(&scp->sc->videoio_in_progress, 1);
 
+    cpos = scp->cursor_pos;
+    copos = scp->cursor_oldpos;
+
 #ifndef SC_NO_CUTPASTE
     /* remove the previous mouse pointer image if necessary */
     if (scp->status & MOUSE_VISIBLE) {
@@ -2031,9 +2059,9 @@ scrn_update(scr_stat *scp, int show_cursor, int flags)
        if ((scp->status & (MOUSE_MOVED | MOUSE_HIDDEN))
            || and_region(&s, &e, scp->start, scp->end)
            || ((scp->status & CURSOR_ENABLED) && 
-               (scp->cursor_pos != scp->cursor_oldpos) &&
-               (and_region(&s, &e, scp->cursor_pos, scp->cursor_pos)
-                || and_region(&s, &e, scp->cursor_oldpos, scp->cursor_oldpos)))) {
+               (cpos != copos) &&
+               (and_region(&s, &e, cpos, cpos)
+                || and_region(&s, &e, copos, copos)))) {
            sc_remove_mouse_image(scp);
            if (scp->end >= scp->xsize*scp->ysize)
                scp->end = scp->xsize*scp->ysize - 1;
@@ -2041,17 +2069,16 @@ scrn_update(scr_stat *scp, int show_cursor, int flags)
     }
 #endif /* !SC_NO_CUTPASTE */
 
-#if 1
-    /* debug: XXX */
+    /*
+     * WARNING: Don't add debugging kprintf()s with syscons locked.
+     *         Or at all.
+     */
     if (scp->end >= scp->xsize*scp->ysize) {
-       kprintf("scrn_update(): scp->end %d > size_of_screen!!\n", scp->end);
        scp->end = scp->xsize*scp->ysize - 1;
     }
     if (scp->start < 0) {
-       kprintf("scrn_update(): scp->start %d < 0\n", scp->start);
        scp->start = 0;
     }
-#endif
 
     /*
      * Main update, extract update range and reset
@@ -2063,11 +2090,15 @@ scrn_update(scr_stat *scp, int show_cursor, int flags)
     scp->start = scp->xsize * scp->ysize - 1;
 
     if (start <= end)  {
-       if (flags & SCRN_BULKUNLOCK)
+       if (flags & SCRN_BULKUNLOCK) {
+           atomic_add_int(&scp->sc->videoio_in_progress, -1);
            syscons_unlock();
+       }
        (*scp->rndr->draw)(scp, start, end - start + 1, FALSE);
-       if (flags & SCRN_BULKUNLOCK)
+       if (flags & SCRN_BULKUNLOCK) {
            syscons_lock();
+           atomic_add_int(&scp->sc->videoio_in_progress, 1);
+       }
 
        /*
         * Handle cut sequence
@@ -2084,11 +2115,15 @@ scrn_update(scr_stat *scp, int show_cursor, int flags)
            }
            /* does the cut-mark region overlap with the update region? */
            if (and_region(&s, &e, start, end)) {
-               if (flags & SCRN_BULKUNLOCK)
+               if (flags & SCRN_BULKUNLOCK) {
+                   atomic_add_int(&scp->sc->videoio_in_progress, -1);
                    syscons_unlock();
+               }
                (*scp->rndr->draw)(scp, s, e - s + 1, TRUE);
-               if (flags & SCRN_BULKUNLOCK)
+               if (flags & SCRN_BULKUNLOCK) {
                    syscons_lock();
+                   atomic_add_int(&scp->sc->videoio_in_progress, 1);
+               }
            }
        }
     }
@@ -2097,27 +2132,28 @@ scrn_update(scr_stat *scp, int show_cursor, int flags)
     if (!show_cursor)
        goto cleanup;
 
+#if 0
     /* update cursor image */
-    if (scp->status & CURSOR_ENABLED) {
+    if ((scp->status & CURSOR_ENABLED) && start <= end) {
        s = start;
        e = end;
         /* did cursor move since last time ? */
-        if (scp->cursor_pos != scp->cursor_oldpos) {
+        if (cpos != copos) {
             /* do we need to remove old cursor image ? */
-            if (!and_region(&s, &e, scp->cursor_oldpos, scp->cursor_oldpos))
+            if (!and_region(&s, &e, copos, copos))
                 sc_remove_cursor_image(scp);
             sc_draw_cursor_image(scp);
         } else {
-            if (and_region(&s, &e, scp->cursor_pos, scp->cursor_pos))
+            if (and_region(&s, &e, cpos, cpos))
                /* cursor didn't move, but has been overwritten */
                sc_draw_cursor_image(scp);
            else if (scp->sc->flags & SC_BLINK_CURSOR)
                /* if it's a blinking cursor, update it */
-               (*scp->rndr->blink_cursor)(scp, scp->cursor_pos,
-                                          sc_inside_cutmark(scp,
-                                              scp->cursor_pos));
+               (*scp->rndr->blink_cursor)(scp, cpos,
+                                          sc_inside_cutmark(scp, cpos));
         }
     }
+#endif
 
 #ifndef SC_NO_CUTPASTE
     /* update "pseudo" mouse pointer image */