Re: zfs questions

commit 931c7c7a2fcfcf2efdb735f99fc99119a77eadc3
Author: YAMAMOTO Takashi <[email protected]>
Date:   Wed Feb 18 07:56:41 2026 +0000

    zfs: fix "slow rm" issue
    
    * stop commiting zil in zfs_netbsd_reclaim and other operations
      in vnode reclaim path.
    
    * retire zfs_zget_cleaner/VN_RELE_CLEANER.
      instead, just use normal zfs_zget and vrele_async.
    
    note that these two changes depend on each other:
    
    * zfs_zget_cleaner relies on zil_commit in zfs_netbsd_reclaim to
      ensure that the znode referenced by TX_WRITE itx is always in-core.
    
    * otoh, zfs_zget_clear makes zil_commit in the vnode reclaim path
      possible. that is, zfs_netbsd_reclaim (VOP_RECLAIM) is called with
      the vnode in VS_RECLAIMING state, which would make vcache_vget
      block.
      if the vnode being reclaimed happened to have TX_WRITE itx on the
      zil, it deadlocks.
    
    an alternative would be to make the upper layer (vfs_vnode.c) retain
    unlinked vnodes for a while. (a bit longer than the 5 sec txg commit
    interval should be enough.) eg. by making zfs_netbsd_inactive report
    a_recycle = 0. but i guess it's better to remove
    zfs_zget_cleaner/VN_RELE_CLEANER to to keep the code less diverged
    from the upstream zfs.
    
    PR/59885
    https://gnats.netbsd.org/59885

diff --git a/external/cddl/osnet/dist/uts/common/fs/zfs/sys/zfs_znode.h b/external/cddl/osnet/dist/uts/common/fs/zfs/sys/zfs_znode.h
index 935b4f18d985..128309d233cc 100644
--- a/external/cddl/osnet/dist/uts/common/fs/zfs/sys/zfs_znode.h
+++ b/external/cddl/osnet/dist/uts/common/fs/zfs/sys/zfs_znode.h
@@ -336,7 +336,6 @@ extern int	zfs_loadvnode(struct mount *, struct vnode *,
     const void *, size_t, const void **);
 extern int	zfs_newvnode(struct mount *, struct vnode *, struct vnode *,
     struct vattr *, kauth_cred_t, void *, size_t *, const void **);
-extern int	zfs_zget_cleaner(zfsvfs_t *, uint64_t, znode_t **);
 #endif
 extern int	zfs_zget(zfsvfs_t *, uint64_t, znode_t **);
 extern int	zfs_rezget(znode_t *);
diff --git a/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c b/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c
index d66debdfddd7..c2d4cd0dec95 100644
--- a/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c
+++ b/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_vnops.c
@@ -1449,7 +1449,7 @@ zfs_get_done(zgd_t *zgd, int error)
 	 * Release the vnode asynchronously as we currently have the
 	 * txg stopped from syncing.
 	 */
-	VN_RELE_CLEANER(ZTOV(zp), dsl_pool_vnrele_taskq(dmu_objset_pool(os)));
+	VN_RELE_ASYNC(ZTOV(zp), dsl_pool_vnrele_taskq(dmu_objset_pool(os)));
 
 	if (error == 0 && zgd->zgd_bp)
 		zil_add_block(zgd->zgd_zilog, zgd->zgd_bp);
@@ -1484,14 +1484,14 @@ zfs_get_data(void *arg, lr_write_t *lr, char *buf, zio_t *zio)
 	/*
 	 * Nothing to do if the file has been removed
 	 */
-	if (zfs_zget_cleaner(zfsvfs, object, &zp) != 0)
+	if (zfs_zget(zfsvfs, object, &zp) != 0)
 		return (SET_ERROR(ENOENT));
 	if (zp->z_unlinked) {
 		/*
 		 * Release the vnode asynchronously as we currently have the
 		 * txg stopped from syncing.
 		 */
-		VN_RELE_CLEANER(ZTOV(zp),
+		VN_RELE_ASYNC(ZTOV(zp),
 		    dsl_pool_vnrele_taskq(dmu_objset_pool(os)));
 		return (SET_ERROR(ENOENT));
 	}
@@ -5522,6 +5522,20 @@ zfs_netbsd_fsync(void *v)
 {
 	struct vop_fsync_args *ap = v;
 
+	/*
+	 * it isn't safe or necessary to call zil_commit when reclaiming
+	 * a vnode.
+	 *
+	 * - it can deadlock by attempting vcache_get on itself.
+	 *   (zfs_get_data)
+	 *
+	 * - for the purpose of vnode reclaim, we only need to push the
+	 *   data to the txg. no need to log the intent.
+	 */
+	if ((ap->a_flags & FSYNC_RECLAIM) != 0) {
+		return (0);
+	}
+
 	return (zfs_fsync(ap->a_vp, ap->a_flags, ap->a_cred, NULL));
 }
 
@@ -5881,14 +5895,6 @@ zfs_netbsd_reclaim(void *v)
 		}
 	}
 
-	/*
-	 * Operation zfs_znode.c::zfs_zget_cleaner() depends on this
-	 * zil_commit() as a barrier to guarantee the znode cannot
-	 * get freed before its log entries are resolved.
-	 */
-	if (zfsvfs->z_log)
-		zil_commit(zfsvfs->z_log, zp->z_id);
-
 	if (zp->z_sa_hdl == NULL)
 		zfs_znode_free(zp);
 	else
@@ -6228,8 +6234,6 @@ zfs_netbsd_putpages(void *v)
 	uint64_t len;
 	int error;
 	bool cleaned = false;
-
-	bool async = (flags & PGO_SYNCIO) == 0;
 	bool cleaning = (flags & PGO_CLEANIT) != 0;
 
 	if (cleaning) {
@@ -6275,16 +6279,21 @@ zfs_netbsd_putpages(void *v)
 	}
 	error = genfs_putpages(v);
 	if (cleaning) {
+		bool commit = (flags & (PGO_SYNCIO|PGO_RECLAIM)) == PGO_SYNCIO;
+
 		tsd_set(zfs_putpage_key, NULL);
 		zfs_range_unlock(rl);
 
 		/*
 		 * Only zil_commit() if we cleaned something.  This avoids 
 		 * deadlock if we're called from zfs_netbsd_setsize().
+		 *
+		 * Also, it isn't safe or nessesary to call it for vnode
+		 * reclaim. See the comment in zfs_netbsd_fsync.
 		 */
 
-		if (cleaned)
-		if (!async || zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
+		if (cleaned &&
+		    (commit || zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS))
 			zil_commit(zfsvfs->z_log, zp->z_id);
 fail:
 		ZFS_EXIT(zfsvfs);
diff --git a/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_znode.c b/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_znode.c
index 4d3ab49393d7..d5a132802f5b 100644
--- a/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_znode.c
+++ b/external/cddl/osnet/dist/uts/common/fs/zfs/zfs_znode.c
@@ -1291,43 +1291,6 @@ zfs_zget(zfsvfs_t *zfsvfs, uint64_t obj_num, znode_t **zpp)
 	return error;
 }
 
-/*
- * Get a known cached znode, to be used from zil_commit()->zfs_get_data()
- * to resolve log entries.  Doesn't take a reference, will never fail and
- * depends on zfs_vnops.c::zfs_netbsd_reclaim() running a zil_commit()
- * before the znode gets freed.
- */
-int
-zfs_zget_cleaner(zfsvfs_t *zfsvfs, uint64_t obj_num, znode_t **zpp)
-{
-	dmu_buf_t *db;
-	sa_handle_t *hdl;
-	dmu_object_info_t doi;
-	znode_t *zp;
-
-	ZFS_OBJ_HOLD_ENTER(zfsvfs, obj_num);
-
-	VERIFY(0 == sa_buf_hold(zfsvfs->z_os, obj_num, NULL, &db));
-
-	dmu_object_info_from_db(db, &doi);
-	ASSERT(doi.doi_bonus_type == DMU_OT_SA ||
-	    (doi.doi_bonus_type == DMU_OT_ZNODE &&
-	    doi.doi_bonus_size >= sizeof (znode_phys_t)));
-
-	hdl = dmu_buf_get_user(db);
-	ASSERT3P(hdl, !=, NULL);
-
-	zp = sa_get_userdata(hdl);
-	ASSERT3U(zp->z_id, ==, obj_num);
-
-	sa_buf_rele(db, NULL);
-
-	ZFS_OBJ_HOLD_EXIT(zfsvfs, obj_num);
-
-	*zpp = zp;
-	return (0);
-}
-
 #else /* __NetBSD__ */
 
 int
diff --git a/external/cddl/osnet/sys/sys/vnode.h b/external/cddl/osnet/sys/sys/vnode.h
index fddea6d82088..76fca9cf77e5 100644
--- a/external/cddl/osnet/sys/sys/vnode.h
+++ b/external/cddl/osnet/sys/sys/vnode.h
@@ -172,7 +172,6 @@ do {								      \
 #define	VN_URELE(v)	vput(v)
 #undef VN_RELE_ASYNC
 #define VN_RELE_ASYNC(vp, taskq) 	vrele_async((vp))
-#define VN_RELE_CLEANER(vp, taskq)	/* nothing */
 
 #define	vnevent_create(vp, ct)			do { } while (0)
 #define	vnevent_link(vp, ct)			do { } while (0)