d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

vdev_geom may associate multiple vdevs per g_consumer

Browse Source 
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
This commit is contained in:
Alan Somers 2017-05-11 16:26:56 +00:00
parent 855fe445b3
commit 7ac72c256f
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=318189
1 changed files with 65 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

114
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
View File

@ -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, 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;
@ -130,37 +135,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.
@ -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, 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);
@ -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 *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) {
@ -905,11 +926,6 @@ vdev_geom_open(vdev_t *vd, uint64_t *psize, uint64_t *max_psize,
*/
vd->vdev_nowritecache = B_FALSE;
/*
* Determine the device's rotation rate.
*/
vdev_geom_set_rotation_rate(vd, cp);
return (0);
}