From owner-svn-src-all@FreeBSD.ORG Fri Jul 30 13:23:22 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 14FDE106566B; Fri, 30 Jul 2010 13:23:22 +0000 (UTC) (envelope-from jh@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 02FA68FC15; Fri, 30 Jul 2010 13:23:22 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o6UDNLQa072475; Fri, 30 Jul 2010 13:23:21 GMT (envelope-from jh@svn.freebsd.org) Received: (from jh@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o6UDNLcA072472; Fri, 30 Jul 2010 13:23:21 GMT (envelope-from jh@svn.freebsd.org) Message-Id: <201007301323.o6UDNLcA072472@svn.freebsd.org> From: Jaakko Heinonen Date: Fri, 30 Jul 2010 13:23:21 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r210646 - stable/8/sys/geom X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Jul 2010 13:23:22 -0000 Author: jh Date: Fri Jul 30 13:23:21 2010 New Revision: 210646 URL: http://svn.freebsd.org/changeset/base/210646 Log: MFC r207671: Fix deadlock between GEOM class unloading and withering. Withering can't proceed while g_unload_class() blocks the event thread. Fix this by not running g_unload_class() as a GEOM event and dropping the topology lock when withering needs to proceed. PR: kern/139847 Modified: stable/8/sys/geom/geom.h stable/8/sys/geom/geom_subr.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) Modified: stable/8/sys/geom/geom.h ============================================================================== --- stable/8/sys/geom/geom.h Fri Jul 30 12:56:34 2010 (r210645) +++ stable/8/sys/geom/geom.h Fri Jul 30 13:23:21 2010 (r210646) @@ -353,6 +353,9 @@ g_free(void *ptr) sx_assert(&topology_lock, SX_UNLOCKED); \ } while (0) +#define g_topology_sleep(chan, timo) \ + sx_sleep(chan, &topology_lock, 0, "gtopol", timo) + #define DECLARE_GEOM_CLASS(class, name) \ static moduledata_t name##_mod = { \ #name, g_modevent, &class \ Modified: stable/8/sys/geom/geom_subr.c ============================================================================== --- stable/8/sys/geom/geom_subr.c Fri Jul 30 12:56:34 2010 (r210645) +++ stable/8/sys/geom/geom_subr.c Fri Jul 30 13:23:21 2010 (r210646) @@ -134,65 +134,73 @@ g_load_class(void *arg, int flag) } } -static void -g_unload_class(void *arg, int flag) +static int +g_unload_class(struct g_class *mp) { - struct g_hh00 *hh; - struct g_class *mp; struct g_geom *gp; struct g_provider *pp; struct g_consumer *cp; int error; - g_topology_assert(); - hh = arg; - mp = hh->mp; - G_VALID_CLASS(mp); + g_topology_lock(); 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 */ +retry: + G_VALID_CLASS(mp); 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; - return; + g_topology_unlock(); + return (EBUSY); } LIST_FOREACH(cp, &gp->consumer, consumer) if (cp->acr || cp->acw || cp->ace) { - hh->error = EBUSY; - return; + g_topology_unlock(); + return (EBUSY); } + /* If the geom is withering, wait for it to finish. */ + if (gp->flags & G_GEOM_WITHER) { + g_topology_sleep(mp, 1); + goto retry; + } + } + + /* + * 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) { + g_topology_unlock(); + return (EOPNOTSUPP); } /* Bar new entries */ mp->taste = NULL; mp->config = NULL; - error = 0; + LIST_FOREACH(gp, &mp->geom, geom) { + error = mp->destroy_geom(NULL, mp, gp); + if (error != 0) { + g_topology_unlock(); + return (error); + } + } + /* Wait for withering to finish. */ for (;;) { gp = LIST_FIRST(&mp->geom); if (gp == NULL) break; - error = mp->destroy_geom(NULL, mp, gp); - if (error != 0) - break; + KASSERT(gp->flags & G_GEOM_WITHER, + ("Non-withering geom in class %s", mp->name)); + g_topology_sleep(mp, 1); } - if (error == 0) { - if (mp->fini != NULL) - mp->fini(mp); - LIST_REMOVE(mp, class); - } - hh->error = error; - return; + G_VALID_CLASS(mp); + if (mp->fini != NULL) + mp->fini(mp); + LIST_REMOVE(mp, class); + g_topology_unlock(); + + return (0); } int @@ -213,12 +221,12 @@ g_modevent(module_t mod, int type, void g_ignition++; g_init(); } - hh = g_malloc(sizeof *hh, M_WAITOK | M_ZERO); - hh->mp = data; error = EOPNOTSUPP; switch (type) { case MOD_LOAD: - g_trace(G_T_TOPOLOGY, "g_modevent(%s, LOAD)", hh->mp->name); + g_trace(G_T_TOPOLOGY, "g_modevent(%s, LOAD)", mp->name); + hh = g_malloc(sizeof *hh, M_WAITOK | M_ZERO); + hh->mp = mp; /* * Once the system is not cold, MOD_LOAD calls will be * from the userland and the g_event thread will be able @@ -236,18 +244,14 @@ 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; + g_trace(G_T_TOPOLOGY, "g_modevent(%s, UNLOAD)", mp->name); + DROP_GIANT(); + error = g_unload_class(mp); + PICKUP_GIANT(); if (error == 0) { - KASSERT(LIST_EMPTY(&hh->mp->geom), - ("Unloaded class (%s) still has geom", hh->mp->name)); + KASSERT(LIST_EMPTY(&mp->geom), + ("Unloaded class (%s) still has geom", mp->name)); } - g_free(hh); - break; - default: - g_free(hh); break; } return (error);