Date: Tue, 30 May 2017 22:54:52 +0000 (UTC) From: Alan Somers <asomers@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: r319268 - stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs Message-ID: <201705302254.v4UMsqW8080418@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Tue May 30 22:54:52 2017 New Revision: 319268 URL: https://svnweb.freebsd.org/changeset/base/319268 Log: MFC r318189: vdev_geom may associate multiple vdevs per g_consumer vdev_geom.c currently uses the g_consumer's private field to point to a vdev_t. That way, a GEOM event can cause a change to a ZFS vdev. For example, when you remove a disk, the vdev's status will change to REMOVED. However, vdev_geom will sometimes attach multiple vdevs to the same GEOM consumer. If this happens, then geom events will only be propagated to one of the vdevs. Fix this by storing a linked list of vdevs in g_consumer's private field. sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c * g_consumer.private now stores a linked list of vdev pointers associated with the consumer instead of just a single vdev pointer. * Change vdev_geom_set_physpath's signature to more closely match vdev_geom_set_rotation_rate * Don't bother calling g_access in vdev_geom_set_physpath. It's guaranteed that we've already accessed the consumer by the time we get here. * Don't call vdev_geom_set_physpath in vdev_geom_attach. Instead, call it in vdev_geom_open, after we know that the open has succeeded. PR: 218634 Reviewed by: gibbs Sponsored by: Spectra Logic Corp Differential Revision: https://reviews.freebsd.org/D10391 Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.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 Tue May 30 22:48:17 2017 (r319267) +++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Tue May 30 22:54:52 2017 (r319268) @@ -49,6 +49,16 @@ struct g_class zfs_vdev_class = { .attrchanged = vdev_geom_attrchanged, }; +struct consumer_vdev_elem { + SLIST_ENTRY(consumer_vdev_elem) elems; + vdev_t *vd; +}; + +SLIST_HEAD(consumer_priv_t, consumer_vdev_elem); +_Static_assert(sizeof(((struct g_consumer*)NULL)->private) + == sizeof(struct consumer_priv_t*), + "consumer_priv_t* can't be stored in g_consumer.private"); + DECLARE_GEOM_CLASS(zfs_vdev_class, zfs_vdev); SYSCTL_DECL(_vfs_zfs_vdev); @@ -87,21 +97,16 @@ vdev_geom_set_rotation_rate(vdev_t *vd, struct g_consu } static void -vdev_geom_set_physpath(struct g_consumer *cp, boolean_t do_null_update) +vdev_geom_set_physpath(vdev_t *vd, struct g_consumer *cp, + boolean_t do_null_update) { boolean_t needs_update = B_FALSE; - vdev_t *vd; char *physpath; int error, physpath_len; - if (g_access(cp, 1, 0, 0) != 0) - return; - - 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); - g_access(cp, -1, 0, 0); if (error == 0) { char *old_physpath; @@ -132,37 +137,40 @@ vdev_geom_set_physpath(struct g_consumer *cp, boolean_ static void vdev_geom_attrchanged(struct g_consumer *cp, const char *attr) { - vdev_t *vd; char *old_physpath; + struct consumer_priv_t *priv; + struct consumer_vdev_elem *elem; int error; - vd = cp->private; - if (vd == NULL) + priv = (struct consumer_priv_t*)&cp->private; + if (SLIST_EMPTY(priv)) return; - if (strcmp(attr, "GEOM::rotation_rate") == 0) { - vdev_geom_set_rotation_rate(vd, cp); - return; + SLIST_FOREACH(elem, priv, elems) { + vdev_t *vd = elem->vd; + 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(vd, cp, /*null_update*/B_TRUE); + return; + } } - - if (strcmp(attr, "GEOM::physpath") == 0) { - vdev_geom_set_physpath(cp, /*do_null_update*/B_TRUE); - return; - } } static void vdev_geom_orphan(struct g_consumer *cp) { - vdev_t *vd; + struct consumer_priv_t *priv; + struct consumer_vdev_elem *elem; g_topology_assert(); - vd = cp->private; - if (vd == NULL) { + priv = (struct consumer_priv_t*)&cp->private; + if (SLIST_EMPTY(priv)) /* Vdev close in progress. Ignore the event. */ return; - } /* * Orphan callbacks occur from the GEOM event thread. @@ -178,8 +186,12 @@ vdev_geom_orphan(struct g_consumer *cp) * async removal support to invoke a close on this * vdev once it is safe to do so. */ - vd->vdev_remove_wanted = B_TRUE; - spa_async_request(vd->vdev_spa, SPA_ASYNC_REMOVE); + SLIST_FOREACH(elem, priv, elems) { + vdev_t *vd = elem->vd; + + vd->vdev_remove_wanted = B_TRUE; + spa_async_request(vd->vdev_spa, SPA_ASYNC_REMOVE); + } } static struct g_consumer * @@ -267,21 +279,8 @@ vdev_geom_attach(struct g_provider *pp, vdev_t *vd) } } - /* - * BUG: cp may already belong to a vdev. This could happen if: - * 1) That vdev is a shared spare, or - * 2) We are trying to reopen a missing vdev and we are scanning by - * guid. In that case, we'll ultimately fail to open this consumer, - * but not until after setting the private field. - * The solution is to: - * 1) Don't set the private field until after the open succeeds, and - * 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); @@ -291,16 +290,12 @@ static void vdev_geom_detach(struct g_consumer *cp, boolean_t open_for_read) { struct g_geom *gp; - vdev_t *vd; g_topology_assert(); ZFS_LOG(1, "Detaching from %s.", cp->provider && cp->provider->name ? cp->provider->name : "NULL"); - vd = cp->private; - cp->private = NULL; - gp = cp->geom; if (open_for_read) g_access(cp, -1, 0, -1); @@ -326,16 +321,26 @@ static void vdev_geom_close_locked(vdev_t *vd) { struct g_consumer *cp; + struct consumer_priv_t *priv; + struct consumer_vdev_elem *elem, *elem_temp; g_topology_assert(); cp = vd->vdev_tsd; - vd->vdev_tsd = NULL; vd->vdev_delayed_close = B_FALSE; if (cp == NULL) return; ZFS_LOG(1, "Closing access to %s.", cp->provider->name); + KASSERT(cp->private != NULL, ("%s: cp->private is NULL", __func__)); + priv = (struct consumer_priv_t*)&cp->private; + vd->vdev_tsd = NULL; + SLIST_FOREACH_SAFE(elem, priv, elems, elem_temp) { + if (elem->vd == vd) { + SLIST_REMOVE(priv, elem, consumer_vdev_elem, elems); + g_free(elem); + } + } vdev_geom_detach(cp, B_TRUE); } @@ -872,11 +877,27 @@ vdev_geom_open(vdev_t *vd, uint64_t *psize, uint64_t * cp = NULL; } } + if (cp != NULL) { + struct consumer_priv_t *priv; + struct consumer_vdev_elem *elem; + priv = (struct consumer_priv_t*)&cp->private; + if (cp->private == NULL) + SLIST_INIT(priv); + elem = g_malloc(sizeof(*elem), M_WAITOK|M_ZERO); + elem->vd = vd; + SLIST_INSERT_HEAD(priv, elem, elems); + } + /* Fetch initial physical path information for this device. */ - if (cp != NULL) + if (cp != NULL) { vdev_geom_attrchanged(cp, "GEOM::physpath"); + /* Set other GEOM characteristics */ + vdev_geom_set_physpath(vd, cp, /*do_null_update*/B_FALSE); + vdev_geom_set_rotation_rate(vd, cp); + } + g_topology_unlock(); PICKUP_GIANT(); if (cp == NULL) { @@ -906,11 +927,6 @@ skip_open: * we will try again. */ vd->vdev_nowritecache = B_FALSE; - - /* - * Determine the device's rotation rate. - */ - vdev_geom_set_rotation_rate(vd, cp); return (0); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201705302254.v4UMsqW8080418>