Attachment 228714 Details for Bug 258208 – old ZFS VOP_LOCK work

[patch] old ZFS VOP_LOCK work

zfs_vop_lock.diff (text/plain), 11.63 KB, created by Andriy Gapon on 2021-10-15 09:30:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andriy Gapon

Created: 2021-10-15 09:30:14 UTC

Size: 11.63 KB

patch

obsolete

>diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/rrwlock.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/rrwlock.c
>index 6e7456efb2d5..eaee28932776 100644
>--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/rrwlock.c
>+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/rrwlock.c
>@@ -159,8 +159,8 @@ rrw_destroy(rrwlock_t *rrl)
> 	zfs_refcount_destroy(&rrl->rr_linked_rcount);
> }
> 
>-static void
>-rrw_enter_read_impl(rrwlock_t *rrl, boolean_t prio, void *tag)
>+static int
>+rrw_enter_read_impl(rrwlock_t *rrl, boolean_t prio, void *tag, boolean_t try)
> {
> 	mutex_enter(&rrl->rr_lock);
> #if !defined(DEBUG) && defined(_KERNEL)
>@@ -168,7 +168,7 @@ rrw_enter_read_impl(rrwlock_t *rrl, boolean_t prio, void *tag)
> 	    !rrl->rr_track_all) {
> 		rrl->rr_anon_rcount.rc_count++;
> 		mutex_exit(&rrl->rr_lock);
>-		return;
>+		return (1);
> 	}
> 	DTRACE_PROBE(zfs__rrwfastpath__rdmiss);
> #endif
>@@ -177,8 +177,13 @@ rrw_enter_read_impl(rrwlock_t *rrl, boolean_t prio, void *tag)
> 
> 	while (rrl->rr_writer != NULL || (rrl->rr_writer_wanted &&
> 	    zfs_refcount_is_zero(&rrl->rr_anon_rcount) && !prio &&
>-	    rrn_find(rrl) == NULL))
>-		cv_wait(&rrl->rr_cv, &rrl->rr_lock);
>+	    rrn_find(rrl) == NULL)) {
>+		if (try) {
>+			mutex_exit(&rrl->rr_lock);
>+			return (0);
>+		} else
>+			cv_wait(&rrl->rr_cv, &rrl->rr_lock);
>+	}
> 
> 	if (rrl->rr_writer_wanted || rrl->rr_track_all) {
> 		/* may or may not be a re-entrant enter */
>@@ -189,12 +194,13 @@ rrw_enter_read_impl(rrwlock_t *rrl, boolean_t prio, void *tag)
> 	}
> 	ASSERT(rrl->rr_writer == NULL);
> 	mutex_exit(&rrl->rr_lock);
>+	return (1);
> }
> 
> void
> rrw_enter_read(rrwlock_t *rrl, void *tag)
> {
>-	rrw_enter_read_impl(rrl, B_FALSE, tag);
>+	(void)rrw_enter_read_impl(rrl, B_FALSE, tag, B_FALSE);
> }
> 
> /*
>@@ -206,7 +212,7 @@ rrw_enter_read(rrwlock_t *rrl, void *tag)
> void
> rrw_enter_read_prio(rrwlock_t *rrl, void *tag)
> {
>-	rrw_enter_read_impl(rrl, B_TRUE, tag);
>+	(void)rrw_enter_read_impl(rrl, B_TRUE, tag, B_FALSE);
> }
> 
> 
>@@ -236,6 +242,13 @@ rrw_enter(rrwlock_t *rrl, krw_t rw, void *tag)
> 		rrw_enter_write(rrl);
> }
> 
>+int
>+rrw_tryenter(rrwlock_t *rrl, krw_t rw, void *tag)
>+{
>+	ASSERT(rw == RW_READER);
>+	return (rrw_enter_read_impl(rrl, B_FALSE, tag, B_TRUE));
>+}
>+
> void
> rrw_exit(rrwlock_t *rrl, void *tag)
> {
>@@ -357,6 +370,12 @@ rrm_enter(rrmlock_t *rrl, krw_t rw, void *tag)
>  */
> #define	RRM_TD_LOCK()	(((uint32_t)(uintptr_t)(curthread)) % RRM_NUM_LOCKS)
> 
>+int
>+rrm_tryenter(rrmlock_t *rrl, krw_t rw, void *tag)
>+{
>+	return (rrw_tryenter(&rrl->locks[RRM_TD_LOCK()], rw, tag));
>+}
>+
> void
> rrm_enter_read(rrmlock_t *rrl, void *tag)
> {
>diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/refcount.h b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/refcount.h
>index f1fd04792fef..745206374309 100644
>--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/refcount.h
>+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/refcount.h
>@@ -53,7 +53,11 @@ typedef struct reference {
> } reference_t;
> 
> typedef struct refcount {
>+#ifndef _KERNEL
> 	kmutex_t rc_mtx;
>+#else
>+	struct mtx rc_mtx;
>+#endif
> 	boolean_t rc_tracked;
> 	list_t rc_list;
> 	list_t rc_removed;
>diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/rrwlock.h b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/rrwlock.h
>index e0898dfe0ae8..2ec26718a923 100644
>--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/rrwlock.h
>+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/rrwlock.h
>@@ -51,7 +51,11 @@ extern "C" {
>  * - rr_writer_wanted: a writer wants the lock
>  */
> typedef struct rrwlock {
>+#ifndef _KERNEL
> 	kmutex_t	rr_lock;
>+#else
>+	struct mtx	rr_lock;
>+#endif
> 	kcondvar_t	rr_cv;
> 	kthread_t	*rr_writer;
> 	zfs_refcount_t	rr_anon_rcount;
>@@ -71,6 +75,7 @@ void rrw_enter(rrwlock_t *rrl, krw_t rw, void *tag);
> void rrw_enter_read(rrwlock_t *rrl, void *tag);
> void rrw_enter_read_prio(rrwlock_t *rrl, void *tag);
> void rrw_enter_write(rrwlock_t *rrl);
>+int rrw_tryenter(rrwlock_t *rrl, krw_t rw, void *tag);
> void rrw_exit(rrwlock_t *rrl, void *tag);
> boolean_t rrw_held(rrwlock_t *rrl, krw_t rw);
> void rrw_tsd_destroy(void *arg);
>@@ -97,6 +102,7 @@ void rrm_destroy(rrmlock_t *rrl);
> void rrm_enter(rrmlock_t *rrl, krw_t rw, void *tag);
> void rrm_enter_read(rrmlock_t *rrl, void *tag);
> void rrm_enter_write(rrmlock_t *rrl);
>+int rrm_tryenter(rrmlock_t *rrl, krw_t rw, void *tag);
> void rrm_exit(rrmlock_t *rrl, void *tag);
> boolean_t rrm_held(rrmlock_t *rrl, krw_t rw);
> 
>diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
>index 1ce186d0862e..c90457f14758 100644
>--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
>+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vfsops.c
>@@ -2073,16 +2073,12 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
> 		zil_close(zfsvfs->z_log);
> 		zfsvfs->z_log = NULL;
> 	}
>-
>-	ZFS_WLOCK_TEARDOWN_INACTIVE(zfsvfs);
>-
> 	/*
> 	 * If we are not unmounting (ie: online recv) and someone already
> 	 * unmounted this file system while we were doing the switcheroo,
> 	 * or a reopen of z_os failed then just bail out now.
> 	 */
> 	if (!unmounting && (zfsvfs->z_unmounted || zfsvfs->z_os == NULL)) {
>-		ZFS_WUNLOCK_TEARDOWN_INACTIVE(zfsvfs);
> 		rrm_exit(&zfsvfs->z_teardown_lock, FTAG);
> 		return (SET_ERROR(EIO));
> 	}
>@@ -2110,7 +2106,6 @@ zfsvfs_teardown(zfsvfs_t *zfsvfs, boolean_t unmounting)
> 	 */
> 	if (unmounting) {
> 		zfsvfs->z_unmounted = B_TRUE;
>-		ZFS_WUNLOCK_TEARDOWN_INACTIVE(zfsvfs);
> 		rrm_exit(&zfsvfs->z_teardown_lock, FTAG);
> 	}
> 
>@@ -2456,7 +2451,6 @@ zfs_resume_fs(zfsvfs_t *zfsvfs, dsl_dataset_t *ds)
> 	znode_t *zp;
> 
> 	ASSERT(RRM_WRITE_HELD(&zfsvfs->z_teardown_lock));
>-	ASSERT(ZFS_TEARDOWN_INACTIVE_WLOCKED(zfsvfs));
> 
> 	/*
> 	 * We already own this, so just update the objset_t, as the one we
>@@ -2490,7 +2484,6 @@ zfs_resume_fs(zfsvfs_t *zfsvfs, dsl_dataset_t *ds)
> 
> bail:
> 	/* release the VOPs */
>-	ZFS_WUNLOCK_TEARDOWN_INACTIVE(zfsvfs);
> 	rrm_exit(&zfsvfs->z_teardown_lock, FTAG);
> 
> 	if (err) {
>diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
>index 9ac9503d2f77..62718c5d6d5d 100644
>--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
>+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
>@@ -4270,13 +4270,11 @@ zfs_inactive(vnode_t *vp, cred_t *cr, caller_context_t *ct)
> 	zfsvfs_t *zfsvfs = zp->z_zfsvfs;
> 	int error;
> 
>-	ZFS_RLOCK_TEARDOWN_INACTIVE(zfsvfs);
> 	if (zp->z_sa_hdl == NULL) {
> 		/*
> 		 * The fs has been unmounted, or we did a
> 		 * suspend/resume and this file no longer exists.
> 		 */
>-		ZFS_RUNLOCK_TEARDOWN_INACTIVE(zfsvfs);
> 		vrecycle(vp);
> 		return;
> 	}
>@@ -4285,7 +4283,6 @@ zfs_inactive(vnode_t *vp, cred_t *cr, caller_context_t *ct)
> 		/*
> 		 * Fast path to recycle a vnode of a removed file.
> 		 */
>-		ZFS_RUNLOCK_TEARDOWN_INACTIVE(zfsvfs);
> 		vrecycle(vp);
> 		return;
> 	}
>@@ -4305,7 +4302,6 @@ zfs_inactive(vnode_t *vp, cred_t *cr, caller_context_t *ct)
> 			dmu_tx_commit(tx);
> 		}
> 	}
>-	ZFS_RUNLOCK_TEARDOWN_INACTIVE(zfsvfs);
> }
> 
> 
>@@ -5416,23 +5412,25 @@ zfs_freebsd_reclaim(ap)
> {
> 	vnode_t	*vp = ap->a_vp;
> 	znode_t	*zp = VTOZ(vp);
>-	zfsvfs_t *zfsvfs = zp->z_zfsvfs;
>+	zfsvfs_t *zfsvfs;
> 
> 	ASSERT(zp != NULL);
>+	zfsvfs = zp->z_zfsvfs;
> 
>-	/*
>-	 * z_teardown_inactive_lock protects from a race with
>-	 * zfs_znode_dmu_fini in zfsvfs_teardown during
>-	 * force unmount.
>-	 */
>-	ZFS_RLOCK_TEARDOWN_INACTIVE(zfsvfs);
> 	if (zp->z_sa_hdl == NULL)
> 		zfs_znode_free(zp);
> 	else
> 		zfs_zinactive(zp);
>-	ZFS_RUNLOCK_TEARDOWN_INACTIVE(zfsvfs);
> 
> 	vp->v_data = NULL;
>+
>+	/*
>+	 * Drop z_teardown_lock acquired in zfs_freebsd_lock,
>+	 * because zfs_freebsd_unlock won't be called on this vnode.
>+	 */
>+	lockmgr_assert(vp->v_vnlock, KA_XLOCKED | KA_NOTRECURSED);
>+	rrm_exit(&zfsvfs->z_teardown_lock, vp);
>+
> 	return (0);
> }
> 
>@@ -6010,9 +6008,8 @@ zfs_vptocnp(struct vop_vptocnp_args *ap)
> 	return (error);
> }
> 
>-#ifdef DIAGNOSTIC
>-static int
>-zfs_lock(ap)
>+int
>+zfs_freebsd_lock(ap)
> 	struct vop_lock1_args /* {
> 		struct vnode *a_vp;
> 		int a_flags;
>@@ -6020,21 +6017,71 @@ zfs_lock(ap)
> 		int line;
> 	} */ *ap;
> {
>-	vnode_t *vp;
>-	znode_t *zp;
>-	int err;
>+	struct vnode *vp = ap->a_vp;
>+	zfsvfs_t *zfsvfs;
>+	int error;
>+	int op;
> 
>-	err = vop_lock(ap);
>-	if (err == 0 && (ap->a_flags & LK_NOWAIT) == 0) {
>-		vp = ap->a_vp;
>-		zp = vp->v_data;
>-		if (vp->v_mount != NULL && !VN_IS_DOOMED(vp) &&
>-		    zp != NULL && (zp->z_pflags & ZFS_XATTR) == 0)
>-			VERIFY(!RRM_LOCK_HELD(&zp->z_zfsvfs->z_teardown_lock));
>+	op = ap->a_flags & LK_TYPE_MASK;
>+	if (op == LK_UPGRADE) {
>+		/*
>+		 * Failure to upgrade means losing shared lock.
>+		 * We drop z_teardown_lock now and re-acquire it
>+		 * if the upgrade is successful.
>+		 */
>+		ASSERT(VTOZ(vp) != NULL && VTOZ(vp)->z_zfsvfs != NULL);
>+		zfsvfs = VTOZ(vp)->z_zfsvfs;
>+		rrm_exit(&zfsvfs->z_teardown_lock, vp);
> 	}
>-	return (err);
>-}
>+	error = vop_lock(ap);
>+	if (op == LK_DOWNGRADE) {
>+		ASSERT(error == 0);
>+		return (error);
>+	}
>+	if (op == LK_TRYUPGRADE)
>+		return (error);
>+	ASSERT(op == LK_SHARED || op == LK_EXCLUSIVE || op == LK_UPGRADE);
>+	if (error != 0 || VN_IS_DOOMED(vp))
>+		return (error);
>+	ASSERT(VTOZ(vp) != NULL && VTOZ(vp)->z_zfsvfs != NULL);
>+	zfsvfs = VTOZ(vp)->z_zfsvfs;
>+
>+	/*
>+	 * zfsvfs_teardown -> zil_close -> zil_commit ->
>+	 * zfs_get_data -> zfs_zget -> zfs_znode_alloc
>+	 */
>+	if (!RRM_WRITE_HELD(&zfsvfs->z_teardown_lock)) {
>+		if ((ap->a_flags & LK_NOWAIT) == 0) {
>+#ifdef DIAGNOSTIC
>+			if (vp->v_mount != NULL && (VTOZ(vp)->z_pflags & ZFS_XATTR) == 0)
>+				ASSERT(!RRM_LOCK_HELD(&zp->z_zfsvfs->z_teardown_lock));
> #endif
>+			rrm_enter(&zfsvfs->z_teardown_lock, RW_READER, vp);
>+		} else if (!rrm_tryenter(&zfsvfs->z_teardown_lock, RW_READER,
>+		    vp)) {
>+			(void)lockmgr_unlock(&vp->v_lock);
>+			return (EBUSY);
>+		}
>+	}
>+	return (0);
>+}
>+
>+int
>+zfs_freebsd_unlock(ap)
>+	struct vop_unlock_args /* {
>+		struct vnode *a_vp;
>+		int a_flags;
>+	} */ *ap;
>+{
>+	struct vnode *vp = ap->a_vp;
>+	zfsvfs_t *zfsvfs = VTOZ(vp)->z_zfsvfs;
>+	int error;
>+
>+	error = vop_unlock(ap);
>+	if (!RRM_WRITE_HELD(&zfsvfs->z_teardown_lock))
>+		rrm_exit(&zfsvfs->z_teardown_lock, vp);
>+	return (error);
>+}
> 
> struct vop_vector zfs_vnodeops;
> struct vop_vector zfs_fifoops;
>@@ -6081,12 +6128,8 @@ struct vop_vector zfs_vnodeops = {
> 	.vop_getpages =		zfs_freebsd_getpages,
> 	.vop_putpages =		zfs_freebsd_putpages,
> 	.vop_vptocnp =		zfs_vptocnp,
>-#ifdef DIAGNOSTIC
>-	.vop_lock1 =		zfs_lock,
>-#else
>-	.vop_lock1 =		vop_lock,
>-#endif
>-	.vop_unlock =		vop_unlock,
>+	.vop_lock1 =		zfs_freebsd_lock,
>+	.vop_unlock =		zfs_freebsd_unlock,
> 	.vop_islocked =		vop_islocked,
> };
> VFS_VOP_VECTOR_REGISTER(zfs_vnodeops);
>@@ -6106,6 +6149,8 @@ struct vop_vector zfs_fifoops = {
> 	.vop_getacl =		zfs_freebsd_getacl,
> 	.vop_setacl =		zfs_freebsd_setacl,
> 	.vop_aclcheck =		zfs_freebsd_aclcheck,
>+	.vop_lock1 =		zfs_freebsd_lock,
>+	.vop_unlock =		zfs_freebsd_unlock,
> };
> VFS_VOP_VECTOR_REGISTER(zfs_fifoops);
> 
>@@ -6120,5 +6165,7 @@ struct vop_vector zfs_shareops = {
> 	.vop_reclaim =		zfs_freebsd_reclaim,
> 	.vop_fid =		zfs_freebsd_fid,
> 	.vop_pathconf =		zfs_freebsd_pathconf,
>+	.vop_lock1 =		zfs_freebsd_lock,
>+	.vop_unlock =		zfs_freebsd_unlock,
> };
> VFS_VOP_VECTOR_REGISTER(zfs_shareops);
>diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c
>index ecc11d16f42a..06e33c22f24f 100644
>--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c
>+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_znode.c
>@@ -679,7 +679,7 @@ zfs_znode_dmu_fini(znode_t *zp)
> {
> 	ASSERT(MUTEX_HELD(ZFS_OBJ_MUTEX(zp->z_zfsvfs, zp->z_id)) ||
> 	    zp->z_unlinked ||
>-	    ZFS_TEARDOWN_INACTIVE_WLOCKED(zp->z_zfsvfs));
>+	    RRM_WRITE_HELD(&zp->z_zfsvfs->z_teardown_lock));
> 
> 	sa_handle_destroy(zp->z_sa_hdl);
> 	zp->z_sa_hdl = NULL;

Actions: View | Diff

Attachments on bug 258208: 227959 | 228714 | 228715 | 228813 | 228815