Date: Tue, 16 Aug 2011 22:20:45 +0000 (UTC) From: "Justin T. Gibbs" <gibbs@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r224920 - projects/zfsd/head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs Message-ID: <201108162220.p7GMKjke075680@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: gibbs Date: Tue Aug 16 22:20:45 2011 New Revision: 224920 URL: http://svn.freebsd.org/changeset/base/224920 Log: Close several race conditions in the ZFS vdev geom module, most triggered by concurrent ZFS reprobe and GEOM orphan processing. sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c: o Make vdev_geom_close() synchronous instead of deferring its work to a GEOM event handler. This prevents a future open call from referencing a consumer that is scheduled for destruction. o Move initialization of the consumer private pointer (which references the ZFS vdev object using this consumer) into vdev_geom_attach(). This guarantees that an orphan event received during the small windows where the topology lock is dropped in open processing, is effective in marking a vdev as needing to be removed. o Move clearing of the consumer private pointer from vdev_geom_close() into vdev_geom_detach(). When vdev_geom_open() fails, vdev_geom_detach is called directly, which used to bypass this necessary step. o Modify vdev_geom_orphan handler to ignore a consumer that that is no longer associated with a vdev. Sponsored by: Spectra Logic Corporation Modified: projects/zfsd/head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Modified: projects/zfsd/head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c ============================================================================== --- projects/zfsd/head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Tue Aug 16 21:51:29 2011 (r224919) +++ projects/zfsd/head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Tue Aug 16 22:20:45 2011 (r224920) @@ -64,6 +64,10 @@ vdev_geom_orphan(struct g_consumer *cp) g_topology_assert(); vd = cp->private; + if (vd == NULL) { + /* Vdev close in progress. Ignore the event. */ + return; + } /* * Orphan callbacks occur from the GEOM event thread. @@ -85,7 +89,7 @@ vdev_geom_orphan(struct g_consumer *cp) } static struct g_consumer * -vdev_geom_attach(struct g_provider *pp) +vdev_geom_attach(struct g_provider *pp, vdev_t *vd) { struct g_geom *gp; struct g_consumer *cp; @@ -140,20 +144,27 @@ vdev_geom_attach(struct g_provider *pp) ZFS_LOG(1, "Used existing consumer for %s.", pp->name); } } + cp->private = vd; return (cp); } static void -vdev_geom_detach(void *arg, int flag __unused) +vdev_geom_detach(void *arg) { struct g_geom *gp; struct g_consumer *cp; + vdev_t *vd; g_topology_assert(); cp = arg; gp = cp->geom; ZFS_LOG(1, "Closing access to %s.", cp->provider->name); + vd = cp->private; + if (vd != NULL) { + vd->vdev_tsd = NULL; + cp->private = NULL; + } g_access(cp, -1, 0, -1); /* Destroy consumer on last close. */ if (cp->acr == 0 && cp->ace == 0) { @@ -328,7 +339,7 @@ vdev_geom_attach_by_guids(vdev_t *vd) if (pguid != spa_guid(vd->vdev_spa) || vguid != vd->vdev_guid) continue; - cp = vdev_geom_attach(pp); + cp = vdev_geom_attach(pp, vd); if (cp == NULL) { printf("ZFS WARNING: Unable to attach to %s.\n", pp->name); @@ -394,7 +405,7 @@ vdev_geom_open_by_path(vdev_t *vd, int c pp = g_provider_by_name(vd->vdev_path + sizeof("/dev/") - 1); if (pp != NULL) { ZFS_LOG(1, "Found provider by name %s.", vd->vdev_path); - cp = vdev_geom_attach(pp); + cp = vdev_geom_attach(pp, vd); if (cp != NULL && check_guid && ISP2(pp->sectorsize) && pp->sectorsize <= VDEV_PAD_SIZE) { g_topology_unlock(); @@ -402,7 +413,7 @@ vdev_geom_open_by_path(vdev_t *vd, int c g_topology_lock(); if (pguid != spa_guid(vd->vdev_spa) || vguid != vd->vdev_guid) { - vdev_geom_detach(cp, 0); + vdev_geom_detach(cp); cp = NULL; ZFS_LOG(1, "guid mismatch for provider %s: " "%ju:%ju != %ju:%ju.", vd->vdev_path, @@ -482,7 +493,7 @@ vdev_geom_open(vdev_t *vd, uint64_t *psi !ISP2(cp->provider->sectorsize)) { ZFS_LOG(1, "Provider %s has unsupported sectorsize.", vd->vdev_path); - vdev_geom_detach(cp, 0); + vdev_geom_detach(cp); error = EINVAL; cp = NULL; } else if (cp->acw == 0 && (spa_mode(vd->vdev_spa) & FWRITE) != 0) { @@ -499,10 +510,11 @@ vdev_geom_open(vdev_t *vd, uint64_t *psi if (error != 0) { printf("ZFS WARNING: Unable to open %s for writing (error=%d).\n", vd->vdev_path, error); - vdev_geom_detach(cp, 0); + vdev_geom_detach(cp); cp = NULL; } } + g_topology_unlock(); PICKUP_GIANT(); if (cp == NULL) { @@ -510,7 +522,6 @@ vdev_geom_open(vdev_t *vd, uint64_t *psi return (error); } - cp->private = vd; vd->vdev_tsd = cp; pp = cp->provider; @@ -547,9 +558,9 @@ vdev_geom_close(vdev_t *vd) cp = vd->vdev_tsd; if (cp == NULL) return; - vd->vdev_tsd = NULL; - vd->vdev_delayed_close = B_FALSE; - g_post_event(vdev_geom_detach, cp, M_WAITOK, NULL); + g_topology_lock(); + vdev_geom_detach(cp); + g_topology_unlock(); } static void
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201108162220.p7GMKjke075680>