git: 372691a7ae18 - main - unionfs: release parent vnodes in deferred context

[Jason A. Harmening]

The branch main has been updated by jah:

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

commit 372691a7ae1878ecdf707195b0854750f07bf44e
Author:     Jason A. Harmening <jah at FreeBSD.org>
AuthorDate: 2021-06-12 19:45:18 +0000
Commit:     Jason A. Harmening <jah at FreeBSD.org>
CommitDate: 2021-06-29 13:02:01 +0000

    unionfs: release parent vnodes in deferred context
    
    Each unionfs node holds a reference to its parent directory vnode.
    A single open file reference can therefore end up keeping an
    arbitrarily deep vnode hierarchy in place.  When that reference is
    released, the resulting VOP_RECLAIM call chain can then exhaust the
    kernel stack.
    
    This is easily reproducible by running the unionfs.sh stress2 test.
    Fix it by deferring recursive unionfs vnode release to taskqueue
    context.
    
    PR: 238883
    Reviewed By:    kib (earlier version), markj
    Differential Revision: https://reviews.freebsd.org/D30748
---
 sys/fs/unionfs/union.h      |  6 ++++-
 sys/fs/unionfs/union_subr.c | 55 ++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 55 insertions(+), 6 deletions(-)

diff --git a/sys/fs/unionfs/union.h b/sys/fs/unionfs/union.h
index ba0318bf185c..64706b2b21a2 100644
--- a/sys/fs/unionfs/union.h
+++ b/sys/fs/unionfs/union.h
@@ -89,7 +89,11 @@ struct unionfs_node {
 						/* unionfs status head */
 	LIST_HEAD(unionfs_node_hashhead, unionfs_node) *un_hashtbl;
 						/* dir vnode hash table */
-	LIST_ENTRY(unionfs_node)   un_hash;	/* hash list entry */
+	union {
+		LIST_ENTRY(unionfs_node) un_hash; /* hash list entry */
+		STAILQ_ENTRY(unionfs_node) un_rele; /* deferred release list */
+	};
+
 	u_long		un_hashmask;		/* bit mask */
 	char           *un_path;		/* path */
 	int		un_flag;		/* unionfs node flag */
diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index 93e4f50d98c5..04d00fd77e39 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -53,6 +53,8 @@
 #include <sys/fcntl.h>
 #include <sys/filedesc.h>
 #include <sys/stat.h>
+#include <sys/sysctl.h>
+#include <sys/taskqueue.h>
 #include <sys/resourcevar.h>
 
 #include <security/mac/mac_framework.h>
@@ -67,6 +69,18 @@ static MALLOC_DEFINE(M_UNIONFSHASH, "UNIONFS hash", "UNIONFS hash table");
 MALLOC_DEFINE(M_UNIONFSNODE, "UNIONFS node", "UNIONFS vnode private part");
 MALLOC_DEFINE(M_UNIONFSPATH, "UNIONFS path", "UNIONFS path private part");
 
+static struct task unionfs_deferred_rele_task;
+static struct mtx unionfs_deferred_rele_lock;
+static STAILQ_HEAD(, unionfs_node) unionfs_deferred_rele_list =
+    STAILQ_HEAD_INITIALIZER(unionfs_deferred_rele_list);
+static TASKQUEUE_DEFINE_THREAD(unionfs_rele);
+
+unsigned int unionfs_ndeferred = 0;
+SYSCTL_UINT(_vfs, OID_AUTO, unionfs_ndeferred, CTLFLAG_RD,
+    &unionfs_ndeferred, 0, "unionfs deferred vnode release");
+
+static void unionfs_deferred_rele(void *, int);
+
 /*
  * Initialize
  */
@@ -74,6 +88,8 @@ int
 unionfs_init(struct vfsconf *vfsp)
 {
 	UNIONFSDEBUG("unionfs_init\n");	/* printed during system boot */
+	TASK_INIT(&unionfs_deferred_rele_task, 0, unionfs_deferred_rele, NULL);
+	mtx_init(&unionfs_deferred_rele_lock, "uniondefr", NULL, MTX_DEF); 
 	return (0);
 }
 
@@ -83,9 +99,35 @@ unionfs_init(struct vfsconf *vfsp)
 int 
 unionfs_uninit(struct vfsconf *vfsp)
 {
+	taskqueue_quiesce(taskqueue_unionfs_rele);
+	taskqueue_free(taskqueue_unionfs_rele);
+	mtx_destroy(&unionfs_deferred_rele_lock);
 	return (0);
 }
 
+static void
+unionfs_deferred_rele(void *arg __unused, int pending __unused)
+{
+	STAILQ_HEAD(, unionfs_node) local_rele_list;
+	struct unionfs_node *unp, *tunp;
+	unsigned int ndeferred;
+
+	ndeferred = 0;
+	STAILQ_INIT(&local_rele_list);
+	mtx_lock(&unionfs_deferred_rele_lock);
+	STAILQ_CONCAT(&local_rele_list, &unionfs_deferred_rele_list);
+	mtx_unlock(&unionfs_deferred_rele_lock);
+	STAILQ_FOREACH_SAFE(unp, &local_rele_list, un_rele, tunp) {
+		++ndeferred;
+		MPASS(unp->un_dvp != NULL);
+		vrele(unp->un_dvp);
+		free(unp, M_UNIONFSNODE);
+	}
+
+	/* We expect this function to be single-threaded, thus no atomic */
+	unionfs_ndeferred += ndeferred;
+}
+
 static struct unionfs_node_hashhead *
 unionfs_get_hashhead(struct vnode *dvp, char *path)
 {
@@ -375,10 +417,6 @@ unionfs_noderem(struct vnode *vp, struct thread *td)
 		vrele(lvp);
 	if (uvp != NULLVP)
 		vrele(uvp);
-	if (dvp != NULLVP) {
-		vrele(dvp);
-		unp->un_dvp = NULLVP;
-	}
 	if (unp->un_path != NULL) {
 		free(unp->un_path, M_UNIONFSPATH);
 		unp->un_path = NULL;
@@ -400,7 +438,14 @@ unionfs_noderem(struct vnode *vp, struct thread *td)
 		LIST_REMOVE(unsp, uns_list);
 		free(unsp, M_TEMP);
 	}
-	free(unp, M_UNIONFSNODE);
+	if (dvp != NULLVP) {
+		mtx_lock(&unionfs_deferred_rele_lock);
+		STAILQ_INSERT_TAIL(&unionfs_deferred_rele_list, unp, un_rele);
+		mtx_unlock(&unionfs_deferred_rele_lock);
+		taskqueue_enqueue(taskqueue_unionfs_rele,
+		    &unionfs_deferred_rele_task);
+	} else
+		free(unp, M_UNIONFSNODE);
 }
 
 /*