Author: mav
Date: Fri Apr 5 10:27:05 2013
New Revision: 249145
URL: http://svnweb.freebsd.org/changeset/base/249145
Log:
MFC r226998, r227004:
Refactor disk disconnection and geom destruction handling sequences.
Do not close/destroy opened consumer directly in case of disconnect. Instead
keep it existing until it will be closed in regular way in response to
upstream provider destruction. Delay geom destruction in the same way.
Previous implementation could destroy consumers still having active
requests and worked only because of global workaround made on GEOM level.
Modified:
stable/9/sys/geom/concat/g_concat.c
stable/9/sys/geom/concat/g_concat.h
stable/9/sys/geom/stripe/g_stripe.c
Directory Properties:
stable/9/sys/ (props changed)
Modified: stable/9/sys/geom/concat/g_concat.c
==============================================================================
--- stable/9/sys/geom/concat/g_concat.c Fri Apr 5 10:22:22 2013
(r249144)
+++ stable/9/sys/geom/concat/g_concat.c Fri Apr 5 10:27:05 2013
(r249145)
@@ -118,25 +118,33 @@ g_concat_remove_disk(struct g_concat_dis
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
@@ -156,37 +164,18 @@ g_concat_orphan(struct g_consumer *cp)
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)
@@ -195,22 +184,24 @@ g_concat_access(struct g_provider *pp, i
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);
}
@@ -392,6 +383,7 @@ g_concat_check_and_run(struct g_concat_s
u_int no, sectorsize = 0;
off_t start;
+ g_topology_assert();
if (g_concat_nvalid(sc) != sc->sc_ndisks)
return;
@@ -420,7 +412,7 @@ g_concat_check_and_run(struct g_concat_s
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
@@ -462,6 +454,7 @@ g_concat_add_disk(struct g_concat_softc
struct g_geom *gp;
int error;
+ g_topology_assert();
/* Metadata corrupted? */
if (no >= sc->sc_ndisks)
return (EINVAL);
@@ -510,6 +503,7 @@ g_concat_add_disk(struct g_concat_softc
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);
@@ -577,8 +571,8 @@ static int
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();
@@ -598,24 +592,23 @@ g_concat_destroy(struct g_concat_softc *
}
}
- 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));
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: stable/9/sys/geom/concat/g_concat.h
==============================================================================
--- stable/9/sys/geom/concat/g_concat.h Fri Apr 5 10:22:22 2013
(r249144)
+++ stable/9/sys/geom/concat/g_concat.h Fri Apr 5 10:27:05 2013
(r249145)
@@ -72,6 +72,7 @@ struct g_concat_disk {
struct g_concat_softc *d_softc;
off_t d_start;
off_t d_end;
+ int d_removed;
};
struct g_concat_softc {
Modified: stable/9/sys/geom/stripe/g_stripe.c
==============================================================================
--- stable/9/sys/geom/stripe/g_stripe.c Fri Apr 5 10:22:22 2013
(r249144)
+++ stable/9/sys/geom/stripe/g_stripe.c Fri Apr 5 10:27:05 2013
(r249145)
@@ -161,28 +161,35 @@ static void
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
@@ -198,36 +205,20 @@ g_stripe_orphan(struct g_consumer *cp)
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;
-
- 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);
- }
+ KASSERT(sc != NULL, ("NULL sc in %s.", __func__));
/* On first open, grab an extra "exclusive" bit */
if (pp->acr == 0 && pp->acw == 0 && pp->ace == 0)
@@ -236,22 +227,23 @@ g_stripe_access(struct g_provider *pp, i
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);
}
@@ -653,6 +645,7 @@ g_stripe_check_and_run(struct g_stripe_s
off_t mediasize, ms;
u_int no, sectorsize = 0;
+ g_topology_assert();
if (g_stripe_nvalid(sc) != sc->sc_ndisks)
return;
@@ -682,7 +675,7 @@ g_stripe_check_and_run(struct g_stripe_s
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
@@ -723,6 +716,7 @@ g_stripe_add_disk(struct g_stripe_softc
struct g_geom *gp;
int error;
+ g_topology_assert();
/* Metadata corrupted? */
if (no >= sc->sc_ndisks)
return (EINVAL);
@@ -735,6 +729,8 @@ g_stripe_add_disk(struct g_stripe_softc
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);
@@ -765,12 +761,8 @@ g_stripe_add_disk(struct g_stripe_softc
}
}
- 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);
@@ -790,6 +782,7 @@ g_stripe_create(struct g_class *mp, cons
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);
@@ -851,8 +844,8 @@ static int
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();
@@ -872,24 +865,22 @@ g_stripe_destroy(struct g_stripe_softc *
}
}
- 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);
}
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
