Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 4 Oct 2017 07:37:57 +0000 (UTC)
From:      Andriy Gapon <avg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r324254 - stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Message-ID:  <201710040737.v947bvCA099381@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: avg
Date: Wed Oct  4 07:37:57 2017
New Revision: 324254
URL: https://svnweb.freebsd.org/changeset/base/324254

Log:
  MFC r323483: zfsctl_snapdir_lookup should be able to handle an uncovered vnode

Modified:
  stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
==============================================================================
--- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c	Wed Oct  4 07:37:36 2017	(r324253)
+++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c	Wed Oct  4 07:37:57 2017	(r324254)
@@ -816,6 +816,12 @@ zfsctl_snapshot_vnode_setup(vnode_t *vp, void *arg)
  * Lookup entry point for the 'snapshot' directory.  Try to open the
  * snapshot if it exist, creating the pseudo filesystem vnode as necessary.
  * Perform a mount of the associated dataset on top of the vnode.
+ * There are four possibilities:
+ * - the snapshot node and vnode do not exist
+ * - the snapshot vnode is covered by the mounted snapshot
+ * - the snapshot vnode is not covered yet, the mount operation is in progress
+ * - the snapshot vnode is not covered, because the snapshot has been unmounted
+ * The last two states are transient and should be relatively short-lived.
  */
 int
 zfsctl_snapdir_lookup(ap)
@@ -881,7 +887,7 @@ zfsctl_snapdir_lookup(ap)
 
 		/*
 		 * The vnode must be referenced at least by this thread and
-		 * the mounted snapshot or the thread doing the mounting.
+		 * the mount point or the thread doing the mounting.
 		 * There can be more references from concurrent lookups.
 		 */
 		KASSERT(vrefcnt(*vpp) > 1, ("found unreferenced mountpoint"));
@@ -893,22 +899,31 @@ zfsctl_snapdir_lookup(ap)
 		if (err != EJUSTRETURN)
 			return (err);
 
-#ifdef INVARIANTS
 		/*
-		 * If the vnode not covered yet, then the mount operation
-		 * must be in progress.
+		 * If the vnode is not covered, then either the mount operation
+		 * is in progress or the snapshot has already been unmounted
+		 * but the vnode hasn't been inactivated and reclaimed yet.
+		 * We can try to re-use the vnode in the latter case.
 		 */
 		VI_LOCK(*vpp);
-		KASSERT(((*vpp)->v_iflag & VI_MOUNT) != 0,
-		    ("snapshot vnode not covered"));
-		VI_UNLOCK(*vpp);
-#endif
-		vput(*vpp);
+		if (((*vpp)->v_iflag & VI_MOUNT) == 0) {
+			/* Upgrade to exclusive lock in order to:
+			 * - avoid race conditions
+			 * - satisfy the contract of mount_snapshot()
+			 */
+			err = VOP_LOCK(*vpp, LK_TRYUPGRADE | LK_INTERLOCK);
+			if (err == 0)
+				break;
+		} else {
+			VI_UNLOCK(*vpp);
+		}
 
 		/*
-		 * In this situation we can loop on uncontested locks and starve
+		 * In this state we can loop on uncontested locks and starve
 		 * the thread doing the lengthy, non-trivial mount operation.
+		 * So, yield to prevent that from happening.
 		 */
+		vput(*vpp);
 		kern_yield(PRI_USER);
 	}
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201710040737.v947bvCA099381>