MPSAFE TTY - Refactor kprintf()'s spinlock, shutdown, move cons_spin
authorMatthew Dillon <dillon@apollo.backplane.com>
Sun, 29 Aug 2010 16:36:53 +0000 (09:36 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Sun, 29 Aug 2010 16:36:53 +0000 (09:36 -0700)
* Adjust panic assertions to reduce reentrant panics.

* Redo the shutdown code to properly interlock panic_cpu_gd

* Move cons_lock into subr_prf.c and use a normal spinlock instead of
  a deprecated (w/interrupt disablement) spinlock.

  Adjust kvcprintf() when used with kputchar() to acquire cons_spin
  only if the current cpu is not the currently panicing cpu.

  Adjust kvcprintf() when used with kputchar() to acquire a hard
  critical section.

18 files changed:
sys/bus/cam/scsi/scsi_low.c
sys/ddb/db_output.c
sys/kern/kern_lock.c
sys/kern/kern_shutdown.c
sys/kern/lwkt_thread.c
sys/kern/lwkt_token.c
sys/kern/subr_prf.c
sys/platform/pc32/i386/machdep.c
sys/platform/pc32/i386/spinlock.s
sys/platform/pc32/i386/userconfig.c
sys/platform/pc32/include/lock.h
sys/platform/pc64/include/lock.h
sys/platform/pc64/x86_64/console.c
sys/platform/pc64/x86_64/machdep.c
sys/platform/pc64/x86_64/spinlock.s
sys/platform/vkernel/platform/console.c
sys/platform/vkernel64/platform/console.c
sys/sys/systm.h

index 258b0e9..f213ac3 100644 (file)
@@ -2262,8 +2262,7 @@ static char tw_chars[] = "|/-\\";
 static void
 scsi_low_twiddle_wait(void)
 {
-       cnputc('\b');
-       cnputc(tw_chars[tw_pos++]);
+       kprintf("\b%c", tw_chars[tw_pos++]);
        tw_pos %= (sizeof(tw_chars) - 1);
        SCSI_LOW_DELAY(TWIDDLEWAIT);
 }
@@ -2278,8 +2277,7 @@ scsi_low_bus_reset(struct scsi_low_softc *slp)
        kprintf("%s: try to reset scsi bus  ", slp->sl_xname);
        for (i = 0; i <= SCSI2_RESET_DELAY / TWIDDLEWAIT ; i++)
                scsi_low_twiddle_wait();
-       cnputc('\b');
-       kprintf("\n");
+       kprintf("\b\n");
 }
 
 int
index 3880875..cf674a0 100644 (file)
@@ -39,6 +39,8 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/cons.h>
+#include <sys/thread2.h>
+#include <sys/spinlock2.h>
 
 #include <machine/stdarg.h>
 
@@ -113,6 +115,8 @@ db_putchar(int c, void *arg)
                return;
        }
 
+       crit_enter_hard();
+
        if (c > ' ' && c <= '~') {
            /*
             * Printing character.
@@ -151,6 +155,7 @@ db_putchar(int c, void *arg)
            cnputc(c);
        }
        /* other characters are assumed non-printing */
+       crit_exit_hard();
 }
 
 /*
@@ -164,6 +169,9 @@ db_print_position(void)
 
 /*
  * Printing
+ *
+ * NOTE: We bypass subr_prf's cons_spin here by using our own putchar
+ *      function.
  */
 void
 db_printf(const char *fmt, ...)
index 9d75504..132d31a 100644 (file)
@@ -163,22 +163,21 @@ debuglockmgr(struct lock *lkp, u_int flags,
        int error;
        int extflags;
        int dowakeup;
-       static int didpanic;
 
        error = 0;
        dowakeup = 0;
 
        if (mycpu->gd_intr_nesting_level &&
            (flags & LK_NOWAIT) == 0 &&
-           (flags & LK_TYPE_MASK) != LK_RELEASE && didpanic == 0) {
+           (flags & LK_TYPE_MASK) != LK_RELEASE &&
+           panic_cpu_gd != mycpu
+       ) {
 
 #ifndef DEBUG_LOCKS
-               didpanic = 1;
                panic("lockmgr %s from %p: called from interrupt, ipi, "
                      "or hard code section",
                      lkp->lk_wmesg, ((int **)&lkp)[-1]);
 #else
-               didpanic = 1;
                panic("lockmgr %s from %s:%d: called from interrupt, ipi, "
                      "or hard code section",
                      lkp->lk_wmesg, file, line);
index 805a726..5c1a08e 100644 (file)
@@ -149,7 +149,6 @@ int dumping;                                /* system is dumping */
 static struct dumperinfo dumper;       /* selected dumper */
 
 #ifdef SMP
-u_int panic_cpu_interlock;             /* panic interlock */
 globaldata_t panic_cpu_gd;             /* which cpu took the panic */
 #endif
 
@@ -668,6 +667,7 @@ void
 panic(const char *fmt, ...)
 {
        int bootopt, newpanic;
+       globaldata_t gd = mycpu;
        __va_list ap;
        static char buf[256];
 
@@ -685,37 +685,66 @@ panic(const char *fmt, ...)
         * Bumping gd_trap_nesting_level will also bypass assertions in
         * lwkt_switch() and allow us to switch away even if we are a
         * FAST interrupt or IPI.
+        *
+        * The setting of panic_cpu_gd also determines how kprintf()
+        * spin-locks itself.  DDB can set panic_cpu_gd as well.
         */
-       if (atomic_poll_acquire_int(&panic_cpu_interlock)) {
-               panic_cpu_gd = mycpu;
-       } else if (panic_cpu_gd != mycpu) {
-               crit_enter();
-               ++mycpu->gd_trap_nesting_level;
-               if (mycpu->gd_trap_nesting_level < 25) {
-                       kprintf("SECONDARY PANIC ON CPU %d THREAD %p\n",
-                               mycpu->gd_cpuid, curthread);
-               }
-               curthread->td_release = NULL;   /* be a grinch */
-               for (;;) {
-                       lwkt_deschedule_self(curthread);
-                       lwkt_switch();
+       for (;;) {
+               globaldata_t xgd = panic_cpu_gd;
+
+               /*
+                * Someone else got the panic cpu
+                */
+               if (xgd && xgd != gd) {
+                       crit_enter();
+                       ++mycpu->gd_trap_nesting_level;
+                       if (mycpu->gd_trap_nesting_level < 25) {
+                               kprintf("SECONDARY PANIC ON CPU %d THREAD %p\n",
+                                       mycpu->gd_cpuid, curthread);
+                       }
+                       curthread->td_release = NULL;   /* be a grinch */
+                       for (;;) {
+                               lwkt_deschedule_self(curthread);
+                               lwkt_switch();
+                       }
+                       /* NOT REACHED */
+                       /* --mycpu->gd_trap_nesting_level */
+                       /* crit_exit() */
                }
-               /* NOT REACHED */
-               /* --mycpu->gd_trap_nesting_level */
-               /* crit_exit() */
+
+               /*
+                * Reentrant panic
+                */
+               if (xgd && xgd == gd)
+                       break;
+
+               /*
+                * We got it
+                */
+               if (atomic_cmpset_ptr(&panic_cpu_gd, NULL, gd))
+                       break;
        }
+#else
+       panic_cpu_gd = gd;
+       kvcreinitspin();
 #endif
+       /*
+        * Setup
+        */
        bootopt = RB_AUTOBOOT | RB_DUMP;
        if (sync_on_panic == 0)
                bootopt |= RB_NOSYNC;
        newpanic = 0;
-       if (panicstr)
+       if (panicstr) {
                bootopt |= RB_NOSYNC;
-       else {
+       else {
                panicstr = fmt;
                newpanic = 1;
        }
 
+       /*
+        * Format the panic string.
+        */
        __va_start(ap, fmt);
        kvsnprintf(buf, sizeof(buf), fmt, ap);
        if (panicstr == fmt)
index 9407854..c8004a0 100644 (file)
@@ -519,7 +519,7 @@ lwkt_switch(void)
        int savegdnest;
        int savegdtrap;
 
-       if (gd->gd_trap_nesting_level == 0 && panicstr == NULL) {
+       if (gd->gd_trap_nesting_level == 0 && panic_cpu_gd != mycpu) {
            panic("lwkt_switch: Attempt to switch from a "
                  "a fast interrupt, ipi, or hard code section, "
                  "td %p\n",
index be36331..a3a4819 100644 (file)
@@ -341,9 +341,11 @@ _lwkt_trytokref2(lwkt_tokref_t nref, thread_t td, int blocking)
                ref = tok->t_ref;
                if (ref == NULL) {
                        KASSERT((blocking == 0 ||
-                               td->td_gd->gd_intr_nesting_level == 0),
+                               td->td_gd->gd_intr_nesting_level == 0 ||
+                               panic_cpu_gd == mycpu),
                                ("Attempt to acquire token %p not already "
                                 "held in hard code section", tok));
+
                        /*
                         * NOTE: If atomic_cmpset_ptr() fails we have to
                         *       loop and try again.  It just means we
index b9dd726..984bd8a 100644 (file)
@@ -59,6 +59,9 @@
 #include <sys/lock.h>
 #include <sys/ctype.h>
 
+#include <sys/thread2.h>
+#include <sys/spinlock2.h>
+
 #ifdef DDB
 #include <ddb/ddb.h>
 #endif
@@ -92,7 +95,6 @@ extern        int log_open;
 
 struct tty *constty;                   /* pointer to console "window" tty */
 
-static void (*v_putc)(int) = cnputc;   /* routine to putc on virtual console */
 static void  msglogchar(int c, int pri);
 static void  msgaddchar(int c, void *dummy);
 static void  kputchar (int ch, void *arg);
@@ -102,6 +104,8 @@ static void  snprintf_func (int ch, void *arg);
 
 static int consintr = 1;               /* Ok to handle console interrupts? */
 static int msgbufmapped;               /* Set when safe to use msgbuf */
+static struct spinlock cons_spin = SPINLOCK_INITIALIZER(cons_spin);
+
 int msgbuftrigger;
 
 static int      log_console_output = 1;
@@ -284,8 +288,6 @@ log_console(struct uio *uio)
 
 /*
  * Output to the console.
- *
- * NOT YET ENTIRELY MPSAFE
  */
 int
 kprintf(const char *fmt, ...)
@@ -301,9 +303,7 @@ kprintf(const char *fmt, ...)
        pca.tty = NULL;
        pca.flags = TOCONS | TOLOG;
        pca.pri = -1;
-       cons_lock();
        retval = kvcprintf(fmt, kputchar, &pca, 10, ap);
-       cons_unlock();
        __va_end(ap);
        if (!panicstr)
                msgbuftrigger = 1;
@@ -323,9 +323,7 @@ kvprintf(const char *fmt, __va_list ap)
        pca.tty = NULL;
        pca.flags = TOCONS | TOLOG;
        pca.pri = -1;
-       cons_lock();
        retval = kvcprintf(fmt, kputchar, &pca, 10, ap);
-       cons_unlock();
        if (!panicstr)
                msgbuftrigger = 1;
        consintr = savintr;             /* reenable interrupts */
@@ -383,7 +381,7 @@ kputchar(int c, void *arg)
        if ((flags & TOLOG))
                msglogchar(c, ap->pri);
        if ((flags & TOCONS) && constty == NULL && c != '\0')
-               (*v_putc)(c);
+               cnputc(c);
 }
 
 /*
@@ -557,10 +555,13 @@ ksprintn(char *nbuf, uintmax_t num, int base, int *lenp, int upper)
  *             ("%6D", ptr, ":")   -> XX:XX:XX:XX:XX:XX
  *             ("%*D", len, ptr, " " -> XX XX XX XX ...
  */
+
+#define PCHAR(c) {int cc=(c); if(func) (*func)(cc,arg); else *d++=cc; retval++;}
+
 int
-kvcprintf(char const *fmt, void (*func)(int, void*), void *arg, int radix, __va_list ap)
+kvcprintf(char const *fmt, void (*func)(int, void*), void *arg,
+         int radix, __va_list ap)
 {
-#define PCHAR(c) {int cc=(c); if (func) (*func)(cc,arg); else *d++ = cc; retval++; }
        char nbuf[MAXNBUF];
        char *d;
        const char *p, *percent, *q;
@@ -572,6 +573,7 @@ kvcprintf(char const *fmt, void (*func)(int, void*), void *arg, int radix, __va_
        int dwidth, upper;
        char padc;
        int retval = 0, stop = 0;
+       int not_panic_cpu = (panic_cpu_gd != mycpu);
 
        num = 0;
        if (!func)
@@ -585,12 +587,17 @@ kvcprintf(char const *fmt, void (*func)(int, void*), void *arg, int radix, __va_
        if (radix < 2 || radix > 36)
                radix = 10;
 
+       if (not_panic_cpu && func == kputchar) {
+               crit_enter_hard();
+               spin_lock_wr(&cons_spin);
+       }
+
        for (;;) {
                padc = ' ';
                width = 0;
                while ((ch = (u_char)*fmt++) != '%' || stop) {
                        if (ch == '\0')
-                               return (retval);
+                               goto done;
                        PCHAR(ch);
                }
                percent = fmt - 1;
@@ -863,9 +870,26 @@ number:
                        break;
                }
        }
+done:
+       if (not_panic_cpu && func == kputchar) {
+               spin_unlock_wr(&cons_spin);
+               crit_exit_hard();
+       }
+       return (retval);
+}
+
 #undef PCHAR
+
+/*
+ * Called from the panic code
+ */
+void
+kvcreinitspin(void)
+{
+       spin_init(&cons_spin);
 }
 
+
 /*
  * Put character in log buffer with a particular priority.
  *
index 668edd8..17c2275 100644 (file)
@@ -2559,9 +2559,6 @@ outb(u_int port, u_char data)
 /* critical region when masking or unmasking interupts */
 struct spinlock_deprecated imen_spinlock;
 
-/* Make FAST_INTR() routines sequential */
-struct spinlock_deprecated fast_intr_spinlock;
-
 /* critical region for old style disable_intr/enable_intr */
 struct spinlock_deprecated mpintr_spinlock;
 
@@ -2574,9 +2571,6 @@ struct spinlock_deprecated mcount_spinlock;
 /* locks com (tty) data/hardware accesses: a FASTINTR() */
 struct spinlock_deprecated com_spinlock;
 
-/* locks kernel kprintfs */
-struct spinlock_deprecated cons_spinlock;
-
 /* lock regions around the clock hardware */
 struct spinlock_deprecated clock_spinlock;
 
@@ -2598,14 +2592,12 @@ init_locks(void)
 #endif
        /* DEPRECATED */
        spin_lock_init(&mcount_spinlock);
-       spin_lock_init(&fast_intr_spinlock);
        spin_lock_init(&intr_spinlock);
        spin_lock_init(&mpintr_spinlock);
        spin_lock_init(&imen_spinlock);
        spin_lock_init(&smp_rv_spinlock);
        spin_lock_init(&com_spinlock);
        spin_lock_init(&clock_spinlock);
-       spin_lock_init(&cons_spinlock);
 
        /* our token pool needs to work early */
        lwkt_token_pool_init();
index 866915f..00e4dad 100644 (file)
@@ -100,11 +100,3 @@ NON_GPROF_ENTRY(com_unlock)
        SPIN_UNLOCK(com_spinlock)
        NON_GPROF_RET
 
-NON_GPROF_ENTRY(cons_lock)
-       SPIN_LOCK(cons_spinlock)
-       NON_GPROF_RET
-
-NON_GPROF_ENTRY(cons_unlock)
-       SPIN_UNLOCK(cons_spinlock)
-       NON_GPROF_RET
-
index 64eb0ab..18eb57e 100644 (file)
@@ -1140,10 +1140,10 @@ putxyl(int x, int y, char *str, int len)
                break;
                
            default:
-               cnputc(*str++); /* not an escape */
+               kprintf("%c", *str++);  /* not an escape */
            }
        }else{
-           cnputc(*str++);             /* emit the character */
+           kprintf("%c", *str++);              /* emit the character */
        }
     }
 }
@@ -1347,10 +1347,8 @@ drawline(int row, int detail, DEV_LIST *list, int inverse, char *dhelp)
 /**
  ** drawlist
  **
- ** Displays (num) lines of the contents of (list) at (row), optionally displaying the
- ** port and IRQ fields as well if (detail) is nonzero
- **
- ** kprintf in the kernel is essentially useless, so we do most of the hard work ourselves here.
+ ** Displays (num) lines of the contents of (list) at (row), optionally
+ ** displaying the port and IRQ fields as well if (detail) is nonzero.
  **/
 static void 
 drawlist(int row, int num, int detail, DEV_LIST *list)
index 48346e2..b12bd6a 100644 (file)
@@ -148,8 +148,6 @@ void        imen_unlock(void);
 void   clock_lock(void);       /* disables int / spinlock combo */
 void   clock_unlock(void);
 
-extern struct spinlock_deprecated smp_rv_spinlock;
-
 void   spin_lock_deprecated(spinlock_t lock);
 void   spin_unlock_deprecated(spinlock_t lock);
 
index a6ccdfd..7b05e62 100644 (file)
@@ -148,8 +148,6 @@ void        imen_unlock(void);
 void   clock_lock(void);       /* disables int / spinlock combo */
 void   clock_unlock(void);
 
-extern struct spinlock_deprecated smp_rv_spinlock;
-
 void   spin_lock_deprecated(spinlock_t lock);
 void   spin_unlock_deprecated(spinlock_t lock);
 
index aab4f51..d4d90c1 100644 (file)
  */
 
 #include <sys/systm.h>
-
-#if JG
-/*
- * Global console locking functions
- */
-void
-cons_lock(void)
-{
-}
-
-void
-cons_unlock(void)
-{
-}
-#endif
index 7836d18..311ddc5 100644 (file)
@@ -2306,9 +2306,6 @@ outb(u_int port, u_char data)
 /* critical region when masking or unmasking interupts */
 struct spinlock_deprecated imen_spinlock;
 
-/* Make FAST_INTR() routines sequential */
-struct spinlock_deprecated fast_intr_spinlock;
-
 /* critical region for old style disable_intr/enable_intr */
 struct spinlock_deprecated mpintr_spinlock;
 
@@ -2321,15 +2318,9 @@ struct spinlock_deprecated mcount_spinlock;
 /* locks com (tty) data/hardware accesses: a FASTINTR() */
 struct spinlock_deprecated com_spinlock;
 
-/* locks kernel kprintfs */
-struct spinlock_deprecated cons_spinlock;
-
 /* lock regions around the clock hardware */
 struct spinlock_deprecated clock_spinlock;
 
-/* lock around the MP rendezvous */
-struct spinlock_deprecated smp_rv_spinlock;
-
 static void
 init_locks(void)
 {
@@ -2345,14 +2336,11 @@ init_locks(void)
 #endif
        /* DEPRECATED */
        spin_lock_init(&mcount_spinlock);
-       spin_lock_init(&fast_intr_spinlock);
        spin_lock_init(&intr_spinlock);
        spin_lock_init(&mpintr_spinlock);
        spin_lock_init(&imen_spinlock);
-       spin_lock_init(&smp_rv_spinlock);
        spin_lock_init(&com_spinlock);
        spin_lock_init(&clock_spinlock);
-       spin_lock_init(&cons_spinlock);
 
        /* our token pool needs to work early */
        lwkt_token_pool_init();
index 42c03a7..2988d6b 100644 (file)
@@ -101,11 +101,3 @@ NON_GPROF_ENTRY(com_unlock)
        SPIN_UNLOCK(com_spinlock)
        NON_GPROF_RET
 
-NON_GPROF_ENTRY(cons_lock)
-       SPIN_LOCK(cons_spinlock)
-       NON_GPROF_RET
-
-NON_GPROF_ENTRY(cons_unlock)
-       SPIN_UNLOCK(cons_spinlock)
-       NON_GPROF_RET
-
index 167a16f..36ab336 100644 (file)
 static int console_stolen_by_kernel;
 static struct kqueue_info *kqueue_console_info;
 
-/*
- * Global console locking functions
- */
-void
-cons_lock(void)
-{
-}
-
-void
-cons_unlock(void)
-{
-}
-
 /************************************************************************
  *                         CONSOLE DEVICE                              *
  ************************************************************************
index 7e82738..7835e24 100644 (file)
 static int console_stolen_by_kernel;
 static struct kqueue_info *kqueue_console_info;
 
-/*
- * Global console locking functions
- */
-void
-cons_lock(void)
-{
-}
-
-void
-cons_unlock(void)
-{
-}
-
 /************************************************************************
  *                         CONSOLE DEVICE                              *
  ************************************************************************
index c5e4c43..f0916ff 100644 (file)
@@ -181,6 +181,7 @@ void        init_param2 (int physpages);
 void   tablefull (const char *);
 int    kvcprintf (char const *, void (*)(int, void*), void *, int,
                      __va_list) __printflike(1, 0);
+void   kvcreinitspin(void);
 int    log (int, const char *, ...) __printflike(2, 3);
 void   logwakeup (void);
 void   log_console (struct uio *);
@@ -250,13 +251,6 @@ void       startprofclock (struct proc *);
 void   stopprofclock (struct proc *);
 void   setstatclockrate (int hzrate);
 
-/*
- * Console I/O spinlocks - these typically also hard-disable interrupts
- * for the duration.
- */
-void   cons_lock(void); 
-void   cons_unlock(void);
-
 /*
  * Kernel environment support functions and sundry.
  */
@@ -343,6 +337,7 @@ int rm_at_fork (forklist_fn function);
 typedef void (*watchdog_tickle_fn) (void);
 
 extern watchdog_tickle_fn      wdog_tickler;
+extern struct globaldata       *panic_cpu_gd;
 
 /* 
  * Common `proc' functions are declared here so that proc.h can be included