diff options
author | Jason A. Harmening <jah@FreeBSD.org> | 2023-12-22 20:13:35 +0000 |
---|---|---|
committer | Jason A. Harmening <jah@FreeBSD.org> | 2024-02-18 15:14:05 +0000 |
commit | cc3ec9f7597882d36ee487fd436d1b90bed0ebfd (patch) | |
tree | f2addfddf7d34cf53a1ca9ab771c3cf06ad62457 | |
parent | 5f7312a0d70c607afa9ce24ccd757321d043e02c (diff) | |
download | src-cc3ec9f7597882d36ee487fd436d1b90bed0ebfd.tar.gz src-cc3ec9f7597882d36ee487fd436d1b90bed0ebfd.zip |
unionfs: cache upper/lower mount objects
Store the upper/lower FS mount objects in unionfs per-mount data and
use these instead of the v_mount field of the upper/lower root
vnodes. As described in the referenced PR, it is unsafe to access this
field on the unionfs unmount path as ZFS rollback may have obliterated
the v_mount field of the upper or lower root vnode. Use these stored
objects to slightly simplify other code that needs access to the
upper/lower mount objects as well.
PR: 275870
Reported by: Karlo Miličević <karlo98.m@gmail.com>
Tested by: Karlo Miličević <karlo98.m@gmail.com>
Reviewed by: kib (prior version), olce
MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D43815
-rw-r--r-- | sys/fs/unionfs/union.h | 2 | ||||
-rw-r--r-- | sys/fs/unionfs/union_vfsops.c | 37 | ||||
-rw-r--r-- | sys/fs/unionfs/union_vnops.c | 4 |
3 files changed, 24 insertions, 19 deletions
diff --git a/sys/fs/unionfs/union.h b/sys/fs/unionfs/union.h index 01bc7185e5fc..0d678b4fed41 100644 --- a/sys/fs/unionfs/union.h +++ b/sys/fs/unionfs/union.h @@ -51,6 +51,8 @@ typedef enum _unionfs_whitemode { } unionfs_whitemode; struct unionfs_mount { + struct mount *um_lowermp; /* MNT_REFed lower mount object */ + struct mount *um_uppermp; /* MNT_REFed upper mount object */ struct vnode *um_lowervp; /* VREFed once */ struct vnode *um_uppervp; /* VREFed once */ struct vnode *um_rootvp; /* ROOT vnode */ diff --git a/sys/fs/unionfs/union_vfsops.c b/sys/fs/unionfs/union_vfsops.c index 6c801998fcfb..8635600549b1 100644 --- a/sys/fs/unionfs/union_vfsops.c +++ b/sys/fs/unionfs/union_vfsops.c @@ -71,7 +71,6 @@ static struct vfsops unionfs_vfsops; static int unionfs_domount(struct mount *mp) { - struct mount *lowermp, *uppermp; struct vnode *lowerrootvp; struct vnode *upperrootvp; struct unionfs_mount *ump; @@ -303,18 +302,18 @@ unionfs_domount(struct mount *mp) * reused until that happens. We assume the caller holds a reference * to lowerrootvp as it is the mount's covered vnode. */ - lowermp = vfs_register_upper_from_vp(ump->um_lowervp, mp, + ump->um_lowermp = vfs_register_upper_from_vp(ump->um_lowervp, mp, &ump->um_lower_link); - uppermp = vfs_register_upper_from_vp(ump->um_uppervp, mp, + ump->um_uppermp = vfs_register_upper_from_vp(ump->um_uppervp, mp, &ump->um_upper_link); vrele(upperrootvp); - if (lowermp == NULL || uppermp == NULL) { - if (lowermp != NULL) - vfs_unregister_upper(lowermp, &ump->um_lower_link); - if (uppermp != NULL) - vfs_unregister_upper(uppermp, &ump->um_upper_link); + if (ump->um_lowermp == NULL || ump->um_uppermp == NULL) { + if (ump->um_lowermp != NULL) + vfs_unregister_upper(ump->um_lowermp, &ump->um_lower_link); + if (ump->um_uppermp != NULL) + vfs_unregister_upper(ump->um_uppermp, &ump->um_upper_link); vflush(mp, 1, FORCECLOSE, curthread); free(ump, M_UNIONFSMNT); mp->mnt_data = NULL; @@ -346,8 +345,8 @@ unionfs_domount(struct mount *mp) } MNT_ILOCK(mp); - if ((lowermp->mnt_flag & MNT_LOCAL) != 0 && - (uppermp->mnt_flag & MNT_LOCAL) != 0) + if ((ump->um_lowermp->mnt_flag & MNT_LOCAL) != 0 && + (ump->um_uppermp->mnt_flag & MNT_LOCAL) != 0) mp->mnt_flag |= MNT_LOCAL; mp->mnt_kern_flag |= MNTK_NOMSYNC | MNTK_UNIONFS; MNT_IUNLOCK(mp); @@ -400,8 +399,8 @@ unionfs_unmount(struct mount *mp, int mntflags) vn_lock(mp->mnt_vnodecovered, LK_EXCLUSIVE | LK_RETRY | LK_CANRECURSE); mp->mnt_vnodecovered->v_vflag &= ~VV_CROSSLOCK; VOP_UNLOCK(mp->mnt_vnodecovered); - vfs_unregister_upper(ump->um_lowervp->v_mount, &ump->um_lower_link); - vfs_unregister_upper(ump->um_uppervp->v_mount, &ump->um_upper_link); + vfs_unregister_upper(ump->um_lowermp, &ump->um_lower_link); + vfs_unregister_upper(ump->um_uppermp, &ump->um_upper_link); free(ump, M_UNIONFSMNT); mp->mnt_data = NULL; @@ -439,7 +438,11 @@ unionfs_quotactl(struct mount *mp, int cmd, uid_t uid, void *arg, bool unbusy; ump = MOUNTTOUNIONFSMOUNT(mp); - uppermp = atomic_load_ptr(&ump->um_uppervp->v_mount); + /* + * Issue a volatile load of um_uppermp here, as the mount may be + * torn down after we call vfs_unbusy(). + */ + uppermp = atomic_load_ptr(&ump->um_uppermp); KASSERT(*mp_busy == true, ("upper mount not busy")); /* * See comment in sys_quotactl() for an explanation of why the @@ -479,7 +482,7 @@ unionfs_statfs(struct mount *mp, struct statfs *sbp) mstat = malloc(sizeof(struct statfs), M_STATFS, M_WAITOK | M_ZERO); - error = VFS_STATFS(ump->um_lowervp->v_mount, mstat); + error = VFS_STATFS(ump->um_lowermp, mstat); if (error) { free(mstat, M_STATFS); return (error); @@ -491,7 +494,7 @@ unionfs_statfs(struct mount *mp, struct statfs *sbp) lbsize = mstat->f_bsize; - error = VFS_STATFS(ump->um_uppervp->v_mount, mstat); + error = VFS_STATFS(ump->um_uppermp, mstat); if (error) { free(mstat, M_STATFS); return (error); @@ -558,10 +561,10 @@ unionfs_extattrctl(struct mount *mp, int cmd, struct vnode *filename_vp, unp = VTOUNIONFS(filename_vp); if (unp->un_uppervp != NULLVP) { - return (VFS_EXTATTRCTL(ump->um_uppervp->v_mount, cmd, + return (VFS_EXTATTRCTL(ump->um_uppermp, cmd, unp->un_uppervp, namespace, attrname)); } else { - return (VFS_EXTATTRCTL(ump->um_lowervp->v_mount, cmd, + return (VFS_EXTATTRCTL(ump->um_lowermp, cmd, unp->un_lowervp, namespace, attrname)); } } diff --git a/sys/fs/unionfs/union_vnops.c b/sys/fs/unionfs/union_vnops.c index 150fbffe0a4a..cb2bed925399 100644 --- a/sys/fs/unionfs/union_vnops.c +++ b/sys/fs/unionfs/union_vnops.c @@ -764,7 +764,7 @@ unionfs_access(struct vop_access_args *ap) if (lvp != NULLVP) { if (accmode & VWRITE) { - if (ump->um_uppervp->v_mount->mnt_flag & MNT_RDONLY) { + if ((ump->um_uppermp->mnt_flag & MNT_RDONLY) != 0) { switch (ap->a_vp->v_type) { case VREG: case VDIR: @@ -835,7 +835,7 @@ unionfs_getattr(struct vop_getattr_args *ap) error = VOP_GETATTR(lvp, ap->a_vap, ap->a_cred); - if (error == 0 && !(ump->um_uppervp->v_mount->mnt_flag & MNT_RDONLY)) { + if (error == 0 && (ump->um_uppermp->mnt_flag & MNT_RDONLY) == 0) { /* correct the attr toward shadow file/dir. */ if (ap->a_vp->v_type == VREG || ap->a_vp->v_type == VDIR) { unionfs_create_uppervattr_core(ump, ap->a_vap, &va, td); |