Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 23 Jan 2019 01:23:20 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r343333 - stable/12/sys/geom
Message-ID:  <201901230123.x0N1NKnQ097902@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: mav
Date: Wed Jan 23 01:23:19 2019
New Revision: 343333
URL: https://svnweb.freebsd.org/changeset/base/343333

Log:
  MFC r342558: Switch from mutexes to atomics in GEOM_DEV I/O path.
  
  Mutexes in I/O path there were used twice per I/O to atomically access
  several variables to close and/or destroy the device on last request
  completion.  I found the way to fit all required info into one integer,
  suitable for atomic operations.  It opened race window on device close,
  but addition of timeout to the msleep() there should cover it.
  
  Profiling shows removal of significant spinning time on those mutexes
  and IOPS increase from ~600K to >800K to NVMe on 72-core systems.

Modified:
  stable/12/sys/geom/geom_dev.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/geom/geom_dev.c
==============================================================================
--- stable/12/sys/geom/geom_dev.c	Wed Jan 23 01:16:59 2019	(r343332)
+++ stable/12/sys/geom/geom_dev.c	Wed Jan 23 01:23:19 2019	(r343333)
@@ -64,7 +64,10 @@ struct g_dev_softc {
 	struct cdev	*sc_dev;
 	struct cdev	*sc_alias;
 	int		 sc_open;
-	int		 sc_active;
+	u_int		 sc_active;
+#define	SC_A_DESTROY	(1 << 31)
+#define	SC_A_OPEN	(1 << 30)
+#define	SC_A_ACTIVE	(SC_A_OPEN - 1)
 };
 
 static d_open_t		g_dev_open;
@@ -420,9 +423,13 @@ g_dev_open(struct cdev *dev, int flags, int fmt, struc
 	if (error == 0) {
 		sc = cp->private;
 		mtx_lock(&sc->sc_mtx);
-		if (sc->sc_open == 0 && sc->sc_active != 0)
+		if (sc->sc_open == 0 && (sc->sc_active & SC_A_ACTIVE) != 0)
 			wakeup(&sc->sc_active);
 		sc->sc_open += r + w + e;
+		if (sc->sc_open == 0)
+			atomic_clear_int(&sc->sc_active, SC_A_OPEN);
+		else
+			atomic_set_int(&sc->sc_active, SC_A_OPEN);
 		mtx_unlock(&sc->sc_mtx);
 	}
 	return (error);
@@ -465,8 +472,12 @@ g_dev_close(struct cdev *dev, int flags, int fmt, stru
 	sc = cp->private;
 	mtx_lock(&sc->sc_mtx);
 	sc->sc_open += r + w + e;
-	while (sc->sc_open == 0 && sc->sc_active != 0)
-		msleep(&sc->sc_active, &sc->sc_mtx, 0, "PRIBIO", 0);
+	if (sc->sc_open == 0)
+		atomic_clear_int(&sc->sc_active, SC_A_OPEN);
+	else
+		atomic_set_int(&sc->sc_active, SC_A_OPEN);
+	while (sc->sc_open == 0 && (sc->sc_active & SC_A_ACTIVE) != 0)
+		msleep(&sc->sc_active, &sc->sc_mtx, 0, "g_dev_close", hz / 10);
 	mtx_unlock(&sc->sc_mtx);
 	g_topology_lock();
 	error = g_access(cp, r, w, e);
@@ -693,7 +704,7 @@ g_dev_done(struct bio *bp2)
 	struct g_consumer *cp;
 	struct g_dev_softc *sc;
 	struct bio *bp;
-	int destroy;
+	int active;
 
 	cp = bp2->bio_from;
 	sc = cp->private;
@@ -713,17 +724,13 @@ g_dev_done(struct bio *bp2)
 		    bp2, bp, bp2->bio_resid, (intmax_t)bp2->bio_completed);
 	}
 	g_destroy_bio(bp2);
-	destroy = 0;
-	mtx_lock(&sc->sc_mtx);
-	if ((--sc->sc_active) == 0) {
-		if (sc->sc_open == 0)
+	active = atomic_fetchadd_int(&sc->sc_active, -1) - 1;
+	if ((active & SC_A_ACTIVE) == 0) {
+		if ((active & SC_A_OPEN) == 0)
 			wakeup(&sc->sc_active);
-		if (sc->sc_dev == NULL)
-			destroy = 1;
+		if (active & SC_A_DESTROY)
+			g_post_event(g_dev_destroy, cp, M_NOWAIT, NULL);
 	}
-	mtx_unlock(&sc->sc_mtx);
-	if (destroy)
-		g_post_event(g_dev_destroy, cp, M_NOWAIT, NULL);
 	biodone(bp);
 }
 
@@ -755,10 +762,8 @@ g_dev_strategy(struct bio *bp)
 		return;
 	}
 #endif
-	mtx_lock(&sc->sc_mtx);
 	KASSERT(sc->sc_open > 0, ("Closed device in g_dev_strategy"));
-	sc->sc_active++;
-	mtx_unlock(&sc->sc_mtx);
+	atomic_add_int(&sc->sc_active, 1);
 
 	for (;;) {
 		/*
@@ -796,18 +801,16 @@ g_dev_callback(void *arg)
 {
 	struct g_consumer *cp;
 	struct g_dev_softc *sc;
-	int destroy;
+	int active;
 
 	cp = arg;
 	sc = cp->private;
 	g_trace(G_T_TOPOLOGY, "g_dev_callback(%p(%s))", cp, cp->geom->name);
 
-	mtx_lock(&sc->sc_mtx);
 	sc->sc_dev = NULL;
 	sc->sc_alias = NULL;
-	destroy = (sc->sc_active == 0);
-	mtx_unlock(&sc->sc_mtx);
-	if (destroy)
+	active = atomic_fetchadd_int(&sc->sc_active, SC_A_DESTROY);
+	if ((active & SC_A_ACTIVE) == 0)
 		g_post_event(g_dev_destroy, cp, M_WAITOK, NULL);
 }

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