svn commit: r325227 - head/sys/geom

[Andriy Gapon]

Author: avg
Date: Tue Oct 31 10:10:13 2017
New Revision: 325227
URL: https://svnweb.freebsd.org/changeset/base/325227

Log:
  geom_slice: do not destroy softc until providers are gone
  
  At present, g_slice_orphan and g_slice_spoiled destroy the softc
  (struct g_slicer) even before calling g_wither_geom, so there can
  be active and incoming io requests at that time and g_slice_start
  can access the softc.
  
  This commit changes the code to destroy the softc only after all
  providers are closed.
  
  While there, a couple of small cleanups.
  
  Reported by:	Ben RUBSON <ben.rubson at gmail.com>
  Tested by:	Ben RUBSON <ben.rubson at gmail.com>
  Reviewed by:	mav, smh (earlier version)
  MFC after:	2 weeks
  Sponsored by:	Panzura
  Differential Revision: https://reviews.freebsd.org/D12809

Modified:
  head/sys/geom/geom_slice.c

Modified: head/sys/geom/geom_slice.c
==============================================================================
--- head/sys/geom/geom_slice.c	Tue Oct 31 09:58:51 2017	(r325226)
+++ head/sys/geom/geom_slice.c	Tue Oct 31 10:10:13 2017	(r325227)
@@ -43,7 +43,6 @@ __FBSDID("$FreeBSD$");
 #include <sys/bio.h>
 #include <sys/sysctl.h>
 #include <sys/proc.h>
-#include <sys/kthread.h>
 #include <sys/lock.h>
 #include <sys/mutex.h>
 #include <sys/errno.h>
@@ -127,6 +126,15 @@ g_slice_access(struct g_provider *pp, int dr, int dw, 
 	if ((cp->acr + dr) == 0 && (cp->acw + dw) == 0 && (cp->ace + de) == 1)
 		de--;
 	error = g_access(cp, dr, dw, de);
+
+	/*
+	 * 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_slice_free(gsp);
+
 	return (error);
 }
 
@@ -470,21 +478,32 @@ g_slice_conf_hot(struct g_geom *gp, u_int idx, off_t o
 }
 
 void
-g_slice_spoiled(struct g_consumer *cp)
+g_slice_orphan(struct g_consumer *cp)
 {
 	struct g_geom *gp;
-	struct g_slicer *gsp;
 
 	g_topology_assert();
 	gp = cp->geom;
-	g_trace(G_T_TOPOLOGY, "g_slice_spoiled(%p/%s)", cp, gp->name);
-	cp->flags |= G_CF_ORPHAN;
-	gsp = gp->softc;
-	gp->softc = NULL;
-	g_slice_free(gsp);
+	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_slice_access() will do that after the last close.
+	 */
+	if ((cp->acr + cp->acw + cp->ace) == 0)
+		g_slice_free(gp->softc);
 }
 
+void
+g_slice_spoiled(struct g_consumer *cp)
+{
+
+	g_trace(G_T_TOPOLOGY, "%s(%p/%s)", __func__, cp, cp->geom->name);
+	cp->flags |= G_CF_ORPHAN;
+	g_slice_orphan(cp);
+}
+
 int
 g_slice_destroy_geom(struct gctl_req *req, struct g_class *mp, struct g_geom *gp)
 {
@@ -507,10 +526,10 @@ g_slice_new(struct g_class *mp, u_int slices, struct g
 	gp = g_new_geomf(mp, "%s", pp->name);
 	gsp = g_slice_alloc(slices, extra);
 	gsp->start = start;
-	gp->access = g_slice_access;
-	gp->orphan = g_slice_orphan;
 	gp->softc = gsp;
 	gp->start = g_slice_start;
+	gp->access = g_slice_access;
+	gp->orphan = g_slice_orphan;
 	gp->spoiled = g_slice_spoiled;
 	if (gp->dumpconf == NULL)
 		gp->dumpconf = g_slice_dumpconf;
@@ -529,19 +548,4 @@ g_slice_new(struct g_class *mp, u_int slices, struct g
 		*vp = gsp->softc;
 	*cpp = cp;
 	return (gp);
-}
-
-void
-g_slice_orphan(struct g_consumer *cp)
-{
-	struct g_slicer *gsp;
-
-	g_trace(G_T_TOPOLOGY, "g_slice_orphan(%p/%s)", cp, cp->provider->name);
-	g_topology_assert();
-
-	/* XXX: Not good enough we leak the softc and its suballocations */
-	gsp = cp->geom->softc;
-	cp->geom->softc = NULL;
-	g_slice_free(gsp);
-	g_wither_geom(cp->geom, ENXIO);
 }