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
This commit is contained in:
Chunwei Chen 2023-12-11 14:42:06 -08:00 committed by GitHub
parent b1748eaee0
commit a9b937e066
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 17 additions and 1 deletions

View File

@ -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,

View File

@ -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);

View File

@ -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);