MPSAFE TTY - Fix deadlock in reporting of probe errors.
authorMatthew Dillon <dillon@apollo.backplane.com>
Sun, 29 Aug 2010 17:50:47 +0000 (10:50 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sun, 29 Aug 2010 17:52:29 +0000 (10:52 -0700)
* When sio can't drain it reports the fact, but it was doing it while
  still holding com_lock.  Rearranging the lock fixes the problem.

* Clean up some unnecessary tty_tokens in critical code paths and rearrange
  code to not hold com_lock across tty_token acquisition calls.

* Release the com_lock around breakpoint() (<enter>~^B sequence) to
  avoid a deadlock.

* Detect and ignore a reentrant kprintf() to try to avoid a deadlock.
  The detection flag is also reset by a panic.

sys/dev/serial/sio/sio.c
sys/kern/subr_prf.c
sys/sys/globaldata.h

index 75dc249..085d830 100644 (file)
@@ -731,10 +731,10 @@ sioprobe(device_t dev, int xrid, u_long rclk)
                (void)sio_getreg(com, com_data);
        }
        if (fn == 256) {
-               kprintf("sio%d: can't drain, serial port might "
-                       "not exist, disabling\n", device_get_unit(dev));
                com_unlock();
                lwkt_reltoken(&tty_token);
+               kprintf("sio%d: can't drain, serial port might "
+                       "not exist, disabling\n", device_get_unit(dev));
                return (ENXIO);
        }
 
@@ -1060,7 +1060,6 @@ sioattach(device_t dev, int xrid, u_long rclk)
        } else
                com->it_in.c_ispeed = com->it_in.c_ospeed = TTYDEF_SPEED;
        if (siosetwater(com, com->it_in.c_ispeed) != 0) {
-               com_unlock();
                /*
                 * Leave i/o resources allocated if this is a `cn'-level
                 * console, so that other devices can't snarf them.
@@ -1070,7 +1069,6 @@ sioattach(device_t dev, int xrid, u_long rclk)
                lwkt_reltoken(&tty_token);
                return (ENOMEM);
        }
-       com_unlock();
        termioschars(&com->it_in);
        com->it_out = com->it_in;
 
@@ -1656,7 +1654,10 @@ siodtrwakeup(void *chan)
 }
 
 /*
- * NOTE: Must be called with tty_token held
+ * NOTE: Normally called with tty_token held but might not be when
+ *      operating as the console.
+ *
+ *      Must be called with com_lock
  */
 static void
 sioinput(struct com_s *com)
@@ -1667,7 +1668,6 @@ sioinput(struct com_s *com)
        int             recv_data;
        struct tty      *tp;
 
-       ASSERT_LWKT_TOKEN_HELD(&tty_token);
        buf = com->ibuf;
        tp = com->tp;
        if (!(tp->t_state & TS_ISOPEN) || !(tp->t_cflag & CREAD)) {
@@ -1784,6 +1784,9 @@ siointr(void *arg)
        lwkt_reltoken(&tty_token);
 }
 
+/*
+ * Called with tty_token held and com_lock held.
+ */
 static void
 siointr1(struct com_s *com)
 {
@@ -1795,7 +1798,6 @@ siointr1(struct com_s *com)
        u_char  int_ctl_new;
        u_int   count;
 
-       lwkt_gettoken(&tty_token);
        int_ctl = inb(com->intr_ctl_port);
        int_ctl_new = int_ctl;
 
@@ -1838,7 +1840,9 @@ siointr1(struct com_s *com)
                                        if (recv_data == KEY_TILDE)
                                                brk_state2 = recv_data;
                                        else if (brk_state2 == KEY_TILDE && recv_data == KEY_CRTLB) {
+                                                       com_unlock();
                                                        breakpoint();
+                                                       com_lock();
                                                        brk_state1 = brk_state2 = 0;
                                                        goto cont;
                                        } else
@@ -1863,7 +1867,9 @@ siointr1(struct com_s *com)
                                if (line_status & LSR_BI) {
 #if defined(DDB) && defined(BREAK_TO_DEBUGGER)
                                        if (com->unit == comconsole) {
+                                               com_unlock();
                                                breakpoint();
+                                               com_lock();
                                                goto cont;
                                        }
 #endif
@@ -1998,11 +2004,9 @@ cont:
                if ((inb(com->int_id_port) & IIR_IMASK) == IIR_NOPEND)
 #endif /* COM_MULTIPORT */
                {
-                       lwkt_reltoken(&tty_token);
                        return;
                }
        }
-       lwkt_reltoken(&tty_token);
 }
 
 static int
@@ -2253,6 +2257,9 @@ repeat:
        lwkt_reltoken(&tty_token);
 }
 
+/*
+ * Called with tty_token held but no com_lock
+ */
 static int
 comparam(struct tty *tp, struct termios *t)
 {
@@ -2264,11 +2271,9 @@ comparam(struct tty *tp, struct termios *t)
        u_char          dlbl;
        int             unit;
 
-       lwkt_gettoken(&tty_token);
        unit = DEV_TO_UNIT(tp->t_dev);
        com = com_addr(unit);
        if (com == NULL) {
-               lwkt_reltoken(&tty_token);
                return (ENODEV);
        }
 
@@ -2280,15 +2285,11 @@ comparam(struct tty *tp, struct termios *t)
        if (t->c_ospeed == 0)
                divisor = 0;
        else {
-               if (t->c_ispeed != t->c_ospeed) {
-                       lwkt_reltoken(&tty_token);
+               if (t->c_ispeed != t->c_ospeed)
                        return (EINVAL);
-               }
                divisor = siodivisor(com->rclk, t->c_ispeed);
-               if (divisor == 0) {
-                       lwkt_reltoken(&tty_token);
+               if (divisor == 0)
                        return (EINVAL);
-               }
        }
 
        /* parameters are OK, convert them to the com struct and the device */
@@ -2439,20 +2440,23 @@ comparam(struct tty *tp, struct termios *t)
         * unconditionally, but that defeated the careful discarding of
         * stale input in sioopen().
         */
-       if (com->state >= (CS_BUSY | CS_TTGO))
+       if (com->state >= (CS_BUSY | CS_TTGO)) {
+               com_lock();
                siointr1(com);
-
-       com_unlock();
+               com_unlock();
+       }
        crit_exit();
        comstart(tp);
        if (com->ibufold != NULL) {
                kfree(com->ibufold, M_DEVBUF);
                com->ibufold = NULL;
        }
-       lwkt_reltoken(&tty_token);
        return (0);
 }
 
+/*
+ * called with tty_token held
+ */
 static int
 siosetwater(struct com_s *com, speed_t speed)
 {
@@ -2461,7 +2465,6 @@ siosetwater(struct com_s *com, speed_t speed)
        int             ibufsize;
        struct tty      *tp;
 
-       lwkt_gettoken(&tty_token);
        /*
         * Make the buffer size large enough to handle a softtty interrupt
         * latency of about 2 ticks without loss of throughput or data
@@ -2471,11 +2474,8 @@ siosetwater(struct com_s *com, speed_t speed)
        cp4ticks = speed / 10 / hz * 4;
        for (ibufsize = 128; ibufsize < cp4ticks;)
                ibufsize <<= 1;
-       if (ibufsize == com->ibufsize) {
-               com_lock();
-               lwkt_reltoken(&tty_token);
+       if (ibufsize == com->ibufsize)
                return (0);
-       }
 
        /*
         * Allocate input buffer.  The extra factor of 2 in the size is
@@ -2494,8 +2494,7 @@ siosetwater(struct com_s *com, speed_t speed)
        }
 
        /*
-        * Read current input buffer, if any.  Continue with interrupts
-        * disabled.
+        * Read current input buffer.
         */
        com_lock();
        if (com->iptr != com->ibuf)
@@ -2516,7 +2515,7 @@ siosetwater(struct com_s *com, speed_t speed)
        com->ibufend = ibuf + ibufsize;
        com->ierroff = ibufsize;
        com->ihighwater = ibuf + 3 * ibufsize / 4;
-       lwkt_reltoken(&tty_token);
+       com_unlock();
        return (0);
 }
 
index 984bd8a..1b4f517 100644 (file)
@@ -573,7 +573,16 @@ kvcprintf(char const *fmt, void (*func)(int, void*), void *arg,
        int dwidth, upper;
        char padc;
        int retval = 0, stop = 0;
-       int not_panic_cpu = (panic_cpu_gd != mycpu);
+       int usespin;
+
+       /*
+        * Make a supreme effort to avoid reentrant panics or deadlocks.
+        */
+       if (func == kputchar) {
+               if (mycpu->gd_flags & GDF_KPRINTF)
+                       return(0);
+               atomic_set_long(&mycpu->gd_flags, GDF_KPRINTF);
+       }
 
        num = 0;
        if (!func)
@@ -587,7 +596,10 @@ kvcprintf(char const *fmt, void (*func)(int, void*), void *arg,
        if (radix < 2 || radix > 36)
                radix = 10;
 
-       if (not_panic_cpu && func == kputchar) {
+       usespin = (panic_cpu_gd != mycpu &&
+                  func == kputchar &&
+                  (((struct putchar_arg *)arg)->flags & TOTTY) == 0);
+       if (usespin) {
                crit_enter_hard();
                spin_lock_wr(&cons_spin);
        }
@@ -871,7 +883,12 @@ number:
                }
        }
 done:
-       if (not_panic_cpu && func == kputchar) {
+       /*
+        * Cleanup reentrancy issues.
+        */
+       if (func == kputchar)
+               atomic_clear_long(&mycpu->gd_flags, GDF_KPRINTF);
+       if (usespin) {
                spin_unlock_wr(&cons_spin);
                crit_exit_hard();
        }
@@ -881,12 +898,14 @@ done:
 #undef PCHAR
 
 /*
- * Called from the panic code
+ * Called from the panic code to try to get the console working
+ * again in case we paniced inside a kprintf().
  */
 void
 kvcreinitspin(void)
 {
        spin_init(&cons_spin);
+       atomic_clear_long(&mycpu->gd_flags, GDF_KPRINTF);
 }
 
 
index 5a0fa33..2c5878d 100644 (file)
@@ -127,7 +127,7 @@ struct globaldata {
        struct thread   *gd_curthread;
        struct thread   *gd_freetd;             /* cache one free td */
        __uint32_t      gd_reqflags;            /* (see note above) */
-       void            *gd_unused00B;
+       long            gd_flags;
        lwkt_queue      gd_tdallq;              /* all threads */
        lwkt_queue      gd_tdrunq;              /* runnable threads */
        __uint32_t      gd_cpuid;
@@ -193,6 +193,11 @@ typedef struct globaldata *globaldata_t;
                                RQF_AST_UPCALL)
 #define RQF_IDLECHECK_MASK     (RQF_IPIQ|RQF_INTPEND|RQF_TIMER)
 
+/*
+ * globaldata flags
+ */
+#define GDF_KPRINTF            0x0001  /* kprintf() reentrancy */
+
 #endif
 
 #ifdef _KERNEL