Attachment 169058 Details for Bug 207667 – possible patch for stable/9 zfs

[patch] possible patch for stable/9 zfs

file_207667.txt (text/plain), 3.16 KB, created by Andriy Gapon on 2016-04-06 21:10:32 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andriy Gapon

Created: 2016-04-06 21:10:32 UTC

Size: 3.16 KB

patch

obsolete

>commit 213908a407fa0f2c3868bfdc2da09e81a6b0118a
>Author: Andriy Gapon <avg@icyb.net.ua>
>Date:   Thu Oct 25 09:58:36 2012 +0300
>
>    zfs/geom: never re-use a consumer for simplicity/clarity
>    
>    And call vdev_geom_detach in vdev_geom_close directly
>    instead of asynchronous call via geom event thread to make sure
>    that the old consumer is detached/destroyed when vdev_geom_close
>    returns.
>    With the new approach to always creating a dedicated consumer the new
>    consumer could fail g_access call because the old consumer could still
>    be attached and holding references (including the exclusive one).
>    
>    My understanding is that the asynchronous approach was used to avoid
>    some locking order issues with zvol, which should be already resolved.
>
>diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
>index 4f2d0fe3fd488..c75a66a123382 100644
>--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
>+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
>@@ -152,30 +152,17 @@ vdev_geom_attach(struct g_provider *pp)
> 		}
> 		ZFS_LOG(1, "Created geom and consumer for %s.", pp->name);
> 	} else {
>-		/* Check if we are already connected to this provider. */
>-		LIST_FOREACH(cp, &gp->consumer, consumer) {
>-			if (cp->provider == pp) {
>-				ZFS_LOG(1, "Found consumer for %s.", pp->name);
>-				break;
>-			}
>+		cp = g_new_consumer(gp);
>+		if (g_attach(cp, pp) != 0) {
>+			g_destroy_consumer(cp);
>+			return (NULL);
> 		}
>-		if (cp == NULL) {
>-			cp = g_new_consumer(gp);
>-			if (g_attach(cp, pp) != 0) {
>-				g_destroy_consumer(cp);
>-				return (NULL);
>-			}
>-			if (g_access(cp, 1, 0, 1) != 0) {
>-				g_detach(cp);
>-				g_destroy_consumer(cp);
>-				return (NULL);
>-			}
>-			ZFS_LOG(1, "Created consumer for %s.", pp->name);
>-		} else {
>-			if (g_access(cp, 1, 0, 1) != 0)
>-				return (NULL);
>-			ZFS_LOG(1, "Used existing consumer for %s.", pp->name);
>+		if (g_access(cp, 1, 0, 1) != 0) {
>+			g_detach(cp);
>+			g_destroy_consumer(cp);
>+			return (NULL);
> 		}
>+		ZFS_LOG(1, "Created consumer for %s.", pp->name);
> 	}
> 	cp->flags |= G_CF_DIRECT_SEND | G_CF_DIRECT_RECEIVE;
> 	return (cp);
>@@ -192,15 +179,10 @@ vdev_geom_detach(void *arg, int flag __unused)
> 	gp = cp->geom;
> 
> 	ZFS_LOG(1, "Closing access to %s.", cp->provider->name);
>-	g_access(cp, -1, 0, -1);
>-	/* Destroy consumer on last close. */
>-	if (cp->acr == 0 && cp->ace == 0) {
>-		ZFS_LOG(1, "Destroyed consumer to %s.", cp->provider->name);
>-		if (cp->acw > 0)
>-			g_access(cp, 0, -cp->acw, 0);
>-		g_detach(cp);
>-		g_destroy_consumer(cp);
>-	}
>+	g_access(cp, -1, -cp->acw, -1);
>+	g_detach(cp);
>+	g_destroy_consumer(cp);
>+
> 	/* Destroy geom if there are no consumers left. */
> 	if (LIST_EMPTY(&gp->consumer)) {
> 		ZFS_LOG(1, "Destroyed geom %s.", gp->name);
>@@ -737,8 +719,9 @@ vdev_geom_close(vdev_t *vd)
> 		return;
> 	vd->vdev_tsd = NULL;
> 	vd->vdev_delayed_close = B_FALSE;
>-	cp->private = NULL;	/* XXX locking */
>-	g_post_event(vdev_geom_detach, cp, M_WAITOK, NULL);
>+	g_topology_lock();
>+	vdev_geom_detach(cp, 0);
>+	g_topology_unlock();
> }
> 
> static void

Actions: View | Diff

Attachments on bug 207667: 169058 | 180741