From owner-p4-projects@FreeBSD.ORG Tue Sep 30 14:30:38 2003 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 63FF116A4C1; Tue, 30 Sep 2003 14:30:38 -0700 (PDT) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2309416A4BF for ; Tue, 30 Sep 2003 14:30:38 -0700 (PDT) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 7E5B343F3F for ; Tue, 30 Sep 2003 14:30:37 -0700 (PDT) (envelope-from imp@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.9/8.12.9) with ESMTP id h8ULUbXJ025258 for ; Tue, 30 Sep 2003 14:30:37 -0700 (PDT) (envelope-from imp@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.9/8.12.9/Submit) id h8ULUbgU025255 for perforce@freebsd.org; Tue, 30 Sep 2003 14:30:37 -0700 (PDT) (envelope-from imp@freebsd.org) Date: Tue, 30 Sep 2003 14:30:37 -0700 (PDT) Message-Id: <200309302130.h8ULUbgU025255@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to imp@freebsd.org using -f From: Warner Losh To: Perforce Change Reviews Subject: PERFORCE change 38920 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Sep 2003 21:30:38 -0000 http://perforce.freebsd.org/chv.cgi?CH=38920 Change 38920 by imp@imp_koguchi on 2003/09/30 14:30:31 Document two races. One against setting/testing 'dead'. The other against si_drv1 being nulled out in destroy_dev(). Affected files ... .. //depot/doc/strawman-driver.c#4 edit Differences ... ==== //depot/doc/strawman-driver.c#4 (text+ko) ==== @@ -18,7 +18,11 @@ { foo_softc *sc; - sc = dev->si_drv1; + sc = dev->si_drv1; /* RACE #2, a */ + if (sc == NULL) + return EGONE; + if (sc->dead) /* RACE #1, a */ + return EGONE; ... case FOO_GERBIL: /* @@ -27,7 +31,7 @@ mtx_lock(&sc->mtx); cv_wait(&sc->cv, &sc->mtx); mtx_unlock(&sc->mtx); - if (sc->dead) + if (sc->dead) /* Race #1, b */ return EGONE; ... } @@ -35,7 +39,7 @@ static void foo_wakeup_my_sleepers(foo_softc *sc) { - sc->dead = 1; + sc->dead = 1; /* Race #1, c */ mtx_lock(&sc->mtx); cv_broadcast(&sc->cv); mtx_unlock(&sc->mtx); @@ -63,6 +67,8 @@ { sc = device_get_softc(dev); + sc->d->si_drv1 = NULL /* Race #2, b */ + foo_disable_intr(sc); /* disable hardware intr ??? */ /* Everybody active here */ @@ -96,3 +102,28 @@ * destroy_dev() makes sure that no devsw calls in flight. */ +/* Races */ + +/* #1 + * + * We have a race here. + * + * The test points 'a' and 'b' might lose the race with the set point 'c'. + * + * If 'c' wins the race with either 'a' or 'b', then we're OK. We properly + * bail out early. If we lose the race that's OK too. In 'b' case, we go + * ahead and maybe sleep in the cv_wait. However, since we're using a condvar + * in that case, we don't sleep and 'win' the race at 'c'. When we're + * racing against 'c', then we'll access state of the device after + * we've started destroying it. That's OK, so long as the driver knows + * what can/can't be active at this point. + */ + +/* #2 + * + * We need to lock the door in a race here with the destroy_dev() + * function setting to NULL before we can get in and do something + * at point 'a'. So at point 'b' we set it to NULL. If 'b' happens + * before 'a', that's OK, because sc is guaranteed to be valid for the + * life of the call to ioctl, or any of the devsw functions. + */