From 438acbcc599b3db8b97949a152027f27f8028066 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Sun, 29 Aug 2010 10:50:47 -0700 Subject: [PATCH] MPSAFE TTY - Fix deadlock in reporting of probe errors. * 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() (~^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 | 57 ++++++++++++++++++++-------------------- sys/kern/subr_prf.c | 27 ++++++++++++++++--- sys/sys/globaldata.h | 7 ++++- 3 files changed, 57 insertions(+), 34 deletions(-) diff --git a/sys/dev/serial/sio/sio.c b/sys/dev/serial/sio/sio.c index 75dc2492d5..085d830e0f 100644 --- a/sys/dev/serial/sio/sio.c +++ b/sys/dev/serial/sio/sio.c @@ -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); } diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c index 984bd8a377..1b4f517211 100644 --- a/sys/kern/subr_prf.c +++ b/sys/kern/subr_prf.c @@ -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); } diff --git a/sys/sys/globaldata.h b/sys/sys/globaldata.h index 5a0fa33eff..2c5878d99d 100644 --- a/sys/sys/globaldata.h +++ b/sys/sys/globaldata.h @@ -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 -- 2.41.0