Date: Mon, 11 May 2020 22:39:53 +0000 (UTC) From: Conrad Meyer <cem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r360941 - head/sys/geom/mirror Message-ID: <202005112239.04BMdrq9084552@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202005112239.04BMdrq9084552>