Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 27 Dec 2019 21:44:13 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r356138 - head/sys/geom/uzip
Message-ID:  <201912272144.xBRLiDl1085121@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mav
Date: Fri Dec 27 21:44:13 2019
New Revision: 356138
URL: https://svnweb.freebsd.org/changeset/base/356138

Log:
  Fix GEOM_UZIP orphanization.
  
  Previous code destroyed softc even with provider still open, that resulted
  in panic under load.  This change postpones the free till the final close,
  when we know for sure there will be no more I/O requests.
  
  MFC after:	2 weeks
  Sponsored by:	iXsystems, Inc.

Modified:
  head/sys/geom/uzip/g_uzip.c

Modified: head/sys/geom/uzip/g_uzip.c
==============================================================================
--- head/sys/geom/uzip/g_uzip.c	Fri Dec 27 20:37:14 2019	(r356137)
+++ head/sys/geom/uzip/g_uzip.c	Fri Dec 27 21:44:13 2019	(r356138)
@@ -143,13 +143,12 @@ static void g_uzip_read_done(struct bio *bp);
 static void g_uzip_do(struct g_uzip_softc *, struct bio *bp);
 
 static void
-g_uzip_softc_free(struct g_uzip_softc *sc, struct g_geom *gp)
+g_uzip_softc_free(struct g_geom *gp)
 {
+	struct g_uzip_softc *sc = gp->softc;
 
-	if (gp != NULL) {
-		DPRINTF(GUZ_DBG_INFO, ("%s: %d requests, %d cached\n",
-		    gp->name, sc->req_total, sc->req_cached));
-	}
+	DPRINTF(GUZ_DBG_INFO, ("%s: %d requests, %d cached\n",
+	    gp->name, sc->req_total, sc->req_cached));
 
 	mtx_lock(&sc->queue_mtx);
 	sc->wrkthr_flags |= GUZ_SHUTDOWN;
@@ -166,6 +165,7 @@ g_uzip_softc_free(struct g_uzip_softc *sc, struct g_ge
 	mtx_destroy(&sc->last_mtx);
 	free(sc->last_buf, M_GEOM_UZIP);
 	free(sc, M_GEOM_UZIP);
+	gp->softc = NULL;
 }
 
 static int
@@ -507,20 +507,35 @@ g_uzip_orphan(struct g_consumer *cp)
 {
 	struct g_geom *gp;
 
-	g_trace(G_T_TOPOLOGY, "%s(%p/%s)", __func__, cp, cp->provider->name);
 	g_topology_assert();
-
+	G_VALID_CONSUMER(cp);
 	gp = cp->geom;
-	g_uzip_softc_free(gp->softc, gp);
-	gp->softc = NULL;
+	g_trace(G_T_TOPOLOGY, "%s(%p/%s)", __func__, cp, gp->name);
 	g_wither_geom(gp, ENXIO);
+
+	/*
+	 * We can safely free the softc now if there are no accesses,
+	 * otherwise g_uzip_access() will do that after the last close.
+	 */
+	if ((cp->acr + cp->acw + cp->ace) == 0)
+		g_uzip_softc_free(gp);
 }
 
+static void
+g_uzip_spoiled(struct g_consumer *cp)
+{
+
+	g_trace(G_T_TOPOLOGY, "%s(%p/%s)", __func__, cp, cp->geom->name);
+	cp->flags |= G_CF_ORPHAN;
+	g_uzip_orphan(cp);
+}
+
 static int
 g_uzip_access(struct g_provider *pp, int dr, int dw, int de)
 {
 	struct g_geom *gp;
 	struct g_consumer *cp;
+	int error;
 
 	gp = pp->geom;
 	cp = LIST_FIRST(&gp->consumer);
@@ -529,22 +544,17 @@ g_uzip_access(struct g_provider *pp, int dr, int dw, i
 	if (cp->acw + dw > 0)
 		return (EROFS);
 
-	return (g_access(cp, dr, dw, de));
-}
+	error = g_access(cp, dr, dw, de);
 
-static void
-g_uzip_spoiled(struct g_consumer *cp)
-{
-	struct g_geom *gp;
+	/*
+	 * Free the softc if all providers have been closed and this geom
+	 * is being removed.
+	 */
+	if (error == 0 && (gp->flags & G_GEOM_WITHER) != 0 &&
+	    (cp->acr + cp->acw + cp->ace) == 0)
+		g_uzip_softc_free(gp);
 
-	G_VALID_CONSUMER(cp);
-	gp = cp->geom;
-	g_trace(G_T_TOPOLOGY, "%s(%p/%s)", __func__, cp, gp->name);
-	g_topology_assert();
-
-	g_uzip_softc_free(gp->softc, gp);
-	gp->softc = NULL;
-	g_wither_geom(gp, ENXIO);
+	return (error);
 }
 
 static int
@@ -954,10 +964,8 @@ g_uzip_destroy_geom(struct gctl_req *req, struct g_cla
 	if (pp->acr > 0 || pp->acw > 0 || pp->ace > 0)
 		return (EBUSY);
 
-	g_uzip_softc_free(gp->softc, gp);
-	gp->softc = NULL;
 	g_wither_geom(gp, ENXIO);
-
+	g_uzip_softc_free(gp);
 	return (0);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201912272144.xBRLiDl1085121>