git: 6ff167aa427c - main - unionfs: allow lock recursion when reclaiming the root vnode

From: Jason A. Harmening <jah_at_FreeBSD.org>
Date: Thu, 03 Feb 2022 02:58:21 UTC
The branch main has been updated by jah:

URL: https://cgit.FreeBSD.org/src/commit/?id=6ff167aa427c75d97153a4ffb343f8cbf93a22db

commit 6ff167aa427c75d97153a4ffb343f8cbf93a22db
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2022-01-30 20:52:08 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2022-02-03 03:08:17 +0000

    unionfs: allow lock recursion when reclaiming the root vnode
    
    The unionfs root vnode will always share a lock with its lower vnode.
    If unionfs was mounted with the 'below' option, this will also be the
    vnode covered by the unionfs mount.  During unmount, the covered vnode
    will be locked by dounmount() while the unionfs root vnode will be
    locked by vgone().  This effectively requires recursion on the same
    underlying like, albeit through two different vnodes.
    
    Reported by:    pho
    Reviewed by:    kib, markj, pho
    Differential Revision:  https://reviews.freebsd.org/D34109
---
 sys/fs/unionfs/union_subr.c  | 13 +++++++++++--
 sys/fs/unionfs/union_vnops.c |  7 +++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index 557d4589df55..466c88c705f4 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -442,7 +442,16 @@ unionfs_noderem(struct vnode *vp)
 	int		count;
 	int		writerefs;
 
-	KASSERT(vp->v_vnlock->lk_recurse == 0,
+	/*
+	 * The root vnode lock may be recursed during forcible, because
+	 * it may share the same lock as the unionfs mount's covered vnode,
+	 * which is locked across VFS_UNMOUNT().  This lock will then be
+	 * recursively taken during the vflush() issued by unionfs_unmount().
+	 * But we still only need to lock the unionfs lock once, because only
+	 * one of those lock operations was taken against a unionfs vnode and
+	 * will be undone against a unionfs vnode.
+	 */
+	KASSERT(vp->v_vnlock->lk_recurse == 0 || (vp->v_vflag & VV_ROOT) != 0,
 	    ("%s: vnode %p locked recursively", __func__, vp));
 	if (lockmgr(&vp->v_lock, LK_EXCLUSIVE | LK_NOWAIT, NULL) != 0)
 		panic("%s: failed to acquire lock for vnode lock", __func__);
@@ -821,7 +830,7 @@ unionfs_node_update(struct unionfs_node *unp, struct vnode *uvp,
 	VNASSERT(vp->v_writecount == 0, vp,
 	    ("%s: non-zero writecount", __func__));
 	/*
-	 * Uppdate the upper vnode's lock state to match the lower vnode,
+	 * Update the upper vnode's lock state to match the lower vnode,
 	 * and then switch the unionfs vnode's lock to the upper vnode.
 	 */
 	lockrec = lvp->v_vnlock->lk_recurse;
diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index 43378f709df7..28e306fdb3f8 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -1957,8 +1957,11 @@ unionfs_lock(struct vop_lock1_args *ap)
 		flags |= LK_NOWAIT;
 
 	/*
-	 * Sometimes, lower or upper is already exclusive locked.
-	 * (ex. vfs_domount: mounted vnode is already locked.)
+	 * During unmount, the root vnode lock may be taken recursively,
+	 * because it may share the same v_vnlock field as the vnode covered by
+	 * the unionfs mount.  The covered vnode is locked across VFS_UNMOUNT(),
+	 * and the same lock may be taken recursively here during vflush()
+	 * issued by unionfs_unmount().
 	 */
 	if ((flags & LK_TYPE_MASK) == LK_EXCLUSIVE &&
 	    (vp->v_vflag & VV_ROOT) != 0)