PERFORCE change 114990 for review

Matt Jacob mjacob at FreeBSD.org
Sat Feb 24 20:03:24 UTC 2007


http://perforce.freebsd.org/chv.cgi?CH=114990

Change 114990 by mjacob at mjexp on 2007/02/24 20:02:27

	Implemnentation of about 2/3 of the changes recommened by
	Pavel's review.

Affected files ...

.. //depot/projects/mjexp/sys/geom/multipath/g_multipath.c#13 edit

Differences ...

==== //depot/projects/mjexp/sys/geom/multipath/g_multipath.c#13 (text+ko) ====

@@ -39,6 +39,7 @@
 #include <sys/mutex.h>
 #include <sys/bio.h>
 #include <sys/sysctl.h>
+#include <sys/kthread.h>
 #include <sys/malloc.h>
 #include <geom/geom.h>
 #include <geom/multipath/g_multipath.h>
@@ -51,9 +52,19 @@
 SYSCTL_UINT(_kern_geom_multipath, OID_AUTO, debug, CTLFLAG_RW,
     &g_multipath_debug, 0, "Debug level");
 
+static enum {
+	GKT_NIL,
+	GKT_RUN,
+	GKT_DIE
+} g_multipath_kt_state;
+static struct bio_queue_head gmtbq;
+static struct mtx gmtbq_mtx;
+
 static void g_multipath_orphan(struct g_consumer *);
 static void g_multipath_start(struct bio *);
 static void g_multipath_done(struct bio *);
+static void g_multipath_done_error(struct bio *);
+static void g_multipath_kt(void *);
 
 static int g_multipath_destroy(struct g_geom *);
 static int
@@ -61,13 +72,17 @@
 
 static g_taste_t g_multipath_taste;
 static g_ctl_req_t g_multipath_config;
+static g_init_t g_multipath_init;
+static g_fini_t g_multipath_fini;
 
 struct g_class g_multipath_class = {
-	.name = G_MULTIPATH_CLASS_NAME,
-	.version = G_VERSION,
-	.ctlreq = g_multipath_config,
-	.taste = g_multipath_taste,
-	.destroy_geom = g_multipath_destroy_geom
+	.name		= G_MULTIPATH_CLASS_NAME,
+	.version	= G_VERSION,
+	.ctlreq		= g_multipath_config,
+	.taste		= g_multipath_taste,
+	.destroy_geom	= g_multipath_destroy_geom,
+	.init		= g_multipath_init,
+	.fini		= g_multipath_fini
 };
 
 #define	MP_BAD		0x1
@@ -130,73 +145,98 @@
 static void
 g_multipath_done(struct bio *bp)
 {
+	if (bp->bio_error == ENXIO || bp->bio_error == EIO) {
+		mtx_lock(&gmtbq_mtx);
+		bioq_insert_tail(&gmtbq, bp);
+		wakeup(&g_multipath_kt_state);
+		mtx_unlock(&gmtbq_mtx);
+	} else {
+		g_std_done(bp);
+	}
+}
+
+static void
+g_multipath_done_error(struct bio *bp)
+{
 	struct bio *pbp = bp->bio_parent;
 	struct g_geom *gp = pbp->bio_to->geom;
 	struct g_multipath_softc *sc = gp->softc;
-	int dofail;
+	struct g_consumer *cp = bp->bio_from;
+	struct g_provider *pp = cp->provider;
 
-	KASSERT(sc != NULL, ("NULL sc"));
+	/*
+	 * If we had a failure, we have to check first to see
+	 * whether the consumer it failed on was the currently
+	 * active consumer (i.e., this is the first in perhaps
+	 * a number of failures). If so, we then switch consumers
+	 * to the next available consumer.
+	 */
 
-	if (bp->bio_error == ENXIO || bp->bio_error == EIO) {
-		dofail = 1;
-	} else {
-		dofail = 0;
+	g_topology_lock();
+	cp->index |= MP_BAD;
+	if (cp->nend == cp->nstart && pp->nend == pp->nstart) {
+		cp->index |= MP_POSTED;
+		g_post_event(g_mpd, cp, M_NOWAIT, NULL);
 	}
-		
-	if (dofail) {
-		struct g_consumer *cp = bp->bio_from;
+	if (cp == sc->cp_active) {
 		struct g_consumer *lcp;
-		struct g_provider *pp = cp->provider;
-
-		/*
-		 * If we had a failure, we have to check first to see
-		 * whether the consumer it failed on was the currently
-		 * active consumer (i.e., this is the first in perhaps
-		 * a number of failures). If so, we then switch consumers
-		 * to the next available consumer.
-		 */
-		g_topology_lock();
-		cp->index |= MP_BAD;
-		if (cp->nend == cp->nstart && pp->nend == pp->nstart) {
-			cp->index |= MP_POSTED;
-			g_post_event(g_mpd, cp, M_NOWAIT, NULL);
+		printf("GEOM_MULTIPATH: %s failed in %s\n",
+		    pp->name, sc->sc_name);
+		sc->cp_active = NULL;
+		LIST_FOREACH(lcp, &gp->consumer, consumer) {
+			if ((lcp->index & MP_BAD) == 0) {
+				sc->cp_active = lcp;
+				break;
+			}
 		}
-		if (cp == sc->cp_active) {
-			printf("GEOM_MULTIPATH: %s failed in %s\n",
-			    pp->name, sc->sc_name);
-			sc->cp_active = NULL;
-			LIST_FOREACH(lcp, &gp->consumer, consumer) {
-				if ((lcp->index & MP_BAD) == 0) {
-					sc->cp_active = lcp;
-					break;
-				}
-			}
-			if (sc->cp_active == NULL) {
-				printf("GEOM_MULTIPATH: out of providers\n");
-				g_topology_unlock();
-				goto out;
-			}
+		if (sc->cp_active == NULL) {
+			printf("GEOM_MULTIPATH: out of providers for %s\n",
+			    sc->sc_name);
+			return;
+		} else {
 			printf("GEOM_MULTIPATH: %s now active path in %s\n",
 			    sc->cp_active->provider->name, sc->sc_name);
 		}
-		g_topology_unlock();
+	}
+	g_topology_unlock();
+
+	/*
+	 * If we can fruitfully restart the I/O, do so.
+	 */
+	if (sc->cp_active) {
+		g_destroy_bio(bp);
+		pbp->bio_children--;
+		g_multipath_start(pbp);
+	} else {
+		g_std_done(bp);
+	}
+}
 
-		/*
-		 * If we can fruitfully restart the I/O, do so.
-		 */
-		if (sc->cp_active) {
-			g_destroy_bio(bp);
-			pbp->bio_children--;
-			g_multipath_start(pbp);
-			return;
+static void
+g_multipath_kt(void *arg)
+{
+	g_multipath_kt_state = GKT_RUN;
+	mtx_lock(&gmtbq_mtx);
+	while (g_multipath_kt_state == GKT_RUN) {
+		for (;;) {
+			struct bio *bp;
+			bp = bioq_takefirst(&gmtbq);
+			if (bp == NULL) {
+				break;
+			}
+			mtx_unlock(&gmtbq_mtx);
+			g_multipath_done_error(bp);
+			mtx_lock(&gmtbq_mtx);
 		}
+		msleep(&g_multipath_kt_state, &gmtbq_mtx,PRIBIO,
+		    "gkt:wait", hz / 10);
 	}
-out:
-	g_std_done(bp);
+	mtx_unlock(&gmtbq_mtx);
+	wakeup(&g_multipath_kt_state);
+	kthread_exit(0);
 }
 
 
-
 static int
 g_multipath_access(struct g_provider *pp, int dr, int dw, int de)
 {
@@ -231,7 +271,6 @@
 	struct g_multipath_softc *sc;
 	struct g_geom *gp;
 	struct g_provider *pp;
-	char name[64];
 
 	g_topology_assert();
 
@@ -260,16 +299,15 @@
 	memcpy(sc->sc_uuid, md->md_uuid, sizeof (sc->sc_uuid));
 	memcpy(sc->sc_name, md->md_name, sizeof (sc->sc_name));
 
-	snprintf(name, sizeof(name), "multipath/%s", md->md_name);
-	pp = g_new_providerf(gp, name);
+	pp = g_new_providerf(gp, "multipath/%s", md->md_name);
 	if (pp == NULL) {
 		goto fail;
 	}
 	/* limit the provider to not have it stomp on metadata */
 	pp->mediasize = md->md_size - md->md_sectorsize;
 	pp->sectorsize = md->md_sectorsize;
+	sc->pp = pp;
 	g_error_provider(pp, 0);
-	sc->pp = pp;
 	return (gp);
 fail:
 	if (gp != NULL) {
@@ -288,6 +326,8 @@
 	struct g_consumer *cp, *nxtcp;
 	int error;
 
+	g_topology_assert();
+
 	sc = gp->softc;
 	KASSERT(sc, ("no softc"));
 
@@ -311,7 +351,8 @@
 	}
 	error = g_attach(cp, pp);
 	if (error != 0) {
-		printf("cannot attach %s to %s", pp->name, sc->sc_name);
+		printf("GEOM_MULTIPATH: cannot attach %s to %s",
+		    pp->name, sc->sc_name);
 		g_destroy_consumer(cp);
 		return (error);
 	}
@@ -369,6 +410,29 @@
 	return (g_multipath_destroy(gp));
 }
 
+static void
+g_multipath_init(struct g_class *mp)
+{
+	bioq_init(&gmtbq);
+	mtx_init(&gmtbq_mtx, "gmtbq", NULL, MTX_DEF);
+	if (kthread_create(g_multipath_kt, mp, NULL, 0, 0, "g_mp_kt") == 0) {
+		g_multipath_kt_state = GKT_RUN;
+	}
+}
+
+static void
+g_multipath_fini(struct g_class *mp)
+{
+	if (g_multipath_kt_state == GKT_RUN) {
+		mtx_lock(&gmtbq_mtx);
+		g_multipath_kt_state = GKT_DIE;
+		wakeup(&g_multipath_kt_state);
+		msleep(&g_multipath_kt_state, &gmtbq_mtx, PRIBIO,
+		    "gmp:fini", 0);
+		mtx_unlock(&gmtbq_mtx);
+	}
+}
+
 static int
 g_multipath_read_metadata(struct g_consumer *cp,
     struct g_multipath_metadata *md)
@@ -602,9 +666,9 @@
 		return;
 	}
     	if (pp0->mediasize != pp1->mediasize) {
-		gctl_error(req, "Provider %s is %zu; Provider %s is %zu",
-		    pp0->name, (size_t) pp0->mediasize,
-		    pp1->name, (size_t) pp1->mediasize);
+		gctl_error(req, "Provider %s is %jd; Provider %s is %jd",
+		    pp0->name, (intmax_t) pp0->mediasize,
+		    pp1->name, (intmax_t) pp1->mediasize);
 		return;
 	}
     	if (pp0->sectorsize != pp1->sectorsize) {


More information about the p4-projects mailing list