From owner-svn-src-projects@FreeBSD.ORG Tue Aug 16 22:20:45 2011
Return-Path: <owner-svn-src-projects@FreeBSD.ORG>
Delivered-To: svn-src-projects@freebsd.org
Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34])
by hub.freebsd.org (Postfix) with ESMTP id A195D1065673;
Tue, 16 Aug 2011 22:20:45 +0000 (UTC)
(envelope-from gibbs@FreeBSD.org)
Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c])
by mx1.freebsd.org (Postfix) with ESMTP id 906508FC19;
Tue, 16 Aug 2011 22:20:45 +0000 (UTC)
Received: from svn.freebsd.org (localhost [127.0.0.1])
by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p7GMKj7o075682;
Tue, 16 Aug 2011 22:20:45 GMT (envelope-from gibbs@svn.freebsd.org)
Received: (from gibbs@localhost)
by svn.freebsd.org (8.14.4/8.14.4/Submit) id p7GMKjke075680;
Tue, 16 Aug 2011 22:20:45 GMT (envelope-from gibbs@svn.freebsd.org)
Message-Id: <201108162220.p7GMKjke075680@svn.freebsd.org>
From: "Justin T. Gibbs" <gibbs@FreeBSD.org>
Date: Tue, 16 Aug 2011 22:20:45 +0000 (UTC)
To: src-committers@freebsd.org, svn-src-projects@freebsd.org
X-SVN-Group: projects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Cc:
Subject: svn commit: r224920 -
projects/zfsd/head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
X-BeenThere: svn-src-projects@freebsd.org
X-Mailman-Version: 2.1.5
Precedence: list
List-Id: "SVN commit messages for the src " projects"
tree" <svn-src-projects.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-projects>,
<mailto:svn-src-projects-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/svn-src-projects>
List-Post: <mailto:svn-src-projects@freebsd.org>
List-Help: <mailto:svn-src-projects-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/svn-src-projects>,
<mailto:svn-src-projects-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Tue, 16 Aug 2011 22:20:45 -0000
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