From 4ae931aa9340e8ca61f21c98ae7ba5554e39129b Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Wed, 11 Jun 2025 14:13:48 -0400 Subject: [PATCH] Polish db_rwlock scope dbuf_verify(): Don't need the lock, since we only compare pointers. dbuf_findbp(): Don't need the lock, since aside of unneeded assert we only produce the pointer, but don't de-reference it. dnode_next_offset_level(): When working on top level indirection should lock dnode buffer's db_rwlock, since it is our parent. If dnode has no buffer, then it is meta-dnode or one of quotas and we should lock the dataset's ds_bp_rwlock instead. Reviewed-by: Alan Somers Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #17441 --- module/zfs/dbuf.c | 17 +++-------------- module/zfs/dnode.c | 11 +++++++++++ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index f94cfd726..fd0909531 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -1185,16 +1185,9 @@ dbuf_verify(dmu_buf_impl_t *db) ASSERT3U(db->db_parent->db_level, ==, db->db_level+1); ASSERT3U(db->db_parent->db.db_object, ==, db->db.db_object); - /* - * dnode_grow_indblksz() can make this fail if we don't - * have the parent's rwlock. XXX indblksz no longer - * grows. safe to do this now? - */ - if (RW_LOCK_HELD(&db->db_parent->db_rwlock)) { - ASSERT3P(db->db_blkptr, ==, - ((blkptr_t *)db->db_parent->db.db_data + - db->db_blkid % epb)); - } + ASSERT3P(db->db_blkptr, ==, + ((blkptr_t *)db->db_parent->db.db_data + + db->db_blkid % epb)); } } if ((db->db_blkptr == NULL || BP_IS_HOLE(db->db_blkptr)) && @@ -3381,12 +3374,8 @@ dbuf_findbp(dnode_t *dn, int level, uint64_t blkid, int fail_sparse, *parentp = NULL; return (err); } - rw_enter(&(*parentp)->db_rwlock, RW_READER); *bpp = ((blkptr_t *)(*parentp)->db.db_data) + (blkid & ((1ULL << epbs) - 1)); - if (blkid > (dn->dn_phys->dn_maxblkid >> (level * epbs))) - ASSERT(BP_IS_HOLE(*bpp)); - rw_exit(&(*parentp)->db_rwlock); return (0); } else { /* the block is referenced from the dnode */ diff --git a/module/zfs/dnode.c b/module/zfs/dnode.c index 88ea0fe6a..904a039ed 100644 --- a/module/zfs/dnode.c +++ b/module/zfs/dnode.c @@ -2559,6 +2559,11 @@ dnode_next_offset_level(dnode_t *dn, int flags, uint64_t *offset, error = 0; epb = dn->dn_phys->dn_nblkptr; data = dn->dn_phys->dn_blkptr; + if (dn->dn_dbuf != NULL) + rw_enter(&dn->dn_dbuf->db_rwlock, RW_READER); + else if (dmu_objset_ds(dn->dn_objset) != NULL) + rrw_enter(&dmu_objset_ds(dn->dn_objset)->ds_bp_rwlock, + RW_READER, FTAG); } else { uint64_t blkid = dbuf_whichblock(dn, lvl, *offset); error = dbuf_hold_impl(dn, lvl, blkid, TRUE, FALSE, FTAG, &db); @@ -2663,6 +2668,12 @@ dnode_next_offset_level(dnode_t *dn, int flags, uint64_t *offset, if (db != NULL) { rw_exit(&db->db_rwlock); dbuf_rele(db, FTAG); + } else { + if (dn->dn_dbuf != NULL) + rw_exit(&dn->dn_dbuf->db_rwlock); + else if (dmu_objset_ds(dn->dn_objset) != NULL) + rrw_exit(&dmu_objset_ds(dn->dn_objset)->ds_bp_rwlock, + FTAG); } return (error);