PERFORCE change 38920 for review

Warner Losh imp at FreeBSD.org
Tue Sep 30 14:30:38 PDT 2003


http://perforce.freebsd.org/chv.cgi?CH=38920

Change 38920 by imp at 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.
+ */


More information about the p4-projects mailing list