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

zfs: do not hold an extra reference on a root vnode while a filesystem is mounted

Browse Source 
At present zfs_domount() acquires a reference on the filesystem's root vnode
and that reference is kept until zfs_umount.
The latter calls vflush(rootrefs = 1) to dispose of the extra reference.

There is no explanation of why that reference is kept - what problem it
solves or what behavior it improves.
Also, that logic is FreeBSD specific.

There is one real problem with that reference, though.
zfs recv -F may receive a full, non-incremental stream to a mounted filesystem.
In that case the received root object is likely to have a different z_gen
attribute value. Because of that, zfs_rezget will leave the previous root znode
and vnode disassociated from the actual object (z_sa_hdl == NULL).
Thus, future calls to VFS_ROOT() -> zfs_root() will produce a new vnode-znode
pair, while the old one will be kept alive by the outstanding reference.
So, the outstanding reference will not actually be for the new root vnode
(or, more precisely, vnodes - because a root vnode may be recycled and a newer
one can be created).
As a result, when vflush(rootrefs = 1) s called there will be two problems:

- a leaked reference on the old root vnode preventing a graceful unmount
- insufficient references on the actual root vnode leading to a crash upon
  access to the vnode after it is destroyed by vgone() + vdrop()

The second issue will actually override the first one.

Differential Revision:	https://reviews.freebsd.org/D2353
Reviewed by:		delphij, kib, smh
MFC after:	17 days
This commit is contained in:
Andriy Gapon 2015-05-05 11:01:06 +00:00
parent 5031d03e5a
commit 24dd1a8242
2 changed files with 2 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -261,10 +261,6 @@ VTOZ(vnode_t *vp)
} \
}
/* Called on entry to each ZFS vnode and vfs operation that can not return EIO */
#define ZFS_ENTER_NOERROR(zfsvfs) \
rrm_enter(&(zfsvfs)->z_teardown_lock, RW_READER, FTAG)
/* Must be called before exiting the vop */
#define ZFS_EXIT(zfsvfs) rrm_exit(&(zfsvfs)->z_teardown_lock, FTAG)

7
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
View File

@ -1239,9 +1239,6 @@ zfs_domount(vfs_t *vfsp, char *osname)
}
vfs_mountedfrom(vfsp, osname);
/* Grab extra reference. */
VERIFY(VFS_ROOT(vfsp, LK_EXCLUSIVE, &vp) == 0);
VOP_UNLOCK(vp, 0);
if (!zfsvfs->z_issnap)
zfsctl_create(zfsvfs);
@ -1819,7 +1816,7 @@ zfs_root(vfs_t *vfsp, int flags, vnode_t **vpp)
znode_t *rootzp;
int error;
ZFS_ENTER_NOERROR(zfsvfs);
ZFS_ENTER(zfsvfs);
error = zfs_zget(zfsvfs, zfsvfs->z_root, &rootzp);
if (error == 0)
@ -1994,7 +1991,7 @@ zfs_umount(vfs_t *vfsp, int fflag)
/*
* Flush all the files.
*/
ret = vflush(vfsp, 1, (fflag & MS_FORCE) ? FORCECLOSE : 0, td);
ret = vflush(vfsp, 0, (fflag & MS_FORCE) ? FORCECLOSE : 0, td);
if (ret != 0) {
if (!zfsvfs->z_issnap) {
zfsctl_create(zfsvfs);