d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

remove emulation of VFS_HOLD and VFS_RELE from opensolaris compat

Browse Source 
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
This commit is contained in:
avg 2016-04-02 16:25:46 +00:00
parent e0e88029f1
commit 4a4b7f856b
4 changed files with 39 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
sys/cddl/compat/opensolaris/sys/vfs.h
View File

@ -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

4
sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c
View File

@ -589,7 +589,9 @@ gfs_root_create(size_t size, vfs_t *vfsp, vnodeops_t *ops, ino64_t ino,
{
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)

33
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
View File

@ -1443,7 +1443,14 @@ getzfsvfs(const char *dsname, zfsvfs_t **zfvp)
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, nvlist_t *args, nvlist_t *outnvl)
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);

4
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c
View File

@ -743,7 +743,9 @@ zfs_znode_alloc(zfsvfs_t *zfsvfs, dmu_buf_t *db, int blksz,
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