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);