commit 931c7c7a2fcfcf2efdb735f99fc99119a77eadc3 Author: YAMAMOTO Takashi 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)