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 <behlendorf1@llnl.gov>
Co-authored-by: Sanjeev Bagewadi <sanjeev.bagewadi@nutanix.com>
Signed-off-by: Chunwei Chen <david.chen@nutanix.com>
Closes #12482
Closes #15643

diff --git a/include/sys/dbuf.h b/include/sys/dbuf.h
index 1800a7e31da0cd4e7b11fc23aa67f4b5a7eb9717..2ff0bc72b2701e5895569d4c3bad1d0c1719226d 100644 (file)
--- 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 7ae74ad1318d47249dd4217fbaefbcd172b56c33..ba28aa06a91f9c66e46e621c639feaa3d7287745 100644 (file)
--- 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 8cffbdb9d20bf647535d44b99fbf2b55b998ba05..f67dad002319f93a9f81310ed7f1db9d1b3e84c5 100644 (file)
--- 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);