Date: Mon, 16 May 2016 15:13:16 +0000 (UTC) From: Andriy Gapon <avg@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r299946 - in head/sys/cddl/contrib/opensolaris/uts/common/fs: . zfs Message-ID: <201605161513.u4GFDG1U023997@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: avg Date: Mon May 16 15:13:16 2016 New Revision: 299946 URL: https://svnweb.freebsd.org/changeset/base/299946 Log: gfs_lookup_dot() does not have to acquire any locks In fact, that was dangerous. For example, zfsctl_snapshot_reclaim() calls gfs_dir_lookup() on ".." path and that ends up calling gfs_lookup_dot() which violated locking order by acquiring the parent's directory vnode lock after the child's vnode lock. Also, the previous behavior was inconsistent as gfs_dir_lookup() returned a locked vnode for . and .. lookups, but not for any other. Now gfs_lookup_dot() just references a resulting vnode and the locking is done in its consumers, where necessary. Note that we do not enable shared locking support for any gfs / zfsctl vnodes. This commit partially reverts r273641. MFC after: 5 weeks Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c Mon May 16 15:03:52 2016 (r299945) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/gfs.c Mon May 16 15:13:16 2016 (r299946) @@ -442,19 +442,9 @@ gfs_lookup_dot(vnode_t **vpp, vnode_t *d *vpp = dvp; return (0); } else if (strcmp(nm, "..") == 0) { - if (pvp == NULL) { - ASSERT(dvp->v_flag & VROOT); - VN_HOLD(dvp); - *vpp = dvp; - ASSERT_VOP_ELOCKED(dvp, "gfs_lookup_dot: non-locked dvp"); - } else { - ltype = VOP_ISLOCKED(dvp); - VOP_UNLOCK(dvp, 0); - VN_HOLD(pvp); - *vpp = pvp; - vn_lock(*vpp, LK_EXCLUSIVE | LK_RETRY); - vn_lock(dvp, ltype | LK_RETRY); - } + ASSERT(pvp != NULL); + VN_HOLD(pvp); + *vpp = pvp; return (0); } Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c ============================================================================== --- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c Mon May 16 15:03:52 2016 (r299945) +++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ctldir.c Mon May 16 15:13:16 2016 (r299946) @@ -987,6 +987,11 @@ zfsctl_snapdir_lookup(ap) ZFS_ENTER(zfsvfs); if (gfs_lookup_dot(vpp, dvp, zfsvfs->z_ctldir, nm) == 0) { + if (nm[0] == '.' && nm[1] == '.' && nm[2] =='\0') { + VOP_UNLOCK(dvp, 0); + VERIFY0(vn_lock(*vpp, LK_EXCLUSIVE)); + VERIFY0(vn_lock(dvp, LK_EXCLUSIVE)); + } ZFS_EXIT(zfsvfs); return (0); } @@ -1151,6 +1156,11 @@ zfsctl_shares_lookup(ap) strlcpy(nm, cnp->cn_nameptr, cnp->cn_namelen + 1); if (gfs_lookup_dot(vpp, dvp, zfsvfs->z_ctldir, nm) == 0) { + if (nm[0] == '.' && nm[1] == '.' && nm[2] =='\0') { + VOP_UNLOCK(dvp, 0); + VERIFY0(vn_lock(*vpp, LK_EXCLUSIVE)); + VERIFY0(vn_lock(dvp, LK_EXCLUSIVE)); + } ZFS_EXIT(zfsvfs); return (0); } @@ -1488,7 +1498,6 @@ zfsctl_snapshot_reclaim(ap) VERIFY(gfs_dir_lookup(vp, "..", &dvp, cr, 0, NULL, NULL) == 0); sdp = dvp->v_data; - VOP_UNLOCK(dvp, 0); /* this may already have been unmounted */ if (sdp == NULL) { VN_RELE(dvp);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201605161513.u4GFDG1U023997>