From nobody Sat Nov 06 14:02:17 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 85EB21843F3F; Sat, 6 Nov 2021 14:02:17 +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 4HmfFj3H8lz3Khb; Sat, 6 Nov 2021 14:02:17 +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 51F05499D; Sat, 6 Nov 2021 14:02:17 +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 1A6E2HFk023179; Sat, 6 Nov 2021 14:02:17 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1A6E2HbB023178; Sat, 6 Nov 2021 14:02:17 GMT (envelope-from git) Date: Sat, 6 Nov 2021 14:02:17 GMT Message-Id: <202111061402.1A6E2HbB023178@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: 866dd6335a6c - main - unionfs: various locking fixes List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org 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: 866dd6335a6c74cc7615a8ad8ae2f2b2ec4ece94 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=866dd6335a6c74cc7615a8ad8ae2f2b2ec4ece94 commit 866dd6335a6c74cc7615a8ad8ae2f2b2ec4ece94 Author: Jason A. Harmening AuthorDate: 2021-10-24 17:24:18 +0000 Commit: Jason A. Harmening CommitDate: 2021-11-06 14:08:33 +0000 unionfs: various locking fixes --Clearing cached subdirectories in unionfs_noderem() should be done under the vnode interlock --When preparing to switch the vnode lock in both unionfs_node_update() and unionfs_noderem(), the incoming lock should be acquired before updating the v_vnlock field to point to it. Otherwise we effectively break the locking contract for a brief window. Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D32629 --- sys/fs/unionfs/union_subr.c | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c index 326483bcc400..7bde71bb17b1 100644 --- a/sys/fs/unionfs/union_subr.c +++ b/sys/fs/unionfs/union_subr.c @@ -439,6 +439,9 @@ unionfs_noderem(struct vnode *vp, struct thread *td) struct vnode *dvp; int count; + if (lockmgr(&(vp->v_lock), LK_EXCLUSIVE, NULL) != 0) + panic("the lock for deletion is unacquirable."); + /* * Use the interlock to protect the clearing of v_data to * prevent faults in unionfs_lock(). @@ -459,6 +462,22 @@ unionfs_noderem(struct vnode *vp, struct thread *td) 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 + * be necessary during forced unmount, when the vnode may + * be reclaimed with a non-zero use count. Otherwise the + * reference held by each child should prevent reclamation. + */ + for (count = 0; count <= UNIONFSHASHMASK; count++) { + hd = unp->un_hashtbl + count; + LIST_FOREACH_SAFE(unp_t1, hd, un_hash, unp_t2) { + LIST_REMOVE(unp_t1, un_hash); + unp_t1->un_hash.le_next = NULL; + unp_t1->un_hash.le_prev = NULL; + } + } + } VI_UNLOCK(vp); if (lvp != NULLVP) @@ -469,9 +488,6 @@ unionfs_noderem(struct vnode *vp, struct thread *td) if (dvp != NULLVP) unionfs_rem_cached_vnode(unp, dvp); - if (lockmgr(vp->v_vnlock, LK_EXCLUSIVE, VI_MTX(vp)) != 0) - panic("the lock for deletion is unacquirable."); - if (lvp != NULLVP) vrele(lvp); if (uvp != NULLVP) @@ -483,14 +499,6 @@ unionfs_noderem(struct vnode *vp, struct thread *td) } if (unp->un_hashtbl != NULL) { - for (count = 0; count <= UNIONFSHASHMASK; count++) { - hd = unp->un_hashtbl + count; - LIST_FOREACH_SAFE(unp_t1, hd, un_hash, unp_t2) { - LIST_REMOVE(unp_t1, un_hash); - unp_t1->un_hash.le_next = NULL; - unp_t1->un_hash.le_prev = NULL; - } - } hashdestroy(unp->un_hashtbl, M_UNIONFSHASH, UNIONFSHASHMASK); } @@ -798,15 +806,16 @@ unionfs_node_update(struct unionfs_node *unp, struct vnode *uvp, dvp = unp->un_dvp; /* - * lock update + * Uppdate the upper vnode's lock state to match the lower vnode, + * and then switch the unionfs vnode's lock to the upper vnode. */ + lockrec = lvp->v_vnlock->lk_recurse; + for (count = 0; count < lockrec; count++) + vn_lock(uvp, LK_EXCLUSIVE | LK_CANRECURSE | LK_RETRY); VI_LOCK(vp); unp->un_uppervp = uvp; vp->v_vnlock = uvp->v_vnlock; VI_UNLOCK(vp); - lockrec = lvp->v_vnlock->lk_recurse; - for (count = 0; count < lockrec; count++) - vn_lock(uvp, LK_EXCLUSIVE | LK_CANRECURSE | LK_RETRY); /* * Re-cache the unionfs vnode against the upper vnode