vn_lock_pair(): handle the case of vp1->v_vnlock == vp2->v_vnlock

It is not enough to check vp1 == vp2 to detect lock recursion, since
vnodes might share the locks.  This might happen for e.g. stacked
filesystems (nullfs and other), and for FFS snapshots.

Switch from checking vnode equiality to check v_vnlock equiality, and
recheck the condition after vnode relock since reclamation or otner
parallel operation might change the vnode locks under us.

Return a value (not really an error) indicating the case that vnodes
share the lock, to simplify the unlock in caller.

Reviewed by:	jah, markj
Tested by:	pho
Sponsored by:	The FreeBSD Foundation
MFC after:	1 week
Differential revision:	https://reviews.freebsd.org/D57035

2 files changed, 26 insertions, 8 deletions

diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c
index 15704634ff62..1aa20954e4bd 100644
--- a/sys/kern/vfs_vnops.c
+++ b/sys/kern/vfs_vnops.c
@@ -4354,9 +4354,16 @@ vn_lock_pair_pause(const char *wmesg)
  * Only one of LK_SHARED and LK_EXCLUSIVE must be specified.
  * LK_NODDLKTREAT can be optionally passed.
  *
- * If vp1 == vp2, only one, most exclusive, lock is obtained on it.
+ * If vp1->v_vnlock == vp2->v_vnlock, only one, most exclusive, lock
+ * is obtained on the vnode(s).  The function accounts for the
+ * possibility of vp1 or vp2' v_vnlock changing while the
+ * corresponding vnode is unlocked.
+ *
+ * Return values:
+ *    0       - locked, two unlocks are required
+ *    EDEADLK - locked, vnodes share the same lock, only one unlock is due.
  */
-void
+int
 vn_lock_pair(struct vnode *vp1, bool vp1_locked, int lkflags1,
     struct vnode *vp2, bool vp2_locked, int lkflags2)
 {
@@ -4370,9 +4377,10 @@ vn_lock_pair(struct vnode *vp1, bool vp1_locked, int lkflags1,
 	MPASS((lkflags2 & ~(LK_SHARED | LK_EXCLUSIVE | LK_NODDLKTREAT)) == 0);
 
 	if (vp1 == NULL && vp2 == NULL)
-		return;
+		return (0);
 
-	if (vp1 == vp2) {
+recheck_same:
+	if (vp1 != NULL && vp2 != NULL && vp1->v_vnlock == vp2->v_vnlock) {
 		MPASS(vp1_locked == vp2_locked);
 
 		/* Select the most exclusive mode for lock. */
@@ -4385,20 +4393,26 @@ vn_lock_pair(struct vnode *vp1, bool vp1_locked, int lkflags1,
 			/* No need to relock if any lock is exclusive. */
 			if ((vp1->v_vnlock->lock_object.lo_flags &
 			    LK_NOSHARE) != 0)
-				return;
+				return (EDEADLK);
 
 			locked1 = VOP_ISLOCKED(vp1);
 			if (((lkflags1 & LK_SHARED) != 0 &&
 			    locked1 != LK_EXCLUSIVE) ||
 			    ((lkflags1 & LK_EXCLUSIVE) != 0 &&
 			    locked1 == LK_EXCLUSIVE))
-				return;
+				return (EDEADLK);
 			VOP_UNLOCK(vp1);
 		}
 
 		ASSERT_VOP_UNLOCKED(vp1, "vp1");
 		vn_lock(vp1, lkflags1 | LK_RETRY);
-		return;
+		if (vp1->v_vnlock == vp2->v_vnlock)
+			return (EDEADLK);
+		VOP_UNLOCK(vp1);
+		if (vp2_locked) {
+			VOP_UNLOCK(vp2);
+			vp2_locked = false;
+		}
 	}		
 
 	if (vp1 != NULL) {
@@ -4469,6 +4483,9 @@ vn_lock_pair(struct vnode *vp1, bool vp1_locked, int lkflags1,
 			vn_lock(vp1, lkflags1 | LK_RETRY);
 			vp1_locked = true;
 		}
+		if (vp1 != NULL && vp2 != NULL &&
+		    vp1->v_vnlock == vp2->v_vnlock)
+			goto recheck_same;
 	}
 	if (vp1 != NULL) {
 		if (lkflags1 == LK_EXCLUSIVE)
@@ -4482,6 +4499,7 @@ vn_lock_pair(struct vnode *vp1, bool vp1_locked, int lkflags1,
 		else
 			ASSERT_VOP_LOCKED(vp2, "vp2 ret");
 	}
+	return (0);
 }
 
 int
diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
index 4fee025a93ea..41b5e21fb879 100644
--- a/sys/sys/vnode.h
+++ b/sys/sys/vnode.h
@@ -781,7 +781,7 @@ bool	vn_isdisk_error(struct vnode *vp, int *errp);
 bool	vn_isdisk(struct vnode *vp);
 int	_vn_lock(struct vnode *vp, int flags, const char *file, int line);
 #define vn_lock(vp, flags) _vn_lock(vp, flags, __FILE__, __LINE__)
-void	vn_lock_pair(struct vnode *vp1, bool vp1_locked, int lkflags1,
+int	vn_lock_pair(struct vnode *vp1, bool vp1_locked, int lkflags1,
 	    struct vnode *vp2, bool vp2_locked, int lkflags2);
 int	vn_open(struct nameidata *ndp, int *flagp, int cmode, struct file *fp);
 int	vn_open_cred(struct nameidata *ndp, int *flagp, int cmode,