zfsctl_snapdir_lookup should be able to handle an uncovered vnode

The uncovered vnode is possible because there is no guarantee that
its hold count would go to zero (and it would be inactivated and reclaimed)
immediately after a covering filesystem is unmounted.
So, such a vnode should be expected and it is possible to re-use it
without any trouble.

MFC after:	3 weeks
Sponsored by:	Panzura

1 files changed, 25 insertions, 10 deletions

diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
index 7c1b6d3cea7a..9636d6d4b32c 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c
@@ -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);
 	}