git: d877dd5767bc - main - unionfs: simplify writecount management

The branch main has been updated by jah:

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

commit d877dd5767bcbda5f3138facaceac6cab5820da3
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2021-12-21 23:49:15 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2022-01-03 03:52:58 +0000

    unionfs: simplify writecount management
    
    Use atomics to track the writecount granted to the underlying FS,
    and avoid holding the vnode interlock while calling the underling FS'
    VOP_ADD_WRITECOUNT().  This also fixes a WITNESS warning about nesting
    the same lock type.  Also add comments explaining why we need to track
    the writecount on the unionfs vnode in the first place.  Finally,
    simplify writecount management to only use the upper vnode and assert
    that we shouldn't have an active writecount on the lower vnode through
    unionfs.
    
    Reviewed by:    kib
    Differential Revision:  https://reviews.freebsd.org/D33611
---
 sys/fs/unionfs/union_subr.c  | 25 ++++++++++++++++++-------
 sys/fs/unionfs/union_vnops.c | 36 ++++++++++++++++++++----------------
 2 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index 551553fd8b20..9a706d79f9f7 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -57,6 +57,8 @@
 #include <sys/taskqueue.h>
 #include <sys/resourcevar.h>
 
+#include <machine/atomic.h>
+
 #include <security/mac/mac_framework.h>
 
 #include <vm/uma.h>
@@ -435,6 +437,7 @@ unionfs_noderem(struct vnode *vp)
 	struct vnode   *uvp;
 	struct vnode   *dvp;
 	int		count;
+	int		writerefs;
 
 	KASSERT(vp->v_vnlock->lk_recurse == 0,
 	    ("%s: vnode %p locked recursively", __func__, vp));
@@ -454,13 +457,6 @@ unionfs_noderem(struct vnode *vp)
 	vp->v_vnlock = &(vp->v_lock);
 	vp->v_data = NULL;
 	vp->v_object = NULL;
-	if (vp->v_writecount > 0) {
-		if (uvp != NULL)
-			VOP_ADD_WRITECOUNT(uvp, -vp->v_writecount);
-		else if (lvp != NULL)
-			VOP_ADD_WRITECOUNT(lvp, -vp->v_writecount);
-	} else if (vp->v_writecount < 0)
-		vp->v_writecount = 0;
 	if (unp->un_hashtbl != NULL) {
 		/*
 		 * Clear out any cached child vnodes.  This should only
@@ -479,6 +475,19 @@ unionfs_noderem(struct vnode *vp)
 	}
 	VI_UNLOCK(vp);
 
+	writerefs = atomic_load_int(&vp->v_writecount);
+	VNASSERT(writerefs >= 0, vp,
+	    ("%s: write count %d, unexpected text ref", __func__, writerefs));
+	/*
+	 * If we were opened for write, we leased the write reference
+	 * to the lower vnode.  If this is a reclamation due to the
+	 * forced unmount, undo the reference now.
+	 */
+	if (writerefs > 0) {
+		VNASSERT(uvp != NULL, vp,
+		    ("%s: write reference without upper vnode", __func__));
+		VOP_ADD_WRITECOUNT(uvp, -writerefs);
+	}
 	if (lvp != NULLVP)
 		VOP_UNLOCK(lvp);
 	if (uvp != NULLVP)
@@ -805,6 +814,8 @@ unionfs_node_update(struct unionfs_node *unp, struct vnode *uvp,
 	ASSERT_VOP_ELOCKED(uvp, __func__);
 	dvp = unp->un_dvp;
 
+	VNASSERT(vp->v_writecount == 0, vp,
+	    ("%s: non-zero writecount", __func__));
 	/*
 	 * Uppdate the upper vnode's lock state to match the lower vnode,
 	 * and then switch the unionfs vnode's lock to the upper vnode.
diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index 6cedfa3d142d..dc02e20f376f 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -61,6 +61,8 @@
 
 #include <fs/unionfs/union.h>
 
+#include <machine/atomic.h>
+
 #include <vm/vm.h>
 #include <vm/vm_extern.h>
 #include <vm/vm_object.h>
@@ -2523,26 +2525,28 @@ unionfs_add_writecount(struct vop_add_writecount_args *ap)
 {
 	struct vnode *tvp, *vp;
 	struct unionfs_node *unp;
-	int error;
+	int error, writerefs;
 
 	vp = ap->a_vp;
 	unp = VTOUNIONFS(vp);
-	tvp = unp->un_uppervp != NULL ? unp->un_uppervp : unp->un_lowervp;
-	VI_LOCK(vp);
+	tvp = unp->un_uppervp;
+	KASSERT(tvp != NULL,
+	    ("%s: adding write ref without upper vnode", __func__));
+	error = VOP_ADD_WRITECOUNT(tvp, ap->a_inc);
+	if (error != 0)
+		return (error);
+	/*
+	 * We need to track the write refs we've passed to the underlying
+	 * vnodes so that we can undo them in case we are forcibly unmounted.
+	 */
+	writerefs = atomic_fetchadd_int(&vp->v_writecount, ap->a_inc);
 	/* text refs are bypassed to lowervp */
-	VNASSERT(vp->v_writecount >= 0, vp, ("wrong null writecount"));
-	VNASSERT(vp->v_writecount + ap->a_inc >= 0, vp,
-	    ("wrong writecount inc %d", ap->a_inc));
-	if (tvp != NULL)
-		error = VOP_ADD_WRITECOUNT(tvp, ap->a_inc);
-	else if (vp->v_writecount < 0)
-		error = ETXTBSY;
-	else
-		error = 0;
-	if (error == 0)
-		vp->v_writecount += ap->a_inc;
-	VI_UNLOCK(vp);
-	return (error);
+	VNASSERT(writerefs >= 0, vp,
+	    ("%s: invalid write count %d", __func__, writerefs));
+	VNASSERT(writerefs + ap->a_inc >= 0, vp,
+	    ("%s: invalid write count inc %d + %d", __func__,
+	    writerefs, ap->a_inc));
+	return (0);
 }
 
 static int