From f28297d01f737f3ca830b4161abd767edf0db5a3 Mon Sep 17 00:00:00 2001 From: pjd Date: Mon, 4 Jun 2007 11:31:46 +0000 Subject: Reimplement traverse() helper function: 1. Pass locking flags to VFS_ROOT(). 2. Check v_mountedhere while the vnode is locked. 3. Always return locked vnode on success. Change 1 fixes problem reported by Stephen M. Rumble - after zfs_vfsops.c,1.9 change, zfs_root() no longer locks the vnode unconditionally and traverse() didn't pass right lock type to VFS_ROOT(). The result was that kernel paniced when .zfs/ directory was accessed via NFS. --- sys/cddl/compat/opensolaris/kern/opensolaris_vfs.c | 22 +++++++++++++--------- sys/cddl/compat/opensolaris/sys/vfs.h | 2 +- .../opensolaris/uts/common/fs/zfs/zfs_ctldir.c | 19 +++++++++---------- sys/compat/opensolaris/kern/opensolaris_vfs.c | 22 +++++++++++++--------- sys/compat/opensolaris/sys/vfs.h | 2 +- .../opensolaris/uts/common/fs/zfs/zfs_ctldir.c | 19 +++++++++---------- 6 files changed, 46 insertions(+), 40 deletions(-) (limited to 'sys') diff --git a/sys/cddl/compat/opensolaris/kern/opensolaris_vfs.c b/sys/cddl/compat/opensolaris/kern/opensolaris_vfs.c index e3325ae..79a4c5b 100644 --- a/sys/cddl/compat/opensolaris/kern/opensolaris_vfs.c +++ b/sys/cddl/compat/opensolaris/kern/opensolaris_vfs.c @@ -110,7 +110,7 @@ vfs_optionisset(const vfs_t *vfsp, const char *opt, char **argp) } int -traverse(vnode_t **cvpp) +traverse(vnode_t **cvpp, int lktype) { kthread_t *td = curthread; vnode_t *cvp; @@ -119,7 +119,7 @@ traverse(vnode_t **cvpp) int error; cvp = *cvpp; - error = 0; + tvp = NULL; /* * If this vnode is mounted on, then we transparently indirect @@ -135,22 +135,26 @@ traverse(vnode_t **cvpp) vfsp = vn_mountedvfs(cvp); if (vfsp == NULL) break; - VN_RELE(cvp); + /* + * tvp is NULL for *cvpp vnode, which we can't unlock. + */ + if (tvp != NULL) + vput(cvp); + else + vrele(cvp); /* * The read lock must be held across the call to VFS_ROOT() to * prevent a concurrent unmount from destroying the vfs. */ - error = VFS_ROOT(vfsp, 0, &tvp, td); - if (error) - break; - VOP_UNLOCK(tvp, 0, td); - + error = VFS_ROOT(vfsp, lktype, &tvp, td); + if (error != 0) + return (error); cvp = tvp; } *cvpp = cvp; - return (error); + return (0); } int diff --git a/sys/cddl/compat/opensolaris/sys/vfs.h b/sys/cddl/compat/opensolaris/sys/vfs.h index 4a018ff..c2d8a6b 100644 --- a/sys/cddl/compat/opensolaris/sys/vfs.h +++ b/sys/cddl/compat/opensolaris/sys/vfs.h @@ -107,7 +107,7 @@ void vfs_setmntopt(vfs_t *vfsp, const char *name, const char *arg, int flags __unused); void vfs_clearmntopt(vfs_t *vfsp, const char *name); int vfs_optionisset(const vfs_t *vfsp, const char *opt, char **argp); -int traverse(vnode_t **cvpp); +int traverse(vnode_t **cvpp, int lktype); int domount(kthread_t *td, vnode_t *vp, const char *fstype, char *fspath, char *fspec, int fsflags); diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c index 7a50389..0624fc2 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c @@ -924,18 +924,14 @@ zfsctl_snapshot_inactive(ap) } static int -zfsctl_traverse_begin(vnode_t **vpp, kthread_t *td) +zfsctl_traverse_begin(vnode_t **vpp, int lktype, kthread_t *td) { - int err; VN_HOLD(*vpp); /* Snapshot should be already mounted, but just in case. */ if (vn_mountedvfs(*vpp) == NULL) return (ENOENT); - err = traverse(vpp); - if (err == 0) - vn_lock(*vpp, LK_SHARED | LK_RETRY, td); - return (err); + return (traverse(vpp, lktype)); } static void @@ -960,7 +956,7 @@ zfsctl_snapshot_getattr(ap) vnode_t *vp = ap->a_vp; int err; - err = zfsctl_traverse_begin(&vp, ap->a_td); + err = zfsctl_traverse_begin(&vp, LK_SHARED | LK_RETRY, ap->a_td); if (err == 0) err = VOP_GETATTR(vp, ap->a_vap, ap->a_cred, ap->a_td); zfsctl_traverse_end(vp, err); @@ -977,7 +973,7 @@ zfsctl_snapshot_fid(ap) vnode_t *vp = ap->a_vp; int err; - err = zfsctl_traverse_begin(&vp, curthread); + err = zfsctl_traverse_begin(&vp, LK_SHARED | LK_RETRY, curthread); if (err == 0) err = VOP_VPTOFH(vp, (void *)ap->a_fid); zfsctl_traverse_end(vp, err); @@ -1026,7 +1022,7 @@ zfsctl_lookup_objset(vfs_t *vfsp, uint64_t objsetid, zfsvfs_t **zfsvfsp) if (sep != NULL) { VN_HOLD(vp); - error = traverse(&vp); + error = traverse(&vp, LK_SHARED | LK_RETRY); if (error == 0) { if (vp == sep->se_root) error = EINVAL; @@ -1034,7 +1030,10 @@ zfsctl_lookup_objset(vfs_t *vfsp, uint64_t objsetid, zfsvfs_t **zfsvfsp) *zfsvfsp = VTOZ(vp)->z_zfsvfs; } mutex_exit(&sdp->sd_lock); - VN_RELE(vp); + if (error == 0) + VN_URELE(vp); + else + VN_RELE(vp); } else { error = EINVAL; mutex_exit(&sdp->sd_lock); diff --git a/sys/compat/opensolaris/kern/opensolaris_vfs.c b/sys/compat/opensolaris/kern/opensolaris_vfs.c index e3325ae..79a4c5b 100644 --- a/sys/compat/opensolaris/kern/opensolaris_vfs.c +++ b/sys/compat/opensolaris/kern/opensolaris_vfs.c @@ -110,7 +110,7 @@ vfs_optionisset(const vfs_t *vfsp, const char *opt, char **argp) } int -traverse(vnode_t **cvpp) +traverse(vnode_t **cvpp, int lktype) { kthread_t *td = curthread; vnode_t *cvp; @@ -119,7 +119,7 @@ traverse(vnode_t **cvpp) int error; cvp = *cvpp; - error = 0; + tvp = NULL; /* * If this vnode is mounted on, then we transparently indirect @@ -135,22 +135,26 @@ traverse(vnode_t **cvpp) vfsp = vn_mountedvfs(cvp); if (vfsp == NULL) break; - VN_RELE(cvp); + /* + * tvp is NULL for *cvpp vnode, which we can't unlock. + */ + if (tvp != NULL) + vput(cvp); + else + vrele(cvp); /* * The read lock must be held across the call to VFS_ROOT() to * prevent a concurrent unmount from destroying the vfs. */ - error = VFS_ROOT(vfsp, 0, &tvp, td); - if (error) - break; - VOP_UNLOCK(tvp, 0, td); - + error = VFS_ROOT(vfsp, lktype, &tvp, td); + if (error != 0) + return (error); cvp = tvp; } *cvpp = cvp; - return (error); + return (0); } int diff --git a/sys/compat/opensolaris/sys/vfs.h b/sys/compat/opensolaris/sys/vfs.h index 4a018ff..c2d8a6b 100644 --- a/sys/compat/opensolaris/sys/vfs.h +++ b/sys/compat/opensolaris/sys/vfs.h @@ -107,7 +107,7 @@ void vfs_setmntopt(vfs_t *vfsp, const char *name, const char *arg, int flags __unused); void vfs_clearmntopt(vfs_t *vfsp, const char *name); int vfs_optionisset(const vfs_t *vfsp, const char *opt, char **argp); -int traverse(vnode_t **cvpp); +int traverse(vnode_t **cvpp, int lktype); int domount(kthread_t *td, vnode_t *vp, const char *fstype, char *fspath, char *fspec, int fsflags); diff --git a/sys/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c b/sys/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c index 7a50389..0624fc2 100644 --- a/sys/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c +++ b/sys/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c @@ -924,18 +924,14 @@ zfsctl_snapshot_inactive(ap) } static int -zfsctl_traverse_begin(vnode_t **vpp, kthread_t *td) +zfsctl_traverse_begin(vnode_t **vpp, int lktype, kthread_t *td) { - int err; VN_HOLD(*vpp); /* Snapshot should be already mounted, but just in case. */ if (vn_mountedvfs(*vpp) == NULL) return (ENOENT); - err = traverse(vpp); - if (err == 0) - vn_lock(*vpp, LK_SHARED | LK_RETRY, td); - return (err); + return (traverse(vpp, lktype)); } static void @@ -960,7 +956,7 @@ zfsctl_snapshot_getattr(ap) vnode_t *vp = ap->a_vp; int err; - err = zfsctl_traverse_begin(&vp, ap->a_td); + err = zfsctl_traverse_begin(&vp, LK_SHARED | LK_RETRY, ap->a_td); if (err == 0) err = VOP_GETATTR(vp, ap->a_vap, ap->a_cred, ap->a_td); zfsctl_traverse_end(vp, err); @@ -977,7 +973,7 @@ zfsctl_snapshot_fid(ap) vnode_t *vp = ap->a_vp; int err; - err = zfsctl_traverse_begin(&vp, curthread); + err = zfsctl_traverse_begin(&vp, LK_SHARED | LK_RETRY, curthread); if (err == 0) err = VOP_VPTOFH(vp, (void *)ap->a_fid); zfsctl_traverse_end(vp, err); @@ -1026,7 +1022,7 @@ zfsctl_lookup_objset(vfs_t *vfsp, uint64_t objsetid, zfsvfs_t **zfsvfsp) if (sep != NULL) { VN_HOLD(vp); - error = traverse(&vp); + error = traverse(&vp, LK_SHARED | LK_RETRY); if (error == 0) { if (vp == sep->se_root) error = EINVAL; @@ -1034,7 +1030,10 @@ zfsctl_lookup_objset(vfs_t *vfsp, uint64_t objsetid, zfsvfs_t **zfsvfsp) *zfsvfsp = VTOZ(vp)->z_zfsvfs; } mutex_exit(&sdp->sd_lock); - VN_RELE(vp); + if (error == 0) + VN_URELE(vp); + else + VN_RELE(vp); } else { error = EINVAL; mutex_exit(&sdp->sd_lock); -- cgit v1.1