imrcpan/freebsd
1
0 Fork 0
mirror of https://github.com/freebsd/freebsd-src.git synced 2026-10-28 11:25:21 +01:00
Code Issues Packages Projects Releases Wiki Activity

zfs: spa_sync_upgrades() should only take lock when needed

Browse Source 
"zfs get -Hp ..." command got into waiting for memory allocation
while holding READER lock as dmu_objset_hold holds dp_config_rwlock READ
via dsl_pool_hold.

pageout was freeing memory by pushing some pages to swap zvol,
but got waiting for txg_wait_synced() and the memory was not freed.

txg_sync thread: spa_sync -> spa_sync_upgrades ->
rrw_enter(&dp->dp_config_rwlock, RW_WRITER) blocked.

This scenario did lead to investigation if spa_sync_upgrades() is
actually correct about requiring writer lock and it turns out that
we only do need write lock for three cases, where upgrade does change
DSL name space.

In other cases, and to check uberblock versions, we do not need
to set this lock.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Toomas Soome <[email protected]>
Closes #19092
This commit is contained in:
Toomas Soome authored and GitHub committed 2026-09-11 10:15:24 -07:00
1 parent 4e015d8d41
commit 61fbdf1a20
1 file changed
+40 -24
Download Patch File Download Diff File Expand all files Collapse all files
module/zfs/spa.c
+40 -24
View File
@@ -27,6 +27,7 @@
* Copyright (c) 2023 Hewlett Packard Enterprise Development LP.
* Copyright (c) 2023-2026, Klara, Inc.
* Copyright (c) 2026, TrueNAS.
* Copyright 2026 Edgecast Cloud LLC.
*/
/*
@@ -10867,32 +10868,49 @@ spa_sync_upgrades(spa_t *spa, dmu_tx_t *tx)
if (spa_sync_pass(spa) != 1)
return;
dsl_pool_t *dp = spa->spa_dsl_pool;
rrw_enter(&dp->dp_config_rwlock, RW_WRITER, FTAG);
uint64_t oldver = spa->spa_ubsync.ub_version;
uint64_t newver = spa->spa_uberblock.ub_version;
if (spa->spa_ubsync.ub_version < SPA_VERSION_ORIGIN &&
spa->spa_uberblock.ub_version >= SPA_VERSION_ORIGIN) {
dsl_pool_create_origin(dp, tx);
/*
* These upgrades change DSL namespace, so they need the
* writer lock.
*/
boolean_t need_origin = oldver < SPA_VERSION_ORIGIN &&
newver >= SPA_VERSION_ORIGIN;
boolean_t need_clones = oldver < SPA_VERSION_NEXT_CLONES &&
newver >= SPA_VERSION_NEXT_CLONES;
boolean_t need_dir_clones = oldver < SPA_VERSION_DIR_CLONES &&
newver >= SPA_VERSION_DIR_CLONES;
/* Keeping the origin open increases spa_minref */
spa->spa_minref += 3;
if (need_origin || need_clones || need_dir_clones) {
dsl_pool_t *dp = spa->spa_dsl_pool;
rrw_enter(&dp->dp_config_rwlock, RW_WRITER, FTAG);
if (need_origin) {
dsl_pool_create_origin(dp, tx);
/* Keeping the origin open increases spa_minref */
spa->spa_minref += 3;
}
if (need_clones) {
dsl_pool_upgrade_clones(dp, tx);
}
if (need_dir_clones) {
dsl_pool_upgrade_dir_clones(dp, tx);
/* Keeping the freedir open increases spa_minref */
spa->spa_minref += 3;
}
rrw_exit(&dp->dp_config_rwlock, FTAG);
}
if (spa->spa_ubsync.ub_version < SPA_VERSION_NEXT_CLONES &&
spa->spa_uberblock.ub_version >= SPA_VERSION_NEXT_CLONES) {
dsl_pool_upgrade_clones(dp, tx);
}
/* Remaining upgrades do not need dp_config_rwlock */
if (spa->spa_ubsync.ub_version < SPA_VERSION_DIR_CLONES &&
spa->spa_uberblock.ub_version >= SPA_VERSION_DIR_CLONES) {
dsl_pool_upgrade_dir_clones(dp, tx);
/* Keeping the freedir open increases spa_minref */
spa->spa_minref += 3;
}
if (spa->spa_ubsync.ub_version < SPA_VERSION_FEATURES &&
spa->spa_uberblock.ub_version >= SPA_VERSION_FEATURES) {
if (oldver < SPA_VERSION_FEATURES && newver >= SPA_VERSION_FEATURES) {
spa_feature_create_zap_objects(spa, tx);
}
@@ -10902,7 +10920,7 @@ spa_sync_upgrades(spa_t *spa, dmu_tx_t *tx)
* Old pools that have this feature enabled must be upgraded to have
* this feature active
*/
if (spa->spa_uberblock.ub_version >= SPA_VERSION_FEATURES) {
if (newver >= SPA_VERSION_FEATURES) {
boolean_t lz4_en = spa_feature_is_enabled(spa,
SPA_FEATURE_LZ4_COMPRESS);
boolean_t lz4_ac = spa_feature_is_active(spa,
@@ -10924,8 +10942,6 @@ spa_sync_upgrades(spa_t *spa, dmu_tx_t *tx)
sizeof (spa->spa_cksum_salt.zcs_bytes),
spa->spa_cksum_salt.zcs_bytes, tx));
}
rrw_exit(&dp->dp_config_rwlock, FTAG);
}
static void