Date: Mon, 5 Apr 2010 12:26:12 +0300 From: Jaakko Heinonen <jh@FreeBSD.org> To: freebsd-geom@FreeBSD.org Subject: GEOM class unload deadlock Message-ID: <20100405092611.GA1948@a91-153-117-195.elisa-laajakaista.fi>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100405092611.GA1948>