Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 25 Mar 2013 05:45:24 +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: r248694 - head/sys/geom
Message-ID:  <201303250545.r2P5jOAL085192@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mav
Date: Mon Mar 25 05:45:24 2013
New Revision: 248694
URL: http://svnweb.freebsd.org/changeset/base/248694

Log:
  In GEOM DISK:
   - Replace single done mutex with per-disk ones.  On system with several
  disks on several HBAs that removes small, but measurable lock congestion.
   - Modify disk destruction process to not destroy the mutex prematurely.
   - Remove some extra pointer derefences.

Modified:
  head/sys/geom/geom_disk.c

Modified: head/sys/geom/geom_disk.c
==============================================================================
--- head/sys/geom/geom_disk.c	Mon Mar 25 00:31:14 2013	(r248693)
+++ head/sys/geom/geom_disk.c	Mon Mar 25 05:45:24 2013	(r248694)
@@ -60,6 +60,7 @@ __FBSDID("$FreeBSD$");
 #include <dev/led/led.h>
 
 struct g_disk_softc {
+	struct mtx		 done_mtx;
 	struct disk		*dp;
 	struct sysctl_ctx_list	sysctl_ctx;
 	struct sysctl_oid	*sysctl_tree;
@@ -67,11 +68,7 @@ struct g_disk_softc {
 	uint32_t		state;
 };
 
-static struct mtx g_disk_done_mtx;
-
 static g_access_t g_disk_access;
-static g_init_t g_disk_init;
-static g_fini_t g_disk_fini;
 static g_start_t g_disk_start;
 static g_ioctl_t g_disk_ioctl;
 static g_dumpconf_t g_disk_dumpconf;
@@ -80,8 +77,6 @@ static g_provgone_t g_disk_providergone;
 static struct g_class g_disk_class = {
 	.name = "DISK",
 	.version = G_VERSION,
-	.init = g_disk_init,
-	.fini = g_disk_fini,
 	.start = g_disk_start,
 	.access = g_disk_access,
 	.ioctl = g_disk_ioctl,
@@ -93,20 +88,6 @@ SYSCTL_DECL(_kern_geom);
 static SYSCTL_NODE(_kern_geom, OID_AUTO, disk, CTLFLAG_RW, 0,
     "GEOM_DISK stuff");
 
-static void
-g_disk_init(struct g_class *mp __unused)
-{
-
-	mtx_init(&g_disk_done_mtx, "g_disk_done", NULL, MTX_DEF);
-}
-
-static void
-g_disk_fini(struct g_class *mp __unused)
-{
-
-	mtx_destroy(&g_disk_done_mtx);
-}
-
 DECLARE_GEOM_CLASS(g_disk_class, g_disk);
 
 static void __inline
@@ -135,7 +116,7 @@ g_disk_access(struct g_provider *pp, int
 	g_trace(G_T_ACCESS, "g_disk_access(%s, %d, %d, %d)",
 	    pp->name, r, w, e);
 	g_topology_assert();
-	sc = pp->geom->softc;
+	sc = pp->private;
 	if (sc == NULL || (dp = sc->dp) == NULL || dp->d_destroyed) {
 		/*
 		 * Allow decreasing access count even if disk is not
@@ -240,42 +221,36 @@ static void
 g_disk_done(struct bio *bp)
 {
 	struct bio *bp2;
-	struct disk *dp;
 	struct g_disk_softc *sc;
 
 	/* See "notes" for why we need a mutex here */
 	/* XXX: will witness accept a mix of Giant/unGiant drivers here ? */
-	mtx_lock(&g_disk_done_mtx);
-	bp->bio_completed = bp->bio_length - bp->bio_resid;
-
 	bp2 = bp->bio_parent;
+	sc = bp2->bio_to->private;
+	bp->bio_completed = bp->bio_length - bp->bio_resid;
+	mtx_lock(&sc->done_mtx);
 	if (bp2->bio_error == 0)
 		bp2->bio_error = bp->bio_error;
 	bp2->bio_completed += bp->bio_completed;
-	if ((bp->bio_cmd & (BIO_READ|BIO_WRITE|BIO_DELETE)) != 0 &&
-	    (sc = bp2->bio_to->geom->softc) != NULL &&
-	    (dp = sc->dp) != NULL) {
-		devstat_end_transaction_bio(dp->d_devstat, bp);
-	}
+	if ((bp->bio_cmd & (BIO_READ|BIO_WRITE|BIO_DELETE)) != 0)
+		devstat_end_transaction_bio(sc->dp->d_devstat, bp);
 	g_destroy_bio(bp);
 	bp2->bio_inbed++;
 	if (bp2->bio_children == bp2->bio_inbed) {
 		bp2->bio_resid = bp2->bio_bcount - bp2->bio_completed;
 		g_io_deliver(bp2, bp2->bio_error);
 	}
-	mtx_unlock(&g_disk_done_mtx);
+	mtx_unlock(&sc->done_mtx);
 }
 
 static int
 g_disk_ioctl(struct g_provider *pp, u_long cmd, void * data, int fflag, struct thread *td)
 {
-	struct g_geom *gp;
 	struct disk *dp;
 	struct g_disk_softc *sc;
 	int error;
 
-	gp = pp->geom;
-	sc = gp->softc;
+	sc = pp->private;
 	dp = sc->dp;
 
 	if (dp->d_ioctl == NULL)
@@ -295,7 +270,7 @@ g_disk_start(struct bio *bp)
 	int error;
 	off_t off;
 
-	sc = bp->bio_to->geom->softc;
+	sc = bp->bio_to->private;
 	if (sc == NULL || (dp = sc->dp) == NULL || dp->d_destroyed) {
 		g_io_deliver(bp, ENXIO);
 		return;
@@ -496,6 +471,7 @@ g_disk_create(void *arg, int flag)
 	g_topology_assert();
 	dp = arg;
 	sc = g_malloc(sizeof(*sc), M_WAITOK | M_ZERO);
+	mtx_init(&sc->done_mtx, "g_disk_done", NULL, MTX_DEF);
 	sc->dp = dp;
 	gp = g_new_geomf(&g_disk_class, "%s%d", dp->d_name, dp->d_unit);
 	gp->softc = sc;
@@ -539,19 +515,22 @@ g_disk_providergone(struct g_provider *p
 	struct disk *dp;
 	struct g_disk_softc *sc;
 
-	sc = (struct g_disk_softc *)pp->geom->softc;
-
-	/*
-	 * If the softc is already NULL, then we've probably been through
-	 * g_disk_destroy already; there is nothing for us to do anyway.
-	 */
-	if (sc == NULL)
-		return;
-
+	sc = (struct g_disk_softc *)pp->private;
 	dp = sc->dp;
-
-	if (dp->d_gone != NULL)
+	if (dp != NULL && dp->d_gone != NULL)
 		dp->d_gone(dp);
+	if (sc->sysctl_tree != NULL) {
+		sysctl_ctx_free(&sc->sysctl_ctx);
+		sc->sysctl_tree = NULL;
+	}
+	if (sc->led[0] != 0) {
+		led_set(sc->led, "0");
+		sc->led[0] = 0;
+	}
+	pp->private = NULL;
+	pp->geom->softc = NULL;
+	mtx_destroy(&sc->done_mtx);
+	g_free(sc);
 }
 
 static void
@@ -566,16 +545,9 @@ g_disk_destroy(void *ptr, int flag)
 	gp = dp->d_geom;
 	if (gp != NULL) {
 		sc = gp->softc;
-		if (sc->sysctl_tree != NULL) {
-			sysctl_ctx_free(&sc->sysctl_ctx);
-			sc->sysctl_tree = NULL;
-		}
-		if (sc->led[0] != 0) {
-			led_set(sc->led, "0");
-			sc->led[0] = 0;
-		}
-		g_free(sc);
-		gp->softc = NULL;
+		if (sc != NULL)
+			sc->dp = NULL;
+		dp->d_geom = NULL;
 		g_wither_geom(gp, ENXIO);
 	}
 	g_free(dp);

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