git: 93fe61afde72 - main - unionfs_mkdir(): handle dvp reclamation
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 18 Apr 2023 01:35:45 UTC
The branch main has been updated by jah: URL: https://cgit.FreeBSD.org/src/commit/?id=93fe61afde72e6841251ea43551631c30556032d commit 93fe61afde72e6841251ea43551631c30556032d Author: Jason A. Harmening <jah@FreeBSD.org> AuthorDate: 2023-01-16 21:50:59 +0000 Commit: Jason A. Harmening <jah@FreeBSD.org> CommitDate: 2023-04-18 01:31:40 +0000 unionfs_mkdir(): handle dvp reclamation The underlying VOP_MKDIR() implementation may temporarily drop the parent directory vnode's lock. If the vnode is reclaimed during that window, the unionfs vnode will effectively become unlocked because the its v_vnlock field will be reset. To uphold the locking requirements of VOP_MKDIR() and to avoid triggering various VFS assertions, explicitly re-lock the unionfs vnode before returning in this case. Note that there are almost certainly other cases in which we'll similarly need to handle vnode relocking by the underlying FS; this is the only one that's caused problems in stress testing so far. A more general solution, such as that employed for nullfs in null_bypass(), will likely need to be implemented. Tested by: pho Reviewed by: kib, markj Differential Revision: https://reviews.freebsd.org/D39272 --- sys/fs/unionfs/union_vnops.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c index 0da5ecb61bb2..6c8086a6c7c5 100644 --- a/sys/fs/unionfs/union_vnops.c +++ b/sys/fs/unionfs/union_vnops.c @@ -1389,6 +1389,7 @@ unionfs_mkdir(struct vop_mkdir_args *ap) { struct unionfs_node *dunp; struct componentname *cnp; + struct vnode *dvp; struct vnode *udvp; struct vnode *uvp; struct vattr va; @@ -1400,17 +1401,19 @@ unionfs_mkdir(struct vop_mkdir_args *ap) KASSERT_UNIONFS_VNODE(ap->a_dvp); error = EROFS; - dunp = VTOUNIONFS(ap->a_dvp); + dvp = ap->a_dvp; + dunp = VTOUNIONFS(dvp); cnp = ap->a_cnp; lkflags = cnp->cn_lkflags; udvp = dunp->un_uppervp; if (udvp != NULLVP) { + vref(udvp); /* check opaque */ if (!(cnp->cn_flags & ISWHITEOUT)) { error = VOP_GETATTR(udvp, &va, cnp->cn_cred); if (error != 0) - return (error); + goto unionfs_mkdir_cleanup; if ((va.va_flags & OPAQUE) != 0) cnp->cn_flags |= ISWHITEOUT; } @@ -1418,13 +1421,35 @@ unionfs_mkdir(struct vop_mkdir_args *ap) if ((error = VOP_MKDIR(udvp, &uvp, cnp, ap->a_vap)) == 0) { VOP_UNLOCK(uvp); cnp->cn_lkflags = LK_EXCLUSIVE; - error = unionfs_nodeget(ap->a_dvp->v_mount, uvp, NULLVP, - ap->a_dvp, ap->a_vpp, cnp); + /* + * The underlying VOP_MKDIR() implementation may have + * temporarily dropped the parent directory vnode lock. + * Because the unionfs vnode ordinarily shares that + * lock, this may allow the unionfs vnode to be reclaimed + * and its lock field reset. In that case, the unionfs + * vnode is effectively no longer locked, and we must + * explicitly lock it before returning in order to meet + * the locking requirements of VOP_MKDIR(). + */ + if (__predict_false(VTOUNIONFS(dvp) == NULL)) { + error = ENOENT; + goto unionfs_mkdir_cleanup; + } + error = unionfs_nodeget(dvp->v_mount, uvp, NULLVP, + dvp, ap->a_vpp, cnp); cnp->cn_lkflags = lkflags; vrele(uvp); } } +unionfs_mkdir_cleanup: + + if (__predict_false(VTOUNIONFS(dvp) == NULL)) { + vput(udvp); + vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY); + } else if (udvp != NULLVP) + vrele(udvp); + UNIONFS_INTERNAL_DEBUG("unionfs_mkdir: leave (%d)\n", error); return (error);