From a9b937e066644c767e43aeeb46783e3139378b47 Mon Sep 17 00:00:00 2001 From: Chunwei Chen Date: Mon, 11 Dec 2023 14:42:06 -0800 Subject: [PATCH] For db_marker inherit the db pointer for AVL comparision. While evicting dbufs of a dnode, a marker node is added to the AVL. The marker node should be inserted in AVL tree ahead of the dbuf its trying to delete. The blkid and level is used to ensure this. However, this could go wrong there's another dbufs with the same blkid and level in DB_EVICTING state but not yet removed from AVL tree. dbuf_compare() could fail to give the right location or could cause confusion and trigger ASSERTs. To ensure that the marker is inserted before the deleting dbuf, use the pointer value of the original dbuf for comparision. Reviewed-by: Brian Behlendorf Co-authored-by: Sanjeev Bagewadi Signed-off-by: Chunwei Chen Closes #12482 Closes #15643 --- include/sys/dbuf.h | 1 + module/zfs/dnode.c | 8 ++++++++ module/zfs/dnode_sync.c | 9 ++++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/include/sys/dbuf.h b/include/sys/dbuf.h index 1800a7e31d..2ff0bc72b2 100644 --- a/include/sys/dbuf.h +++ b/include/sys/dbuf.h @@ -79,6 +79,7 @@ extern "C" { * dbuf_states_t (see comment on dn_dbufs in dnode.h). */ typedef enum dbuf_states { + DB_MARKER = -2, DB_SEARCH = -1, DB_UNCACHED, DB_FILL, diff --git a/module/zfs/dnode.c b/module/zfs/dnode.c index 7ae74ad131..ba28aa06a9 100644 --- a/module/zfs/dnode.c +++ b/module/zfs/dnode.c @@ -99,6 +99,14 @@ dbuf_compare(const void *x1, const void *x2) if (likely(cmp)) return (cmp); + if (d1->db_state == DB_MARKER) { + ASSERT3S(d2->db_state, !=, DB_MARKER); + return (TREE_PCMP(d1->db_parent, d2)); + } else if (d2->db_state == DB_MARKER) { + ASSERT3S(d1->db_state, !=, DB_MARKER); + return (TREE_PCMP(d1, d2->db_parent)); + } + if (d1->db_state == DB_SEARCH) { ASSERT3S(d2->db_state, !=, DB_SEARCH); return (-1); diff --git a/module/zfs/dnode_sync.c b/module/zfs/dnode_sync.c index 8cffbdb9d2..f67dad0023 100644 --- a/module/zfs/dnode_sync.c +++ b/module/zfs/dnode_sync.c @@ -482,7 +482,14 @@ dnode_evict_dbufs(dnode_t *dn) zfs_refcount_is_zero(&db->db_holds)) { db_marker->db_level = db->db_level; db_marker->db_blkid = db->db_blkid; - db_marker->db_state = DB_SEARCH; + /* + * Insert a MARKER node with the same level and blkid. + * And to resolve any ties in dbuf_compare() use the + * pointer of the dbuf that we are evicting. Pass the + * address in db_parent. + */ + db_marker->db_state = DB_MARKER; + db_marker->db_parent = (void *)((uintptr_t)db - 1); avl_insert_here(&dn->dn_dbufs, db_marker, db, AVL_BEFORE);