Date: Tue, 30 Sep 2003 14:30:37 -0700 (PDT) From: Warner Losh <imp@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 38920 for review Message-ID: <200309302130.h8ULUbgU025255@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
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. + */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200309302130.h8ULUbgU025255>