From b90e37ec76a8f3cfd0c32fa2bd292cb261773d85 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Wed, 7 Sep 2011 15:48:07 -0700 Subject: [PATCH] kernel - Fix panic related to kqueue-based timers * 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 --- sys/kern/kern_event.c | 13 +++++++++---- sys/kern/kern_timeout.c | 40 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/sys/kern/kern_event.c b/sys/kern/kern_event.c index 25483fb77e..0c4c30cfab 100644 --- a/sys/kern/kern_event.c +++ b/sys/kern/kern_event.c @@ -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--; } diff --git a/sys/kern/kern_timeout.c b/sys/kern/kern_timeout.c index 09b8470ed1..e1b695c8b7 100644 --- a/sys/kern/kern_timeout.c +++ b/sys/kern/kern_timeout.c @@ -117,6 +117,7 @@ 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 -- 2.41.0