kernel - Fix panic related to kqueue-based timers
authorMatthew Dillon <dillon@apollo.backplane.com>
Wed, 7 Sep 2011 22:48:07 +0000 (15:48 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Wed, 7 Sep 2011 22:48:07 +0000 (15:48 -0700)
* Fix a panic related to kqueue-based timers.  When a knote is destroyed it
  is possible for the callout_stop() to race a callout on another cpu that
  is blocked on a lock.

* Introduce callout_terminate() which stops a callout and ensures that no
  function calls for the callout is still in progress before returning.
  The kevent code now uses this function instead of callout_stop().

* We can't have this assurance for the normal callout_stop() function because
  it can deadlock a backend stuck waiting for a lock when a frontend holding
  that lock calls callout_stop().

Reported-by: Antonio Huete Jimenez <tuxillo@quantumachine.net>
sys/kern/kern_event.c
sys/kern/kern_timeout.c

index 25483fb..0c4c30c 100644 (file)
@@ -331,9 +331,8 @@ filt_proc(struct knote *kn, long hint)
 }
 
 /*
- * The callout interlocks with callout_stop() (or should), so the
- * knote should still be a valid structure.  However the timeout
- * can race a deletion so if KN_DELETING is set we just don't touch
+ * The callout interlocks with callout_terminate() but can still
+ * race a deletion so if KN_DELETING is set we just don't touch
  * the knote.
  */
 static void
@@ -390,13 +389,19 @@ filt_timerattach(struct knote *kn)
        return (0);
 }
 
+/*
+ * This function is called with the knote flagged locked but it is
+ * still possible to race a callout event due to the callback blocking.
+ * We must call callout_terminate() instead of callout_stop() to deal
+ * with the race.
+ */
 static void
 filt_timerdetach(struct knote *kn)
 {
        struct callout *calloutp;
 
        calloutp = (struct callout *)kn->kn_hook;
-       callout_stop(calloutp);
+       callout_terminate(calloutp);
        FREE(calloutp, M_KQUEUE);
        kq_ncallouts--;
 }
index 09b8470..e1b695c 100644 (file)
 struct softclock_pcpu {
        struct callout_tailq *callwheel;
        struct callout * volatile next;
+       struct callout *running;/* currently running callout */
        int softticks;          /* softticks index */
        int curticks;           /* per-cpu ticks counter */
        int isrunning;
@@ -291,6 +292,7 @@ loop:
                        sc->next = TAILQ_NEXT(c, c_links.tqe);
                        TAILQ_REMOVE(bucket, c, c_links.tqe);
 
+                       sc->running = c;
                        c_func = c->c_func;
                        c_arg = c->c_arg;
                        c->c_func = NULL;
@@ -299,6 +301,7 @@ loop:
                        crit_exit();
                        c_func(c_arg);
                        crit_enter();
+                       sc->running = NULL;
                        /* NOTE: list may have changed */
                }
                ++sc->softticks;
@@ -359,7 +362,7 @@ callout_reset(struct callout *c, int to_ticks, void (*ftn)(void *),
        sc = &softclock_pcpu_ary[gd->gd_cpuid];
        crit_enter_gd(gd);
 
-       if (c->c_flags & CALLOUT_PENDING)
+       if (c->c_flags & CALLOUT_ACTIVE)
                callout_stop(c);
 
        if (to_ticks <= 0)
@@ -389,6 +392,9 @@ callout_reset(struct callout *c, int to_ticks, void (*ftn)(void *),
  * structure regardless of cpu.
  *
  * WARNING! This routine may be called from an IPI
+ *
+ * WARNING! This function can return while it's c_func is still running
+ *         in the callout thread, a secondary check may be needed.
  */
 int
 callout_stop(struct callout *c)
@@ -477,6 +483,38 @@ callout_stop(struct callout *c)
        return (1);
 }
 
+/*
+ * Terminate a callout
+ *
+ * This function will stop any pending callout and also block while the
+ * callout's function is running.  It should only be used in cases where
+ * no deadlock is possible (due to the callout function acquiring locks
+ * that the current caller of callout_terminate() already holds), when
+ * the caller is ready to destroy the callout structure.
+ *
+ * This function clears the CALLOUT_DID_INIT flag.
+ *
+ * lwkt_token locks are ok.
+ */
+void
+callout_terminate(struct callout *c)
+{
+       softclock_pcpu_t sc;
+
+       if (c->c_flags & CALLOUT_DID_INIT) {
+               callout_stop(c);
+               sc = &softclock_pcpu_ary[c->c_gd->gd_cpuid];
+               if (sc->running == c) {
+                       kprintf("callout_terminate: race averted func %p\n",
+                               c->c_func);
+                       while (sc->running == c)
+                               tsleep(&sc->running, 0, "crace", 1);
+               }
+               KKASSERT((c->c_flags & (CALLOUT_PENDING|CALLOUT_ACTIVE)) == 0);
+               c->c_flags &= ~CALLOUT_DID_INIT;
+       }
+}
+
 /*
  * Prepare a callout structure for use by callout_reset() and/or 
  * callout_stop().  The MP version of this routine requires that the callback