svn commit: r360941 - head/sys/geom/mirror

[Conrad Meyer]

Author: cem
Date: Mon May 11 22:39:53 2020
New Revision: 360941
URL: https://svnweb.freebsd.org/changeset/base/360941

Log:
  geom(4) mirror: Do not panic on gmirror(8) insert, resize
  
  Geom_mirror initialization occurs in spurts and the present of a
  non-destroyed g_mirror softc does not always indicate that the geom has
  launched (i.e., has an sc_provider).
  
  Some gmirror(8) commands (via g_mirror_ctl) depend on a g_mirror's
  sc_provider (insert and resize).  For those commands, g_mirror_ctl is
  modified to sleep-poll in an interruptible way until the target geom is
  either launched or destroyed.
  
  Reviewed by:	markj
  Tested by:	markj
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D24780

Modified:
  head/sys/geom/mirror/g_mirror_ctl.c

Modified: head/sys/geom/mirror/g_mirror_ctl.c
==============================================================================
--- head/sys/geom/mirror/g_mirror_ctl.c	Mon May 11 22:38:32 2020	(r360940)
+++ head/sys/geom/mirror/g_mirror_ctl.c	Mon May 11 22:39:53 2020	(r360941)
@@ -44,6 +44,10 @@ __FBSDID("$FreeBSD$");
 #include <geom/geom_int.h>
 #include <geom/mirror/g_mirror.h>
 
+/*
+ * Configure, Rebuild, Remove, Deactivate, Forget, and Stop operations do not
+ * seem to depend on any particular g_mirror initialization state.
+ */
 static struct g_mirror_softc *
 g_mirror_find_device(struct g_class *mp, const char *name)
 {
@@ -61,6 +65,10 @@ g_mirror_find_device(struct g_class *mp, const char *n
 		    strcmp(sc->sc_name, name) == 0) {
 			g_topology_unlock();
 			sx_xlock(&sc->sc_lock);
+			if ((sc->sc_flags & G_MIRROR_DEVICE_FLAG_DESTROY) != 0) {
+				sx_xunlock(&sc->sc_lock);
+				return (NULL);
+			}
 			return (sc);
 		}
 	}
@@ -68,6 +76,55 @@ g_mirror_find_device(struct g_class *mp, const char *n
 	return (NULL);
 }
 
+/* Insert and Resize operations depend on a launched GEOM (sc_provider). */
+#define	GMFL_VALID_FLAGS	(M_WAITOK | M_NOWAIT)
+static struct g_mirror_softc *
+g_mirror_find_launched_device(struct g_class *mp, const char *name, int flags)
+{
+	struct g_mirror_softc *sc;
+	int error;
+
+	KASSERT((flags & ~GMFL_VALID_FLAGS) == 0 &&
+	    flags != GMFL_VALID_FLAGS && flags != 0,
+	    ("%s: Invalid flags %x\n", __func__, (unsigned)flags));
+#undef	GMFL_VALID_FLAGS
+
+	while (true) {
+		sc = g_mirror_find_device(mp, name);
+		if (sc == NULL)
+			return (NULL);
+		if (sc->sc_provider != NULL)
+			return (sc);
+		if (flags & M_NOWAIT) {
+			sx_xunlock(&sc->sc_lock);
+			return (NULL);
+		}
+
+		/*
+		 * This is a dumb hack.  G_mirror does not expose any real
+		 * wakeup API for observing state changes, and even if it did,
+		 * its "RUNNING" state does not actually reflect all softc
+		 * elements being initialized.
+		 *
+		 * Revamping g_mirror to have a 3rd, ACTUALLY_RUNNING state and
+		 * updating all assertions and sc_state checks is a large work
+		 * and would be easy to introduce regressions.
+		 *
+		 * Revamping g_mirror to have a wakeup for state changes would
+		 * be difficult if one wanted to capture more than just
+		 * sc_state and sc_provider.
+		 *
+		 * For now, just dummy sleep-poll until sc_provider shows up,
+		 * the user cancels, or the g_mirror is destroyed.
+		 */
+		error = sx_sleep(&sc, &sc->sc_lock, PRIBIO | PCATCH | PDROP,
+		    "GM:launched", 1);
+		if (error != 0 && error != EWOULDBLOCK)
+			return (NULL);
+	}
+	__unreachable();
+}
+
 static struct g_mirror_disk *
 g_mirror_find_disk(struct g_mirror_softc *sc, const char *name)
 {
@@ -605,7 +662,7 @@ g_mirror_ctl_insert(struct gctl_req *req, struct g_cla
 		gctl_error(req, "No 'arg%u' argument.", 0);
 		return;
 	}
-	sc = g_mirror_find_device(mp, name);
+	sc = g_mirror_find_launched_device(mp, name, M_WAITOK);
 	if (sc == NULL) {
 		gctl_error(req, "No such device: %s.", name);
 		return;
@@ -847,7 +904,7 @@ g_mirror_ctl_resize(struct gctl_req *req, struct g_cla
 		gctl_error(req, "Invalid '%s' argument.", "size");
 		return;
 	}
-	sc = g_mirror_find_device(mp, name);
+	sc = g_mirror_find_launched_device(mp, name, M_WAITOK);
 	if (sc == NULL) {
 		gctl_error(req, "No such device: %s.", name);
 		return;