From 3abf72b2517665f39a6ef2c5247aa5c90b84aac0 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Tue, 19 Aug 2025 17:11:07 +1000 Subject: [PATCH] dnode: add dn_dirtycnt, count of number of txgs this dnode is dirty on Bumped when we take the dirty hold in dnode_setdirty(), dropped when the dnode is finally cleaned up after sync in dnode_rele_task() or userquota_updates_task(). This gives us a way to check if the dnode is dirty on any txg without having to rely on outside information (eg presence on a dirty list), which has been a rich source of bugs in the past. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Suggested-by: Robert Evans Reviewed-by: Brian Behlendorf Reviewed-by: Robert Evans Reviewed-by: Adam Moss Reviewed-by: Alexander Motin Signed-off-by: Rob Norris Closes #16297 Closes #17652 Closes #17658 --- include/sys/dmu_impl.h | 1 + include/sys/dnode.h | 1 + module/zfs/dmu_objset.c | 6 ++++++ module/zfs/dnode.c | 30 +++++++++++++++++++++++------- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/include/sys/dmu_impl.h b/include/sys/dmu_impl.h index 21a8b16a3..1cc18e5ca 100644 --- a/include/sys/dmu_impl.h +++ b/include/sys/dmu_impl.h @@ -169,6 +169,7 @@ extern "C" { * dn_free_txg * dn_assigned_txg * dn_dirty_txg + * dn_dirtycnt * dd_assigned_tx * dn_notxholds * dn_nodnholds diff --git a/include/sys/dnode.h b/include/sys/dnode.h index 76218c8b0..efff9be93 100644 --- a/include/sys/dnode.h +++ b/include/sys/dnode.h @@ -341,6 +341,7 @@ struct dnode { uint64_t dn_free_txg; uint64_t dn_assigned_txg; uint64_t dn_dirty_txg; /* txg dnode was last dirtied */ + uint8_t dn_dirtycnt; kcondvar_t dn_notxholds; kcondvar_t dn_nodnholds; enum dnode_dirtycontext dn_dirtyctx; diff --git a/module/zfs/dmu_objset.c b/module/zfs/dmu_objset.c index a77f338bd..8e6b569c2 100644 --- a/module/zfs/dmu_objset.c +++ b/module/zfs/dmu_objset.c @@ -2037,6 +2037,8 @@ userquota_updates_task(void *arg) dn->dn_id_flags |= DN_ID_CHKED_BONUS; } dn->dn_id_flags &= ~(DN_ID_NEW_EXIST); + ASSERT3U(dn->dn_dirtycnt, >, 0); + dn->dn_dirtycnt--; mutex_exit(&dn->dn_mtx); multilist_sublist_remove(list, dn); @@ -2070,6 +2072,10 @@ dnode_rele_task(void *arg) dnode_t *dn; while ((dn = multilist_sublist_head(list)) != NULL) { + mutex_enter(&dn->dn_mtx); + ASSERT3U(dn->dn_dirtycnt, >, 0); + dn->dn_dirtycnt--; + mutex_exit(&dn->dn_mtx); multilist_sublist_remove(list, dn); dnode_rele(dn, &os->os_synced_dnodes); } diff --git a/module/zfs/dnode.c b/module/zfs/dnode.c index 963ff4123..62e1749a8 100644 --- a/module/zfs/dnode.c +++ b/module/zfs/dnode.c @@ -176,6 +176,7 @@ dnode_cons(void *arg, void *unused, int kmflag) dn->dn_dirty_txg = 0; dn->dn_dirtyctx = 0; dn->dn_dirtyctx_firstset = NULL; + dn->dn_dirtycnt = 0; dn->dn_bonus = NULL; dn->dn_have_spill = B_FALSE; dn->dn_zio = NULL; @@ -232,6 +233,7 @@ dnode_dest(void *arg, void *unused) ASSERT0(dn->dn_dirty_txg); ASSERT0(dn->dn_dirtyctx); ASSERT0P(dn->dn_dirtyctx_firstset); + ASSERT0(dn->dn_dirtycnt); ASSERT0P(dn->dn_bonus); ASSERT(!dn->dn_have_spill); ASSERT0P(dn->dn_zio); @@ -693,6 +695,7 @@ dnode_destroy(dnode_t *dn) dn->dn_free_txg = 0; dn->dn_assigned_txg = 0; dn->dn_dirty_txg = 0; + dn->dn_dirtycnt = 0; dn->dn_dirtyctx = 0; dn->dn_dirtyctx_firstset = NULL; @@ -805,6 +808,7 @@ dnode_allocate(dnode_t *dn, dmu_object_type_t ot, int blocksize, int ibs, dn->dn_free_txg = 0; dn->dn_dirtyctx_firstset = NULL; dn->dn_dirty_txg = 0; + dn->dn_dirtycnt = 0; dn->dn_allocated_txg = tx->tx_txg; dn->dn_id_flags = 0; @@ -958,6 +962,7 @@ dnode_move_impl(dnode_t *odn, dnode_t *ndn) ndn->dn_dirty_txg = odn->dn_dirty_txg; ndn->dn_dirtyctx = odn->dn_dirtyctx; ndn->dn_dirtyctx_firstset = odn->dn_dirtyctx_firstset; + ndn->dn_dirtycnt = odn->dn_dirtycnt; ASSERT0(zfs_refcount_count(&odn->dn_tx_holds)); zfs_refcount_transfer(&ndn->dn_holds, &odn->dn_holds); ASSERT(avl_is_empty(&ndn->dn_dbufs)); @@ -1023,6 +1028,7 @@ dnode_move_impl(dnode_t *odn, dnode_t *ndn) odn->dn_dirty_txg = 0; odn->dn_dirtyctx = 0; odn->dn_dirtyctx_firstset = NULL; + odn->dn_dirtycnt = 0; odn->dn_have_spill = B_FALSE; odn->dn_zio = NULL; odn->dn_oldused = 0; @@ -1757,17 +1763,23 @@ dnode_hold(objset_t *os, uint64_t object, const void *tag, dnode_t **dnp) * reference on the dnode. Returns FALSE if unable to add a * new reference. */ +static boolean_t +dnode_add_ref_locked(dnode_t *dn, const void *tag) +{ + ASSERT(MUTEX_HELD(&dn->dn_mtx)); + if (zfs_refcount_is_zero(&dn->dn_holds)) + return (FALSE); + VERIFY(1 < zfs_refcount_add(&dn->dn_holds, tag)); + return (TRUE); +} + boolean_t dnode_add_ref(dnode_t *dn, const void *tag) { mutex_enter(&dn->dn_mtx); - if (zfs_refcount_is_zero(&dn->dn_holds)) { - mutex_exit(&dn->dn_mtx); - return (FALSE); - } - VERIFY(1 < zfs_refcount_add(&dn->dn_holds, tag)); + boolean_t r = dnode_add_ref_locked(dn, tag); mutex_exit(&dn->dn_mtx); - return (TRUE); + return (r); } void @@ -1916,7 +1928,11 @@ dnode_setdirty(dnode_t *dn, dmu_tx_t *tx) * dnode will hang around after we finish processing its * children. */ - VERIFY(dnode_add_ref(dn, (void *)(uintptr_t)tx->tx_txg)); + mutex_enter(&dn->dn_mtx); + VERIFY(dnode_add_ref_locked(dn, (void *)(uintptr_t)tx->tx_txg)); + dn->dn_dirtycnt++; + ASSERT3U(dn->dn_dirtycnt, <=, 3); + mutex_exit(&dn->dn_mtx); (void) dbuf_dirty(dn->dn_dbuf, tx);