From ba227e2cc218a9983842a0a80eae21cdbf96a08b Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Tue, 10 Jun 2025 12:30:06 -0400 Subject: [PATCH] Make TX abort after assign safer It is not right, but there are few examples when TX is aborted after being assigned in case of error. To handle it better on production systems add extra cleanup steps. While here, replace couple dmu_tx_abort() in simple cases. Reviewed-by: Rob Norris Reviewed-by: Brian Behlendorf Reviewed-by: Igor Kozhukhov Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #17438 --- module/zfs/dmu_tx.c | 11 ++++++++++- module/zfs/zfs_vnops.c | 4 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/module/zfs/dmu_tx.c b/module/zfs/dmu_tx.c index c2e6c749f..c81529a44 100644 --- a/module/zfs/dmu_tx.c +++ b/module/zfs/dmu_tx.c @@ -1392,6 +1392,7 @@ dmu_tx_destroy(dmu_tx_t *tx) void dmu_tx_commit(dmu_tx_t *tx) { + /* This function should only be used on assigned transactions. */ ASSERT(tx->tx_txg != 0); /* @@ -1430,7 +1431,12 @@ dmu_tx_commit(dmu_tx_t *tx) void dmu_tx_abort(dmu_tx_t *tx) { - ASSERT(tx->tx_txg == 0); + /* This function should not be used on assigned transactions. */ + ASSERT0(tx->tx_txg); + + /* Should not be needed, but better be safe than sorry. */ + if (tx->tx_tempreserve_cookie) + dsl_dir_tempreserve_clear(tx->tx_tempreserve_cookie, tx); /* * Call any registered callbacks with an error code. @@ -1438,6 +1444,9 @@ dmu_tx_abort(dmu_tx_t *tx) if (!list_is_empty(&tx->tx_callbacks)) dmu_tx_do_callbacks(&tx->tx_callbacks, SET_ERROR(ECANCELED)); + /* Should not be needed, but better be safe than sorry. */ + dmu_tx_unassign(tx); + dmu_tx_destroy(tx); } diff --git a/module/zfs/zfs_vnops.c b/module/zfs/zfs_vnops.c index 0b9eb05e8..9aba525d7 100644 --- a/module/zfs/zfs_vnops.c +++ b/module/zfs/zfs_vnops.c @@ -1188,7 +1188,7 @@ zfs_rewrite(znode_t *zp, uint64_t off, uint64_t len, uint64_t flags, error = dmu_buf_hold_array_by_dnode(dn, off, n, TRUE, FTAG, &numbufs, &dbp, DMU_READ_PREFETCH | DMU_UNCACHEDIO); if (error) { - dmu_tx_abort(tx); + dmu_tx_commit(tx); break; } for (int i = 0; i < numbufs; i++) { @@ -1860,7 +1860,7 @@ zfs_clone_range(znode_t *inzp, uint64_t *inoffp, znode_t *outzp, */ if (inblksz != outzp->z_blksz) { error = SET_ERROR(EINVAL); - dmu_tx_abort(tx); + dmu_tx_commit(tx); break; }