Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 28 Oct 2016 18:25:32 +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-10@freebsd.org
Subject:   svn commit: r308061 - in stable/10/sys: cddl/contrib/opensolaris/uts/common/fs/zfs geom
Message-ID:  <201610281825.u9SIPWDA068597@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Fri Oct 28 18:25:32 2016
New Revision: 308061
URL: https://svnweb.freebsd.org/changeset/base/308061

Log:
  MFC r300881, r302058 (by asomers):
  Avoid issuing spa config updates for physical path when not necessary
  
  ZFS's configuration needs to be updated whenever the physical path for a
  device changes, but not when a new device is introduced. This is because new
  devices necessarily cause config updates, but only if they are actually
  accepted into the pool.
  
  sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
          Split vdev_geom_set_physpath out of vdev_geom_attrchanged.  When
          setting the vdev's physical path, only request a config update if
          the physical path has changed.  Don't request it when opening a
          device for the first time, because the config sync will happen
          anyway upstack.
  
  sys/geom/geom_dev.c
          Split g_dev_set_physpath and g_dev_set_media out of
          g_dev_attrchanged

Modified:
  stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
  stable/10/sys/geom/geom_dev.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
==============================================================================
--- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c	Fri Oct 28 18:24:05 2016	(r308060)
+++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c	Fri Oct 28 18:25:32 2016	(r308061)
@@ -87,32 +87,17 @@ vdev_geom_set_rotation_rate(vdev_t *vd, 
 }
 
 static void
-vdev_geom_attrchanged(struct g_consumer *cp, const char *attr)
+vdev_geom_set_physpath(struct g_consumer *cp, boolean_t do_null_update)
 {
+	boolean_t needs_update = B_FALSE;
 	vdev_t *vd;
-	spa_t *spa;
 	char *physpath;
 	int error, physpath_len;
 
-	vd = cp->private;
-	if (vd == NULL)
-		return;
-
-	if (strcmp(attr, "GEOM::rotation_rate") == 0) {
-		vdev_geom_set_rotation_rate(vd, cp);
-		return;
-	}
-
-	if (strcmp(attr, "GEOM::physpath") != 0)
-		return;
-
 	if (g_access(cp, 1, 0, 0) != 0)
 		return;
 
-	/*
-	 * Record/Update physical path information for this device.
-	 */
-	spa = vd->vdev_spa;
+	vd = cp->private;
 	physpath_len = MAXPATHLEN;
 	physpath = g_malloc(physpath_len, M_WAITOK|M_ZERO);
 	error = g_io_getattr("GEOM::physpath", cp, &physpath_len, physpath);
@@ -124,12 +109,46 @@ vdev_geom_attrchanged(struct g_consumer 
 		g_topology_assert();
 		old_physpath = vd->vdev_physpath;
 		vd->vdev_physpath = spa_strdup(physpath);
-		spa_async_request(spa, SPA_ASYNC_CONFIG_UPDATE);
 
-		if (old_physpath != NULL)
+		if (old_physpath != NULL) {
+			needs_update = (strcmp(old_physpath,
+						vd->vdev_physpath) != 0);
 			spa_strfree(old_physpath);
+		} else
+			needs_update = do_null_update;
 	}
 	g_free(physpath);
+
+	/*
+	 * If the physical path changed, update the config.
+	 * Only request an update for previously unset physpaths if
+	 * requested by the caller.
+	 */
+	if (needs_update)
+		spa_async_request(vd->vdev_spa, SPA_ASYNC_CONFIG_UPDATE);
+
+}
+
+static void
+vdev_geom_attrchanged(struct g_consumer *cp, const char *attr)
+{
+	vdev_t *vd;
+	char *old_physpath;
+	int error;
+
+	vd = cp->private;
+	if (vd == NULL)
+		return;
+
+	if (strcmp(attr, "GEOM::rotation_rate") == 0) {
+		vdev_geom_set_rotation_rate(vd, cp);
+		return;
+	}
+
+	if (strcmp(attr, "GEOM::physpath") == 0) {
+		vdev_geom_set_physpath(cp, /*do_null_update*/B_TRUE);
+		return;
+	}
 }
 
 static void
@@ -259,8 +278,10 @@ vdev_geom_attach(struct g_provider *pp, 
 	 * 2) Set it to a linked list of vdevs, not just a single vdev
 	 */
 	cp->private = vd;
-	if (vd != NULL)
+	if (vd != NULL) {
 		vd->vdev_tsd = cp;
+		vdev_geom_set_physpath(cp, /*do_null_update*/B_FALSE);
+	}
 
 	cp->flags |= G_CF_DIRECT_SEND | G_CF_DIRECT_RECEIVE;
 	return (cp);

Modified: stable/10/sys/geom/geom_dev.c
==============================================================================
--- stable/10/sys/geom/geom_dev.c	Fri Oct 28 18:24:05 2016	(r308060)
+++ stable/10/sys/geom/geom_dev.c	Fri Oct 28 18:25:32 2016	(r308061)
@@ -222,55 +222,68 @@ g_dev_print(void)
 }
 
 static void
-g_dev_attrchanged(struct g_consumer *cp, const char *attr)
+g_dev_set_physpath(struct g_consumer *cp)
+{
+	struct g_dev_softc *sc;
+	char *physpath;
+	int error, physpath_len;
+
+	if (g_access(cp, 1, 0, 0) != 0)
+		return;
+
+	sc = cp->private;
+	physpath_len = MAXPATHLEN;
+	physpath = g_malloc(physpath_len, M_WAITOK|M_ZERO);
+	error = g_io_getattr("GEOM::physpath", cp, &physpath_len, physpath);
+	g_access(cp, -1, 0, 0);
+	if (error == 0 && strlen(physpath) != 0) {
+		struct cdev *dev, *old_alias_dev;
+		struct cdev **alias_devp;
+
+		dev = sc->sc_dev;
+		old_alias_dev = sc->sc_alias;
+		alias_devp = (struct cdev **)&sc->sc_alias;
+		make_dev_physpath_alias(MAKEDEV_WAITOK, alias_devp, dev,
+		    old_alias_dev, physpath);
+	} else if (sc->sc_alias) {
+		destroy_dev((struct cdev *)sc->sc_alias);
+		sc->sc_alias = NULL;
+	}
+	g_free(physpath);
+}
+
+static void
+g_dev_set_media(struct g_consumer *cp)
 {
 	struct g_dev_softc *sc;
 	struct cdev *dev;
 	char buf[SPECNAMELEN + 6];
 
 	sc = cp->private;
-	if (strcmp(attr, "GEOM::media") == 0) {
-		dev = sc->sc_dev;
+	dev = sc->sc_dev;
+	snprintf(buf, sizeof(buf), "cdev=%s", dev->si_name);
+	devctl_notify_f("DEVFS", "CDEV", "MEDIACHANGE", buf, M_WAITOK);
+	devctl_notify_f("GEOM", "DEV", "MEDIACHANGE", buf, M_WAITOK);
+	dev = sc->sc_alias;
+	if (dev != NULL) {
 		snprintf(buf, sizeof(buf), "cdev=%s", dev->si_name);
 		devctl_notify_f("DEVFS", "CDEV", "MEDIACHANGE", buf, M_WAITOK);
 		devctl_notify_f("GEOM", "DEV", "MEDIACHANGE", buf, M_WAITOK);
-		dev = sc->sc_alias;
-		if (dev != NULL) {
-			snprintf(buf, sizeof(buf), "cdev=%s", dev->si_name);
-			devctl_notify_f("DEVFS", "CDEV", "MEDIACHANGE", buf,
-			    M_WAITOK);
-			devctl_notify_f("GEOM", "DEV", "MEDIACHANGE", buf,
-			    M_WAITOK);
-		}
-		return;
 	}
+}
 
-	if (strcmp(attr, "GEOM::physpath") != 0)
+static void
+g_dev_attrchanged(struct g_consumer *cp, const char *attr)
+{
+
+	if (strcmp(attr, "GEOM::media") == 0) {
+		g_dev_set_media(cp);
 		return;
+	}
 
-	if (g_access(cp, 1, 0, 0) == 0) {
-		char *physpath;
-		int error, physpath_len;
-
-		physpath_len = MAXPATHLEN;
-		physpath = g_malloc(physpath_len, M_WAITOK|M_ZERO);
-		error =
-		    g_io_getattr("GEOM::physpath", cp, &physpath_len, physpath);
-		g_access(cp, -1, 0, 0);
-		if (error == 0 && strlen(physpath) != 0) {
-			struct cdev *old_alias_dev;
-			struct cdev **alias_devp;
-
-			dev = sc->sc_dev;
-			old_alias_dev = sc->sc_alias;
-			alias_devp = (struct cdev **)&sc->sc_alias;
-			make_dev_physpath_alias(MAKEDEV_WAITOK, alias_devp,
-			    dev, old_alias_dev, physpath);
-		} else if (sc->sc_alias) {
-			destroy_dev((struct cdev *)sc->sc_alias);
-			sc->sc_alias = NULL;
-		}
-		g_free(physpath);
+	if (strcmp(attr, "GEOM::physpath") == 0) {
+		g_dev_set_physpath(cp);
+		return;
 	}
 }
 



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