diff options
author | asomers <asomers@FreeBSD.org> | 2017-05-30 22:54:52 +0000 |
---|---|---|
committer | asomers <asomers@FreeBSD.org> | 2017-05-30 22:54:52 +0000 |
commit | 521c6cb98bd27570b3c586d7feb2f40b6f254d6a (patch) | |
tree | f0769362392e881aaec2f6a15822485066b68132 /sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c | |
parent | da217d0db5b3aeb619707c893bc2e054f8b9772d (diff) | |
download | FreeBSD-src-521c6cb98bd27570b3c586d7feb2f40b6f254d6a.zip FreeBSD-src-521c6cb98bd27570b3c586d7feb2f40b6f254d6a.tar.gz |
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
Diffstat (limited to 'sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c')
-rw-r--r-- | sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c | 114 |
1 files changed, 65 insertions, 49 deletions
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c index 6fc03f4..c4b5370 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c @@ -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_consumer *cp) } 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_t do_null_update) 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; - } - - if (strcmp(attr, "GEOM::physpath") == 0) { - vdev_geom_set_physpath(cp, /*do_null_update*/B_TRUE); - 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; + } } } 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 *max_psize, 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) { @@ -907,11 +928,6 @@ skip_open: */ vd->vdev_nowritecache = B_FALSE; - /* - * Determine the device's rotation rate. - */ - vdev_geom_set_rotation_rate(vd, cp); - return (0); } |