git: 53a777bb52e3 - main - unionfs: do not create a new status object during vop_close()

From: Jason A. Harmening <jah_at_FreeBSD.org>
Date: Sat, 13 Jul 2024 01:39:51 UTC
The branch main has been updated by jah:

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

commit 53a777bb52e3f1e8a29fa0cbea9b991daf7ba01e
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2024-06-14 21:01:22 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2024-07-12 22:05:16 +0000

    unionfs: do not create a new status object during vop_close()
    
    Split the portion of unionfs_get_node_status() that searches for an
    existing status object into a new helper function,
    unionfs_find_node_status(), and use that in unionfs_close().
    
    Additionally, modify unionfs_close() to accept a NULL status object
    if unionfs_find_node_status() does not find a matching status
    object.  This can happen due to the unconditional VOP_CLOSE()
    operation issued by vgonel().
    
    Differential Revision:  https://reviews.freebsd.org/D45398
    Reviewed by:            olce
    Tested by:              pho
---
 sys/fs/unionfs/union.h       |  2 ++
 sys/fs/unionfs/union_subr.c  | 45 ++++++++++++++++++++++++++++++--------------
 sys/fs/unionfs/union_vnops.c |  9 +++++++--
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/sys/fs/unionfs/union.h b/sys/fs/unionfs/union.h
index c535a3b288b8..0bd1894a2195 100644
--- a/sys/fs/unionfs/union.h
+++ b/sys/fs/unionfs/union.h
@@ -133,6 +133,8 @@ int	unionfs_uninit(struct vfsconf *);
 int	unionfs_nodeget(struct mount *, struct vnode *, struct vnode *,
 	    struct vnode *, struct vnode **, struct componentname *);
 void	unionfs_noderem(struct vnode *);
+struct unionfs_node_status *	unionfs_find_node_status(struct unionfs_node *,
+	    struct thread *td);
 void	unionfs_get_node_status(struct unionfs_node *, struct thread *,
 	    struct unionfs_node_status **);
 void	unionfs_tryrem_node_status(struct unionfs_node *,
diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index 671322704dc5..e02dd547f249 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -578,35 +578,52 @@ unionfs_noderem(struct vnode *vp)
 }
 
 /*
- * Get the unionfs node status object for the vnode corresponding to unp,
- * for the process that owns td.  Allocate a new status object if one
- * does not already exist.
+ * Find the unionfs node status object for the vnode corresponding to unp,
+ * for the process that owns td.  Return NULL if no such object exists.
  */
-void
-unionfs_get_node_status(struct unionfs_node *unp, struct thread *td,
-    struct unionfs_node_status **unspp)
+struct unionfs_node_status *
+unionfs_find_node_status(struct unionfs_node *unp, struct thread *td)
 {
 	struct unionfs_node_status *unsp;
 	pid_t pid;
 
 	pid = td->td_proc->p_pid;
 
-	KASSERT(NULL != unspp, ("%s: NULL status", __func__));
 	ASSERT_VOP_ELOCKED(UNIONFSTOV(unp), __func__);
 
 	LIST_FOREACH(unsp, &(unp->un_unshead), uns_list) {
 		if (unsp->uns_pid == pid) {
-			*unspp = unsp;
-			return;
+			return (unsp);
 		}
 	}
 
-	/* create a new unionfs node status */
-	unsp = malloc(sizeof(struct unionfs_node_status),
-	    M_TEMP, M_WAITOK | M_ZERO);
+	return (NULL);
+}
+
+/*
+ * Get the unionfs node status object for the vnode corresponding to unp,
+ * for the process that owns td.  Allocate a new status object if one
+ * does not already exist.
+ */
+void
+unionfs_get_node_status(struct unionfs_node *unp, struct thread *td,
+    struct unionfs_node_status **unspp)
+{
+	struct unionfs_node_status *unsp;
+	pid_t pid;
+
+	pid = td->td_proc->p_pid;
 
-	unsp->uns_pid = pid;
-	LIST_INSERT_HEAD(&(unp->un_unshead), unsp, uns_list);
+	KASSERT(NULL != unspp, ("%s: NULL status", __func__));
+	unsp = unionfs_find_node_status(unp, td);
+	if (unsp == NULL) {
+		/* create a new unionfs node status */
+		unsp = malloc(sizeof(struct unionfs_node_status),
+		    M_TEMP, M_WAITOK | M_ZERO);
+
+		unsp->uns_pid = pid;
+		LIST_INSERT_HEAD(&(unp->un_unshead), unsp, uns_list);
+	}
 
 	*unspp = unsp;
 }
diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c
index ae1d3946266d..3f39352ea5c0 100644
--- a/sys/fs/unionfs/union_vnops.c
+++ b/sys/fs/unionfs/union_vnops.c
@@ -793,9 +793,14 @@ unionfs_close(struct vop_close_args *ap)
 	unp = VTOUNIONFS(vp);
 	lvp = unp->un_lowervp;
 	uvp = unp->un_uppervp;
-	unionfs_get_node_status(unp, td, &unsp);
+	unsp = unionfs_find_node_status(unp, td);
 
-	if (unsp->uns_lower_opencnt <= 0 && unsp->uns_upper_opencnt <= 0) {
+	if (unsp == NULL ||
+	    (unsp->uns_lower_opencnt <= 0 && unsp->uns_upper_opencnt <= 0)) {
+#ifdef DIAGNOSTIC
+		if (unsp != NULL)
+			printf("unionfs_close: warning: open count is 0\n");
+#endif
 		if (uvp != NULLVP)
 			ovp = uvp;
 		else