From owner-p4-projects@FreeBSD.ORG Wed Sep 5 21:21:12 2007 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 28DEE16A420; Wed, 5 Sep 2007 21:21:12 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E3AD116A419 for ; Wed, 5 Sep 2007 21:21:11 +0000 (UTC) (envelope-from zec@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id D5EB113C467 for ; Wed, 5 Sep 2007 21:21:11 +0000 (UTC) (envelope-from zec@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.1/8.14.1) with ESMTP id l85LLBUi009535 for ; Wed, 5 Sep 2007 21:21:11 GMT (envelope-from zec@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id l85LLBx2009532 for perforce@freebsd.org; Wed, 5 Sep 2007 21:21:11 GMT (envelope-from zec@FreeBSD.org) Date: Wed, 5 Sep 2007 21:21:11 GMT Message-Id: <200709052121.l85LLBx2009532@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to zec@FreeBSD.org using -f From: Marko Zec To: Perforce Change Reviews Cc: Subject: PERFORCE change 126107 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Sep 2007 21:21:12 -0000 http://perforce.freebsd.org/chv.cgi?CH=126107 Change 126107 by zec@zec_tpx32 on 2007/09/05 21:20:46 I've been foot-shooting myself several times by calling callout_init() on already active callout handles. This change examines the entire callwheel on callout_init() invocations for such a condition and panics if the handle is already linked in. This replaces a check for attempts at double-linking a callout handle in callout_reset(), which was much more expensive (callout_reset() is called much more frequently than callout_init()) and couldn't directly locate the offending callout_init() line. This extra check could be probably usefull outside of the scope of network stack virtualization changes / code... Affected files ... .. //depot/projects/vimage/src/sys/kern/kern_timeout.c#5 edit Differences ... ==== //depot/projects/vimage/src/sys/kern/kern_timeout.c#5 (text+ko) ==== @@ -37,8 +37,6 @@ #include __FBSDID("$FreeBSD: src/sys/kern/kern_timeout.c,v 1.104 2007/06/26 21:42:01 attilio Exp $"); -#include "opt_vimage.h" - #include #include #include @@ -75,6 +73,9 @@ struct callout_tailq *callwheel; int softticks; /* Like ticks, but for softclock(). */ struct mtx callout_lock; +#ifdef INVARIANTS +static int callwheel_initialized = 0; +#endif static struct callout *nextsoftcheck; /* Next callout to be checked. */ @@ -145,6 +146,9 @@ TAILQ_INIT(&callwheel[i]); } mtx_init(&callout_lock, "callout", NULL, MTX_SPIN | MTX_RECURSE); +#ifdef INVARIANTS + callwheel_initialized = 1; +#endif } /* @@ -479,23 +483,6 @@ c->c_flags |= (CALLOUT_ACTIVE | CALLOUT_PENDING); c->c_func = ftn; c->c_time = ticks + to_ticks; -#if (defined(VIMAGE) && defined(INVARIANTS)) - /* - * MARKO XXX - * - * I'm suspecting that some lockups might have been caused by - * a single callout handle being scheduled multiple times. - * This loop examines the entire callwhell before inserting a - * new handle, and if the handle is already linked in it panics. - */ - int callwheel_iter; - struct callout *c_iter; - for (callwheel_iter = 0; callwheel_iter <= callwheelmask; - callwheel_iter++) - TAILQ_FOREACH(c_iter, &callwheel[callwheel_iter], c_links.tqe) - if (c_iter == c) - panic("finally got you!"); -#endif TAILQ_INSERT_TAIL(&callwheel[c->c_time & callwheelmask], c, c_links.tqe); CTR5(KTR_CALLOUT, "%sscheduled %p func %p arg %p in %d", @@ -621,11 +608,36 @@ return (1); } +#ifdef INVARIANTS +/* + * Examine the entire callwhell before initializing a new handle, + * and panic if the handle was already linked in. + */ +#define CALLWHEEL_CHECK(c) \ + if (callwheel_initialized) { \ + int callwheel_iter; \ + struct callout *c_iter; \ + \ + mtx_lock_spin(&callout_lock); \ + for (callwheel_iter = 0; callwheel_iter <= callwheelmask; \ + callwheel_iter++) \ + TAILQ_FOREACH(c_iter, &callwheel[callwheel_iter], \ + c_links.tqe) \ + if (c_iter == c) \ + panic("%s() for active handle!", \ + __FUNCTION__); \ + mtx_unlock_spin(&callout_lock); \ + } +#else +#define CALLWHEEL_CHECK(c) +#endif /* INVARIANTS */ + void callout_init(c, mpsafe) struct callout *c; int mpsafe; { + CALLWHEEL_CHECK(c); bzero(c, sizeof *c); if (mpsafe) { c->c_mtx = NULL; @@ -642,6 +654,7 @@ struct mtx *mtx; int flags; { + CALLWHEEL_CHECK(c); bzero(c, sizeof *c); c->c_mtx = mtx; KASSERT((flags & ~(CALLOUT_RETURNUNLOCKED|CALLOUT_NETGIANT)) == 0,