Skip to content

Commit

Permalink
dbuf: separate refcount calls for dbuf and dbuf_user
Browse files Browse the repository at this point in the history
In 92dc4ad I updated the dbuf_cache accounting to track the size of
userdata associated with dbufs. This adds the size of the dbuf+userdata
together in a single call to zfs_refcount_add_many(), but sometime
removes them in separate calls to zfs_refcount_remove_many(), if dbuf
and userdata are evicted separately.

What I didn't realise is that when refcount tracking is on,
zfs_refcount_add_many() and zfs_refcount_remove_many() are expected to
be paired, with their second & third args (count & holder) the same on
both sides. Splitting the remove part into two calls means the counts
don't match up, tripping a panic.

This commit fixes that, by always adding and removing the dbuf and
userdata counts separately.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reported-by: Mark Johnston <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16191
(cherry picked from commit e675852)
  • Loading branch information
robn authored and 0mp committed Dec 3, 2024
1 parent acd3c7c commit c9304ad
Showing 1 changed file with 19 additions and 9 deletions.
28 changes: 19 additions & 9 deletions module/zfs/dbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,7 @@ dbuf_evict_user(dmu_buf_impl_t *db)
*/
uint64_t size = dbu->dbu_size;
(void) zfs_refcount_remove_many(
&dbuf_caches[db->db_caching_status].size, size, db);
&dbuf_caches[db->db_caching_status].size, size, dbu);
if (db->db_caching_status == DB_DBUF_CACHE)
DBUF_STAT_DECR(cache_levels_bytes[db->db_level], size);
}
Expand Down Expand Up @@ -785,12 +785,15 @@ dbuf_evict_one(void)
if (db != NULL) {
multilist_sublist_remove(mls, db);
multilist_sublist_unlock(mls);
uint64_t size = db->db.db_size + dmu_buf_user_size(&db->db);
uint64_t size = db->db.db_size;
uint64_t usize = dmu_buf_user_size(&db->db);
(void) zfs_refcount_remove_many(
&dbuf_caches[DB_DBUF_CACHE].size, size, db);
(void) zfs_refcount_remove_many(
&dbuf_caches[DB_DBUF_CACHE].size, usize, db->db_user);
DBUF_STAT_BUMPDOWN(cache_levels[db->db_level]);
DBUF_STAT_BUMPDOWN(cache_count);
DBUF_STAT_DECR(cache_levels_bytes[db->db_level], size);
DBUF_STAT_DECR(cache_levels_bytes[db->db_level], size + usize);
ASSERT3U(db->db_caching_status, ==, DB_DBUF_CACHE);
db->db_caching_status = DB_NO_CACHE;
dbuf_destroy(db);
Expand Down Expand Up @@ -3799,16 +3802,21 @@ dbuf_hold_impl(dnode_t *dn, uint8_t level, uint64_t blkid,

multilist_remove(&dbuf_caches[db->db_caching_status].cache, db);

uint64_t size = db->db.db_size + dmu_buf_user_size(&db->db);
uint64_t size = db->db.db_size;
uint64_t usize = dmu_buf_user_size(&db->db);
(void) zfs_refcount_remove_many(
&dbuf_caches[db->db_caching_status].size, size, db);
(void) zfs_refcount_remove_many(
&dbuf_caches[db->db_caching_status].size, usize,
db->db_user);

if (db->db_caching_status == DB_DBUF_METADATA_CACHE) {
DBUF_STAT_BUMPDOWN(metadata_cache_count);
} else {
DBUF_STAT_BUMPDOWN(cache_levels[db->db_level]);
DBUF_STAT_BUMPDOWN(cache_count);
DBUF_STAT_DECR(cache_levels_bytes[db->db_level], size);
DBUF_STAT_DECR(cache_levels_bytes[db->db_level],
size + usize);
}
db->db_caching_status = DB_NO_CACHE;
}
Expand Down Expand Up @@ -4027,10 +4035,12 @@ dbuf_rele_and_unlock(dmu_buf_impl_t *db, const void *tag, boolean_t evicting)
db->db_caching_status = dcs;

multilist_insert(&dbuf_caches[dcs].cache, db);
uint64_t db_size = db->db.db_size +
dmu_buf_user_size(&db->db);
size = zfs_refcount_add_many(
uint64_t db_size = db->db.db_size;
uint64_t dbu_size = dmu_buf_user_size(&db->db);
(void) zfs_refcount_add_many(
&dbuf_caches[dcs].size, db_size, db);
size = zfs_refcount_add_many(
&dbuf_caches[dcs].size, dbu_size, db->db_user);
uint8_t db_level = db->db_level;
mutex_exit(&db->db_mtx);

Expand All @@ -4043,7 +4053,7 @@ dbuf_rele_and_unlock(dmu_buf_impl_t *db, const void *tag, boolean_t evicting)
DBUF_STAT_MAX(cache_size_bytes_max, size);
DBUF_STAT_BUMP(cache_levels[db_level]);
DBUF_STAT_INCR(cache_levels_bytes[db_level],
db_size);
db_size + dbu_size);
}

if (dcs == DB_DBUF_CACHE && !evicting)
Expand Down

0 comments on commit c9304ad

Please sign in to comment.