From owner-dev-commits-src-main@freebsd.org Tue Jun 29 13:01:03 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 68C5364BD50; Tue, 29 Jun 2021 13:01:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GDl332RtGz4pHH; Tue, 29 Jun 2021 13:01:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 3C5F01963F; Tue, 29 Jun 2021 13:01:03 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 15TD13Sw015953; Tue, 29 Jun 2021 13:01:03 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 15TD13Bn015952; Tue, 29 Jun 2021 13:01:03 GMT (envelope-from git) Date: Tue, 29 Jun 2021 13:01:03 GMT Message-Id: <202106291301.15TD13Bn015952@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Jason A. Harmening" Subject: git: 372691a7ae18 - main - unionfs: release parent vnodes in deferred context MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: jah X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 372691a7ae1878ecdf707195b0854750f07bf44e Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 29 Jun 2021 13:01:03 -0000 The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=372691a7ae1878ecdf707195b0854750f07bf44e commit 372691a7ae1878ecdf707195b0854750f07bf44e Author: Jason A. Harmening AuthorDate: 2021-06-12 19:45:18 +0000 Commit: Jason A. Harmening 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 #include #include +#include +#include #include #include @@ -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); } /*