svn commit: r318189 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Alan Somers
asomers at FreeBSD.org
Thu May 11 16:26:57 UTC 2017
Author: asomers
Date: Thu May 11 16:26:56 2017
New Revision: 318189
URL: https://svnweb.freebsd.org/changeset/base/318189
Log:
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
MFC after: 1 week
Sponsored by: Spectra Logic Corp
Differential Revision: https://reviews.freebsd.org/D10391
Modified:
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Thu May 11 15:19:04 2017 (r318188)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Thu May 11 16:26:56 2017 (r318189)
@@ -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);
@@ -85,21 +95,16 @@ vdev_geom_set_rotation_rate(vdev_t *vd,
}
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;
@@ -130,37 +135,40 @@ vdev_geom_set_physpath(struct g_consumer
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.
@@ -176,8 +184,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 *
@@ -265,21 +277,8 @@ vdev_geom_attach(struct g_provider *pp,
}
}
- /*
- * 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);
@@ -289,16 +288,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);
@@ -324,16 +319,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);
}
@@ -870,11 +875,27 @@ vdev_geom_open(vdev_t *vd, uint64_t *psi
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) {
@@ -905,11 +926,6 @@ skip_open:
*/
vd->vdev_nowritecache = B_FALSE;
- /*
- * Determine the device's rotation rate.
- */
- vdev_geom_set_rotation_rate(vd, cp);
-
return (0);
}
More information about the svn-src-head
mailing list