git: 66191a76ace5 - main - unionfs: Improve locking assertions

From: Jason A. Harmening <jah_at_FreeBSD.org>
Date: Sat, 06 Nov 2021 14:02:19 UTC
The branch main has been updated by jah:

URL: https://cgit.FreeBSD.org/src/commit/?id=66191a76ace56af6603b343ad2e9a003e0589d70

commit 66191a76ace56af6603b343ad2e9a003e0589d70
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2021-10-28 05:31:16 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2021-11-06 14:08:33 +0000

    unionfs: Improve locking assertions
    
    Add an assertion to unionfs_node_update() that the upper vnode is
    exclusively locked; we already make the same assertion for the lower
    vnode.
    Also, assert in unionfs_noderem() that the vnode lock is not recursed
    and acquire v_lock with LK_NOWAIT.  Since v_lock is not the active
    lock for the vnode at this point, it should not be contended.
    Finally, remove VDIR assertions from unionfs_get_cached_vnode().
    lvp/uvp will be referenced but not locked at this point, so v_type
    may concurrently change due to vgonel().  The cached unionfs node,
    if one exists, would only have made it into the cache if lvp/uvp
    were of type VDIR at the time of insertion; the corresponding
    VDIR assert in unionfs_ins_cached_vnode() should be safe because
    lvp/uvp will be locked by that time and will not be used if either
    is doomed.
    
    Noted by:       kib
    Reviewed by:    kib
    Differential Revision:  https://reviews.freebsd.org/D32629
---
 sys/fs/unionfs/union_subr.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index 449e171d52cf..0ca209c47502 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -183,11 +183,6 @@ unionfs_get_cached_vnode(struct vnode *uvp, struct vnode *lvp,
 {
 	struct vnode *vp;
 
-	KASSERT(uvp == NULLVP || uvp->v_type == VDIR,
-	    ("%s: v_type != VDIR", __func__));
-	KASSERT(lvp == NULLVP || lvp->v_type == VDIR,
-	    ("%s: v_type != VDIR", __func__));
-
 	vp = NULLVP;
 	VI_LOCK(dvp);
 	if (uvp != NULLVP)
@@ -209,6 +204,8 @@ unionfs_ins_cached_vnode(struct unionfs_node *uncp,
 	struct unionfs_node_hashhead *hd;
 	struct vnode *vp;
 
+	ASSERT_VOP_ELOCKED(uncp->un_uppervp, __func__);
+	ASSERT_VOP_ELOCKED(uncp->un_lowervp, __func__);
 	KASSERT(uncp->un_uppervp == NULLVP || uncp->un_uppervp->v_type == VDIR,
 	    ("%s: v_type != VDIR", __func__));
 	KASSERT(uncp->un_lowervp == NULLVP || uncp->un_lowervp->v_type == VDIR,
@@ -439,7 +436,9 @@ unionfs_noderem(struct vnode *vp, struct thread *td)
 	struct vnode   *dvp;
 	int		count;
 
-	if (lockmgr(&(vp->v_lock), LK_EXCLUSIVE, NULL) != 0)
+	KASSERT(vp->v_vnlock->lk_recurse == 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__);
 
 	/*
@@ -803,6 +802,7 @@ unionfs_node_update(struct unionfs_node *unp, struct vnode *uvp,
 	vp = UNIONFSTOV(unp);
 	lvp = unp->un_lowervp;
 	ASSERT_VOP_ELOCKED(lvp, __func__);
+	ASSERT_VOP_ELOCKED(uvp, __func__);
 	dvp = unp->un_dvp;
 
 	/*