Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

dbuf: separate refcount calls for dbuf and dbuf_user 

In 92dc4ad83 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 <markj@FreeBSD.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes #16191
This commit is contained in:
Rob N 2024-05-16 06:03:41 +10:00 committed by GitHub
parent 3c941d1818
commit e675852bc1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 19 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
module/zfs/dbuf.c
View File

@ -578,7 +578,7 @@ dbuf_evict_user(dmu_buf_impl_t *db)
*/ */
uint64_t size = dbu->dbu_size; uint64_t size = dbu->dbu_size;
(void) zfs_refcount_remove_many( (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) if (db->db_caching_status == DB_DBUF_CACHE)
DBUF_STAT_DECR(cache_levels_bytes[db->db_level], size); DBUF_STAT_DECR(cache_levels_bytes[db->db_level], size);
} }
@ -784,12 +784,15 @@ dbuf_evict_one(void)
if (db != NULL) { if (db != NULL) {
multilist_sublist_remove(mls, db); multilist_sublist_remove(mls, db);
multilist_sublist_unlock(mls); 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( (void) zfs_refcount_remove_many(
&dbuf_caches[DB_DBUF_CACHE].size, size, db); &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_levels[db->db_level]);
DBUF_STAT_BUMPDOWN(cache_count); 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); ASSERT3U(db->db_caching_status, ==, DB_DBUF_CACHE);
db->db_caching_status = DB_NO_CACHE; db->db_caching_status = DB_NO_CACHE;
dbuf_destroy(db); dbuf_destroy(db);
@ -3794,16 +3797,21 @@ dbuf_hold_impl(dnode_t *dn, uint8_t level, uint64_t blkid,
multilist_remove(&dbuf_caches[db->db_caching_status].cache, db); 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( (void) zfs_refcount_remove_many(
&dbuf_caches[db->db_caching_status].size, size, db); &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) { if (db->db_caching_status == DB_DBUF_METADATA_CACHE) {
DBUF_STAT_BUMPDOWN(metadata_cache_count); DBUF_STAT_BUMPDOWN(metadata_cache_count);
} else { } else {
DBUF_STAT_BUMPDOWN(cache_levels[db->db_level]); DBUF_STAT_BUMPDOWN(cache_levels[db->db_level]);
DBUF_STAT_BUMPDOWN(cache_count); 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; db->db_caching_status = DB_NO_CACHE;
} }
@ -4022,10 +4030,12 @@ dbuf_rele_and_unlock(dmu_buf_impl_t *db, const void *tag, boolean_t evicting)
db->db_caching_status = dcs; db->db_caching_status = dcs;
multilist_insert(&dbuf_caches[dcs].cache, db); multilist_insert(&dbuf_caches[dcs].cache, db);
uint64_t db_size = db->db.db_size + uint64_t db_size = db->db.db_size;
dmu_buf_user_size(&db->db); uint64_t dbu_size = dmu_buf_user_size(&db->db);
size = zfs_refcount_add_many( (void) zfs_refcount_add_many(
&dbuf_caches[dcs].size, db_size, db); &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; uint8_t db_level = db->db_level;
mutex_exit(&db->db_mtx); mutex_exit(&db->db_mtx);
@ -4038,7 +4048,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_MAX(cache_size_bytes_max, size);
DBUF_STAT_BUMP(cache_levels[db_level]); DBUF_STAT_BUMP(cache_levels[db_level]);
DBUF_STAT_INCR(cache_levels_bytes[db_level], DBUF_STAT_INCR(cache_levels_bytes[db_level],
db_size); db_size + dbu_size);
} }
if (dcs == DB_DBUF_CACHE && !evicting) if (dcs == DB_DBUF_CACHE && !evicting)