From owner-freebsd-geom@FreeBSD.ORG Mon Apr 5 09:26:15 2010 Return-Path: Delivered-To: freebsd-geom@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 74BDE1065670 for ; Mon, 5 Apr 2010 09:26:15 +0000 (UTC) (envelope-from jh@FreeBSD.org) Received: from gw01.mail.saunalahti.fi (gw01.mail.saunalahti.fi [195.197.172.115]) by mx1.freebsd.org (Postfix) with ESMTP id 0BBDE8FC26 for ; Mon, 5 Apr 2010 09:26:14 +0000 (UTC) Received: from a91-153-117-195.elisa-laajakaista.fi (a91-153-117-195.elisa-laajakaista.fi [91.153.117.195]) by gw01.mail.saunalahti.fi (Postfix) with SMTP id 65EED1513EA for ; Mon, 5 Apr 2010 12:26:12 +0300 (EEST) Date: Mon, 5 Apr 2010 12:26:12 +0300 From: Jaakko Heinonen To: freebsd-geom@FreeBSD.org Message-ID: <20100405092611.GA1948@a91-153-117-195.elisa-laajakaista.fi> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) Cc: Subject: GEOM class unload deadlock X-BeenThere: freebsd-geom@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: GEOM-specific discussions and implementations List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Apr 2010 09:26:15 -0000 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