From owner-svn-src-projects@FreeBSD.ORG Tue Aug 16 22:20:45 2011 Return-Path: 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" 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" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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