Author: avg Date: Sat Apr 2 16:25:46 2016 New Revision: 297513 URL: https://svnweb.freebsd.org/changeset/base/297513
Log: remove emulation of VFS_HOLD and VFS_RELE from opensolaris compat On FreeBSD VFS_HOLD/VN_RELE were mapped to MNT_REF/MNT_REL that manipulate mnt_ref. But the job of properly maintaining the reference count is already automatically performed by insmntque(9) and delmntque(9). So, in effect all ZFS vnodes referenced the corresponding mountpoint twice. That was completely harmless, but we want to be very explicit about what FreeBSD VFS APIs are used, because illumos VFS_HOLD and FreeBSD MNT_REF provide quite different guarantees with respect to the held vfs_t / mountpoint. On illumos VFS_HOLD is sufficient to guarantee that vfs_t.vfs_data stays valid. On the other hand, on FreeBSD MNT_REF does *not* provide the same guarantee about mnt_data. We have to use vfs_busy() to get that guarantee. Thus, the calls to VFS_HOLD/VFS_RELE on vnode init and fini are removed. VFS_HOLD calls are replaced with vfs_busy in the ioctl handlers. And because vfs_busy has a richer interface that can not be dumbed down in all cases it's better to explicitly use it rather than trying to mask it behind VFS_HOLD. This change fixes a panic that could result from a race between zfs_umount() and zfs_ioc_rollback(). We observed a case where zfsvfs_free() tried to destroy data that zfsvfs_teardown() was still using. That happened because there was nothing to prevent unmounting of a ZFS filesystem that was in between zfs_suspend_fs() and zfs_resume_fs(). Reviewed by: kib, smh MFC after: 3 weeks Sponsored by: ClusterHQ Differential Revision: https://reviews.freebsd.org/D2794 Modified: head/sys/cddl/compat/opensolaris/sys/vfs.h head/sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c Modified: head/sys/cddl/compat/opensolaris/sys/vfs.h ============================================================================== --- head/sys/cddl/compat/opensolaris/sys/vfs.h Sat Apr 2 13:51:06 2016 (r297512) +++ head/sys/cddl/compat/opensolaris/sys/vfs.h Sat Apr 2 16:25:46 2016 (r297513) @@ -54,17 +54,6 @@ typedef struct mount vfs_t; #define VFS_NOSETUID MNT_NOSUID #define VFS_NOEXEC MNT_NOEXEC -#define VFS_HOLD(vfsp) do { \ - MNT_ILOCK(vfsp); \ - MNT_REF(vfsp); \ - MNT_IUNLOCK(vfsp); \ -} while (0) -#define VFS_RELE(vfsp) do { \ - MNT_ILOCK(vfsp); \ - MNT_REL(vfsp); \ - MNT_IUNLOCK(vfsp); \ -} while (0) - #define fs_vscan(vp, cr, async) (0) #define VROOT VV_ROOT Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c Sat Apr 2 13:51:06 2016 (r297512) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c Sat Apr 2 16:25:46 2016 (r297513) @@ -589,7 +589,9 @@ gfs_root_create(size_t size, vfs_t *vfsp { vnode_t *vp; +#ifdef illumos VFS_HOLD(vfsp); +#endif vp = gfs_dir_create(size, NULL, vfsp, ops, entries, inode_cb, maxlen, readdir_cb, lookup_cb); /* Manually set the inode */ @@ -700,7 +702,9 @@ found: vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); } else { ASSERT(vp->v_vfsp != NULL); +#ifdef illumos VFS_RELE(vp->v_vfsp); +#endif } #ifdef TODO if (vp->v_flag & V_XATTRDIR) Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Sat Apr 2 13:51:06 2016 (r297512) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Sat Apr 2 16:25:46 2016 (r297513) @@ -1443,7 +1443,14 @@ getzfsvfs(const char *dsname, zfsvfs_t * mutex_enter(&os->os_user_ptr_lock); *zfvp = dmu_objset_get_user(os); if (*zfvp) { +#ifdef illumos VFS_HOLD((*zfvp)->z_vfs); +#else + if (vfs_busy((*zfvp)->z_vfs, 0) != 0) { + *zfvp = NULL; + error = SET_ERROR(ESRCH); + } +#endif } else { error = SET_ERROR(ESRCH); } @@ -1487,7 +1494,11 @@ zfsvfs_rele(zfsvfs_t *zfsvfs, void *tag) rrm_exit(&zfsvfs->z_teardown_lock, tag); if (zfsvfs->z_vfs) { +#ifdef illumos VFS_RELE(zfsvfs->z_vfs); +#else + vfs_unbusy(zfsvfs->z_vfs); +#endif } else { dmu_objset_disown(zfsvfs->z_os, zfsvfs); zfsvfs_free(zfsvfs); @@ -3018,11 +3029,13 @@ zfs_get_vfs(const char *resource) mtx_lock(&mountlist_mtx); TAILQ_FOREACH(vfsp, &mountlist, mnt_list) { if (strcmp(refstr_value(vfsp->vfs_resource), resource) == 0) { - VFS_HOLD(vfsp); + if (vfs_busy(vfsp, MBF_MNTLSTLOCK) != 0) + vfsp = NULL; break; } } - mtx_unlock(&mountlist_mtx); + if (vfsp == NULL) + mtx_unlock(&mountlist_mtx); return (vfsp); } @@ -3475,7 +3488,11 @@ zfs_unmount_snap(const char *snapname) ASSERT(!dsl_pool_config_held(dmu_objset_pool(zfsvfs->z_os))); err = vn_vfswlock(vfsp->vfs_vnodecovered); +#ifdef illumos VFS_RELE(vfsp); +#else + vfs_unbusy(vfsp); +#endif if (err != 0) return (SET_ERROR(err)); @@ -3721,7 +3738,11 @@ zfs_ioc_rollback(const char *fsname, nvl resume_err = zfs_resume_fs(zfsvfs, fsname); error = error ? error : resume_err; } +#ifdef illumos VFS_RELE(zfsvfs->z_vfs); +#else + vfs_unbusy(zfsvfs->z_vfs); +#endif } else { error = dsl_dataset_rollback(fsname, NULL, outnvl); } @@ -4376,7 +4397,11 @@ zfs_ioc_recv(zfs_cmd_t *zc) if (error == 0) error = zfs_resume_fs(zfsvfs, tofs); error = error ? error : end_err; +#ifdef illumos VFS_RELE(zfsvfs->z_vfs); +#else + vfs_unbusy(zfsvfs->z_vfs); +#endif } else { error = dmu_recv_end(&drc, NULL); } @@ -4925,7 +4950,11 @@ zfs_ioc_userspace_upgrade(zfs_cmd_t *zc) } if (error == 0) error = dmu_objset_userspace_upgrade(zfsvfs->z_os); +#ifdef illumos VFS_RELE(zfsvfs->z_vfs); +#else + vfs_unbusy(zfsvfs->z_vfs); +#endif } else { /* XXX kind of reading contents without owning */ error = dmu_objset_hold(zc->zc_name, FTAG, &os); Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c Sat Apr 2 13:51:06 2016 (r297512) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c Sat Apr 2 16:25:46 2016 (r297513) @@ -743,7 +743,9 @@ zfs_znode_alloc(zfsvfs_t *zfsvfs, dmu_bu if (vp->v_type != VFIFO) VN_LOCK_ASHARE(vp); +#ifdef illumos VFS_HOLD(zfsvfs->z_vfs); +#endif return (zp); } @@ -1428,7 +1430,9 @@ zfs_znode_free(znode_t *zp) kmem_cache_free(znode_cache, zp); +#ifdef illumos VFS_RELE(zfsvfs->z_vfs); +#endif } void _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
