From owner-svn-src-head@FreeBSD.ORG Wed May 5 18:53:25 2010 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 1929F1065675; Wed, 5 May 2010 18:53:25 +0000 (UTC) (envelope-from jh@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [69.147.83.44]) by mx1.freebsd.org (Postfix) with ESMTP id 096408FC08; Wed, 5 May 2010 18:53:25 +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 o45IrO0L034085; Wed, 5 May 2010 18:53:24 GMT (envelope-from jh@svn.freebsd.org) Received: (from jh@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o45IrOS6034082; Wed, 5 May 2010 18:53:24 GMT (envelope-from jh@svn.freebsd.org) Message-Id: <201005051853.o45IrOS6034082@svn.freebsd.org> From: Jaakko Heinonen Date: Wed, 5 May 2010 18:53:24 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r207671 - head/sys/geom X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 May 2010 18:53:25 -0000 Author: jh Date: Wed May 5 18:53:24 2010 New Revision: 207671 URL: http://svn.freebsd.org/changeset/base/207671 Log: 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 Silence on: freebsd-geom Modified: head/sys/geom/geom.h head/sys/geom/geom_subr.c Modified: head/sys/geom/geom.h ============================================================================== --- head/sys/geom/geom.h Wed May 5 18:22:29 2010 (r207670) +++ head/sys/geom/geom.h Wed May 5 18:53:24 2010 (r207671) @@ -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: head/sys/geom/geom_subr.c ============================================================================== --- head/sys/geom/geom_subr.c Wed May 5 18:22:29 2010 (r207670) +++ head/sys/geom/geom_subr.c Wed May 5 18:53:24 2010 (r207671) @@ -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);