GEOM class unload deadlock

[Jaakko Heinonen]

Hi,

Below is  a patch to fix a possible deadlock between g_unload_class()
and withering. Probably the easiest way to reproduce the deadlock is to
load geom_mbr.ko module and then try to unload it.

Some details are explained this message:

	http://docs.freebsd.org/cgi/mid.cgi?20081216210311.GA5229

---

Fix deadlock between GEOM class unloading and withering. Withering can't
proceed while g_unload_class() blocks the event thread. Fix this by
queuing the unload event repeatedly until withering has finished.

PR:		kern/139847

%%%
Index: sys/geom/geom_subr.c
===================================================================
--- sys/geom/geom_subr.c	(revision 206155)
+++ sys/geom/geom_subr.c	(working copy)
@@ -146,17 +146,8 @@ g_unload_class(void *arg, int flag)
 	G_VALID_CLASS(mp);
 	g_trace(G_T_TOPOLOGY, "g_unload_class(%s)", mp->name);
 
-	/*
-	 * We allow unloading if we have no geoms, or a class
-	 * method we can use to get rid of them.
-	 */
-	if (!LIST_EMPTY(&mp->geom) && mp->destroy_geom == NULL) {
-		hh->error = EOPNOTSUPP;
-		return;
-	}
-
-	/* We refuse to unload if anything is open */
 	LIST_FOREACH(gp, &mp->geom, geom) {
+		/* We refuse to unload if anything is open */
 		LIST_FOREACH(pp, &gp->provider, provider)
 			if (pp->acr || pp->acw || pp->ace) {
 				hh->error = EBUSY;
@@ -167,6 +158,23 @@ g_unload_class(void *arg, int flag)
 				hh->error = EBUSY;
 				return;
 			}
+		/*
+		 * Check for unfinished withering. We are in the event
+		 * thread, so withering can't proceed.
+		 */
+		if (gp->flags & G_GEOM_WITHER) {
+			hh->error = EDEADLK;
+			return;
+		}
+	}
+
+	/*
+	 * We allow unloading if we have no geoms, or a class
+	 * method we can use to get rid of them.
+	 */
+	if (!LIST_EMPTY(&mp->geom) && mp->destroy_geom == NULL) {
+		hh->error = EOPNOTSUPP;
+		return;
 	}
 
 	/* Bar new entries */
@@ -181,6 +189,12 @@ g_unload_class(void *arg, int flag)
 		error = mp->destroy_geom(NULL, mp, gp);
 		if (error != 0)
 			break;
+		/* Return, if withering needs to proceed. */
+		if (gp == LIST_FIRST(&mp->geom) && gp->flags & G_GEOM_WITHER) {
+			hh->error = EDEADLK;
+			return;
+		}
+
 	}
 	if (error == 0) {
 		if (mp->fini != NULL)
@@ -233,9 +247,12 @@ g_modevent(module_t mod, int type, void 
 		break;
 	case MOD_UNLOAD:
 		g_trace(G_T_TOPOLOGY, "g_modevent(%s, UNLOAD)", hh->mp->name);
-		error = g_waitfor_event(g_unload_class, hh, M_WAITOK, NULL);
-		if (error == 0)
-			error = hh->error;
+		do {
+			error = g_waitfor_event(g_unload_class, hh, M_WAITOK,
+			    NULL);
+			if (error == 0)
+				error = hh->error;
+		} while (error == EDEADLK);
 		if (error == 0) {
 			KASSERT(LIST_EMPTY(&hh->mp->geom),
 			    ("Unloaded class (%s) still has geom", hh->mp->name));
%%%

-- 
Jaakko