[Midnightbsd-cvs] src [9040] trunk/sys/geom: refactor disk disconnect and geom destroy code.

laffer1 at midnightbsd.org laffer1 at midnightbsd.org
Sat Oct 1 05:56:11 EDT 2016


Revision: 9040
          http://svnweb.midnightbsd.org/src/?rev=9040
Author:   laffer1
Date:     2016-10-01 05:56:11 -0400 (Sat, 01 Oct 2016)
Log Message:
-----------
refactor disk disconnect and geom destroy code. do not close opened consumer on disconnect.

Modified Paths:
--------------
    trunk/sys/geom/concat/g_concat.c
    trunk/sys/geom/concat/g_concat.h
    trunk/sys/geom/stripe/g_stripe.c

Modified: trunk/sys/geom/concat/g_concat.c
===================================================================
--- trunk/sys/geom/concat/g_concat.c	2016-10-01 09:55:20 UTC (rev 9039)
+++ trunk/sys/geom/concat/g_concat.c	2016-10-01 09:56:11 UTC (rev 9040)
@@ -119,25 +119,33 @@
 	struct g_consumer *cp;
 	struct g_concat_softc *sc;
 
+	g_topology_assert();
 	KASSERT(disk->d_consumer != NULL, ("Non-valid disk in %s.", __func__));
 	sc = disk->d_softc;
 	cp = disk->d_consumer;
 
-	G_CONCAT_DEBUG(0, "Disk %s removed from %s.", cp->provider->name,
-	    sc->sc_name);
+	if (!disk->d_removed) {
+		G_CONCAT_DEBUG(0, "Disk %s removed from %s.",
+		    cp->provider->name, sc->sc_name);
+		disk->d_removed = 1;
+	}
 
-	disk->d_consumer = NULL;
 	if (sc->sc_provider != NULL) {
 		sc->sc_provider->flags |= G_PF_WITHER;
+		G_CONCAT_DEBUG(0, "Device %s deactivated.",
+		    sc->sc_provider->name);
 		g_orphan_provider(sc->sc_provider, ENXIO);
 		sc->sc_provider = NULL;
-		G_CONCAT_DEBUG(0, "Device %s removed.", sc->sc_name);
 	}
 
 	if (cp->acr > 0 || cp->acw > 0 || cp->ace > 0)
-		g_access(cp, -cp->acr, -cp->acw, -cp->ace);
+		return;
+	disk->d_consumer = NULL;
 	g_detach(cp);
 	g_destroy_consumer(cp);
+	/* If there are no valid disks anymore, remove device. */
+	if (LIST_EMPTY(&sc->sc_geom->consumer))
+		g_concat_destroy(sc, 1);
 }
 
 static void
@@ -157,38 +165,19 @@
 	if (disk == NULL)	/* Possible? */
 		return;
 	g_concat_remove_disk(disk);
-
-	/* If there are no valid disks anymore, remove device. */
-	if (g_concat_nvalid(sc) == 0)
-		g_concat_destroy(sc, 1);
 }
 
 static int
 g_concat_access(struct g_provider *pp, int dr, int dw, int de)
 {
-	struct g_consumer *cp1, *cp2;
-	struct g_concat_softc *sc;
+	struct g_consumer *cp1, *cp2, *tmp;
+	struct g_concat_disk *disk;
 	struct g_geom *gp;
 	int error;
 
+	g_topology_assert();
 	gp = pp->geom;
-	sc = gp->softc;
 
-	if (sc == NULL) {
-		/*
-		 * It looks like geom is being withered.
-		 * In that case we allow only negative requests.
-		 */
-		KASSERT(dr <= 0 && dw <= 0 && de <= 0,
-		    ("Positive access request (device=%s).", pp->name));
-		if ((pp->acr + dr) == 0 && (pp->acw + dw) == 0 &&
-		    (pp->ace + de) == 0) {
-			G_CONCAT_DEBUG(0, "Device %s definitely destroyed.",
-			    gp->name);
-		}
-		return (0);
-	}
-
 	/* On first open, grab an extra "exclusive" bit */
 	if (pp->acr == 0 && pp->acw == 0 && pp->ace == 0)
 		de++;
@@ -196,22 +185,24 @@
 	if ((pp->acr + dr) == 0 && (pp->acw + dw) == 0 && (pp->ace + de) == 0)
 		de--;
 
-	error = ENXIO;
-	LIST_FOREACH(cp1, &gp->consumer, consumer) {
+	LIST_FOREACH_SAFE(cp1, &gp->consumer, consumer, tmp) {
 		error = g_access(cp1, dr, dw, de);
-		if (error == 0)
-			continue;
-		/*
-		 * If we fail here, backout all previous changes.
-		 */
-		LIST_FOREACH(cp2, &gp->consumer, consumer) {
-			if (cp1 == cp2)
-				return (error);
-			g_access(cp2, -dr, -dw, -de);
+		if (error != 0)
+			goto fail;
+		disk = cp1->private;
+		if (cp1->acr == 0 && cp1->acw == 0 && cp1->ace == 0 &&
+		    disk->d_removed) {
+			g_concat_remove_disk(disk); /* May destroy geom. */
 		}
-		/* NOTREACHED */
 	}
+	return (0);
 
+fail:
+	LIST_FOREACH(cp2, &gp->consumer, consumer) {
+		if (cp1 == cp2)
+			break;
+		g_access(cp2, -dr, -dw, -de);
+	}
 	return (error);
 }
 
@@ -393,6 +384,7 @@
 	u_int no, sectorsize = 0;
 	off_t start;
 
+	g_topology_assert();
 	if (g_concat_nvalid(sc) != sc->sc_ndisks)
 		return;
 
@@ -421,7 +413,7 @@
 	sc->sc_provider = pp;
 	g_error_provider(pp, 0);
 
-	G_CONCAT_DEBUG(0, "Device %s activated.", sc->sc_name);
+	G_CONCAT_DEBUG(0, "Device %s activated.", sc->sc_provider->name);
 }
 
 static int
@@ -463,6 +455,7 @@
 	struct g_geom *gp;
 	int error;
 
+	g_topology_assert();
 	/* Metadata corrupted? */
 	if (no >= sc->sc_ndisks)
 		return (EINVAL);
@@ -511,6 +504,7 @@
 	disk->d_softc = sc;
 	disk->d_start = 0;	/* not yet */
 	disk->d_end = 0;	/* not yet */
+	disk->d_removed = 0;
 
 	G_CONCAT_DEBUG(0, "Disk %s attached to %s.", pp->name, sc->sc_name);
 
@@ -578,8 +572,8 @@
 g_concat_destroy(struct g_concat_softc *sc, boolean_t force)
 {
 	struct g_provider *pp;
+	struct g_consumer *cp, *cp1;
 	struct g_geom *gp;
-	u_int no;
 
 	g_topology_assert();
 
@@ -599,12 +593,15 @@
 		}
 	}
 
-	for (no = 0; no < sc->sc_ndisks; no++) {
-		if (sc->sc_disks[no].d_consumer != NULL)
-			g_concat_remove_disk(&sc->sc_disks[no]);
+	gp = sc->sc_geom;
+	LIST_FOREACH_SAFE(cp, &gp->consumer, consumer, cp1) {
+		g_concat_remove_disk(cp->private);
+		if (cp1 == NULL)
+			return (0);	/* Recursion happened. */
 	}
+	if (!LIST_EMPTY(&gp->consumer))
+		return (EINPROGRESS);
 
-	gp = sc->sc_geom;
 	gp->softc = NULL;
 	KASSERT(sc->sc_provider == NULL, ("Provider still exists? (device=%s)",
 	    gp->name));
@@ -611,12 +608,8 @@
 	free(sc->sc_disks, M_CONCAT);
 	free(sc, M_CONCAT);
 
-	pp = LIST_FIRST(&gp->provider);
-	if (pp == NULL || (pp->acr == 0 && pp->acw == 0 && pp->ace == 0))
-		G_CONCAT_DEBUG(0, "Device %s destroyed.", gp->name);
-
+	G_CONCAT_DEBUG(0, "Device %s destroyed.", gp->name);
 	g_wither_geom(gp, ENXIO);
-
 	return (0);
 }
 

Modified: trunk/sys/geom/concat/g_concat.h
===================================================================
--- trunk/sys/geom/concat/g_concat.h	2016-10-01 09:55:20 UTC (rev 9039)
+++ trunk/sys/geom/concat/g_concat.h	2016-10-01 09:56:11 UTC (rev 9040)
@@ -73,6 +73,7 @@
 	struct g_concat_softc	*d_softc;
 	off_t			 d_start;
 	off_t			 d_end;
+	int			 d_removed;
 };
 
 struct g_concat_softc {

Modified: trunk/sys/geom/stripe/g_stripe.c
===================================================================
--- trunk/sys/geom/stripe/g_stripe.c	2016-10-01 09:55:20 UTC (rev 9039)
+++ trunk/sys/geom/stripe/g_stripe.c	2016-10-01 09:56:11 UTC (rev 9040)
@@ -162,28 +162,35 @@
 g_stripe_remove_disk(struct g_consumer *cp)
 {
 	struct g_stripe_softc *sc;
-	u_int no;
 
+	g_topology_assert();
 	KASSERT(cp != NULL, ("Non-valid disk in %s.", __func__));
-	sc = (struct g_stripe_softc *)cp->private;
+	sc = (struct g_stripe_softc *)cp->geom->softc;
 	KASSERT(sc != NULL, ("NULL sc in %s.", __func__));
-	no = cp->index;
 
-	G_STRIPE_DEBUG(0, "Disk %s removed from %s.", cp->provider->name,
-	    sc->sc_name);
+	if (cp->private == NULL) {
+		G_STRIPE_DEBUG(0, "Disk %s removed from %s.",
+		    cp->provider->name, sc->sc_name);
+		cp->private = (void *)(uintptr_t)-1;
+	}
 
-	sc->sc_disks[no] = NULL;
 	if (sc->sc_provider != NULL) {
 		sc->sc_provider->flags |= G_PF_WITHER;
+		G_STRIPE_DEBUG(0, "Device %s deactivated.",
+		    sc->sc_provider->name);
 		g_orphan_provider(sc->sc_provider, ENXIO);
 		sc->sc_provider = NULL;
-		G_STRIPE_DEBUG(0, "Device %s removed.", sc->sc_name);
 	}
 
 	if (cp->acr > 0 || cp->acw > 0 || cp->ace > 0)
-		g_access(cp, -cp->acr, -cp->acw, -cp->ace);
+		return;
+	sc->sc_disks[cp->index] = NULL;
+	cp->index = 0;
 	g_detach(cp);
 	g_destroy_consumer(cp);
+	/* If there are no valid disks anymore, remove device. */
+	if (LIST_EMPTY(&sc->sc_geom->consumer))
+		g_stripe_destroy(sc, 1);
 }
 
 static void
@@ -199,37 +206,21 @@
 		return;
 
 	g_stripe_remove_disk(cp);
-	/* If there are no valid disks anymore, remove device. */
-	if (g_stripe_nvalid(sc) == 0)
-		g_stripe_destroy(sc, 1);
 }
 
 static int
 g_stripe_access(struct g_provider *pp, int dr, int dw, int de)
 {
-	struct g_consumer *cp1, *cp2;
+	struct g_consumer *cp1, *cp2, *tmp;
 	struct g_stripe_softc *sc;
 	struct g_geom *gp;
 	int error;
 
+	g_topology_assert();
 	gp = pp->geom;
 	sc = gp->softc;
+	KASSERT(sc != NULL, ("NULL sc in %s.", __func__));
 
-	if (sc == NULL) {
-		/*
-		 * It looks like geom is being withered.
-		 * In that case we allow only negative requests.
-		 */
-		KASSERT(dr <= 0 && dw <= 0 && de <= 0,
-		    ("Positive access request (device=%s).", pp->name));
-		if ((pp->acr + dr) == 0 && (pp->acw + dw) == 0 &&
-		    (pp->ace + de) == 0) {
-			G_STRIPE_DEBUG(0, "Device %s definitely destroyed.",
-			    gp->name);
-		}
-		return (0);
-	}
-
 	/* On first open, grab an extra "exclusive" bit */
 	if (pp->acr == 0 && pp->acw == 0 && pp->ace == 0)
 		de++;
@@ -237,22 +228,23 @@
 	if ((pp->acr + dr) == 0 && (pp->acw + dw) == 0 && (pp->ace + de) == 0)
 		de--;
 
-	error = ENXIO;
-	LIST_FOREACH(cp1, &gp->consumer, consumer) {
+	LIST_FOREACH_SAFE(cp1, &gp->consumer, consumer, tmp) {
 		error = g_access(cp1, dr, dw, de);
-		if (error == 0)
-			continue;
-		/*
-		 * If we fail here, backout all previous changes.
-		 */
-		LIST_FOREACH(cp2, &gp->consumer, consumer) {
-			if (cp1 == cp2)
-				return (error);
-			g_access(cp2, -dr, -dw, -de);
+		if (error != 0)
+			goto fail;
+		if (cp1->acr == 0 && cp1->acw == 0 && cp1->ace == 0 &&
+		    cp1->private != NULL) {
+			g_stripe_remove_disk(cp1); /* May destroy geom. */
 		}
-		/* NOTREACHED */
 	}
+	return (0);
 
+fail:
+	LIST_FOREACH(cp2, &gp->consumer, consumer) {
+		if (cp1 == cp2)
+			break;
+		g_access(cp2, -dr, -dw, -de);
+	}
 	return (error);
 }
 
@@ -654,6 +646,7 @@
 	off_t mediasize, ms;
 	u_int no, sectorsize = 0;
 
+	g_topology_assert();
 	if (g_stripe_nvalid(sc) != sc->sc_ndisks)
 		return;
 
@@ -683,7 +676,7 @@
 	sc->sc_provider->stripeoffset = 0;
 	g_error_provider(sc->sc_provider, 0);
 
-	G_STRIPE_DEBUG(0, "Device %s activated.", sc->sc_name);
+	G_STRIPE_DEBUG(0, "Device %s activated.", sc->sc_provider->name);
 }
 
 static int
@@ -724,6 +717,7 @@
 	struct g_geom *gp;
 	int error;
 
+	g_topology_assert();
 	/* Metadata corrupted? */
 	if (no >= sc->sc_ndisks)
 		return (EINVAL);
@@ -736,6 +730,8 @@
 	fcp = LIST_FIRST(&gp->consumer);
 
 	cp = g_new_consumer(gp);
+	cp->private = NULL;
+	cp->index = no;
 	error = g_attach(cp, pp);
 	if (error != 0) {
 		g_destroy_consumer(cp);
@@ -766,12 +762,8 @@
 		}
 	}
 
-	cp->private = sc;
-	cp->index = no;
 	sc->sc_disks[no] = cp;
-
 	G_STRIPE_DEBUG(0, "Disk %s attached to %s.", pp->name, sc->sc_name);
-
 	g_stripe_check_and_run(sc);
 
 	return (0);
@@ -791,6 +783,7 @@
 	struct g_geom *gp;
 	u_int no;
 
+	g_topology_assert();
 	G_STRIPE_DEBUG(1, "Creating device %s (id=%u).", md->md_name,
 	    md->md_id);
 
@@ -852,8 +845,8 @@
 g_stripe_destroy(struct g_stripe_softc *sc, boolean_t force)
 {
 	struct g_provider *pp;
+	struct g_consumer *cp, *cp1;
 	struct g_geom *gp;
-	u_int no;
 
 	g_topology_assert();
 
@@ -873,24 +866,22 @@
 		}
 	}
 
-	for (no = 0; no < sc->sc_ndisks; no++) {
-		if (sc->sc_disks[no] != NULL)
-			g_stripe_remove_disk(sc->sc_disks[no]);
+	gp = sc->sc_geom;
+	LIST_FOREACH_SAFE(cp, &gp->consumer, consumer, cp1) {
+		g_stripe_remove_disk(cp);
+		if (cp1 == NULL)
+			return (0);	/* Recursion happened. */
 	}
+	if (!LIST_EMPTY(&gp->consumer))
+		return (EINPROGRESS);
 
-	gp = sc->sc_geom;
 	gp->softc = NULL;
 	KASSERT(sc->sc_provider == NULL, ("Provider still exists? (device=%s)",
 	    gp->name));
 	free(sc->sc_disks, M_STRIPE);
 	free(sc, M_STRIPE);
-
-	pp = LIST_FIRST(&gp->provider);
-	if (pp == NULL || (pp->acr == 0 && pp->acw == 0 && pp->ace == 0))
-		G_STRIPE_DEBUG(0, "Device %s destroyed.", gp->name);
-
+	G_STRIPE_DEBUG(0, "Device %s destroyed.", gp->name);
 	g_wither_geom(gp, ENXIO);
-
 	return (0);
 }
 



More information about the Midnightbsd-cvs mailing list