From ddb01c17fe643c9021b1db81469666f0674dfcd1 Mon Sep 17 00:00:00 2001 From: Paul Dagnelie Date: Wed, 10 Apr 2019 10:54:42 -0700 Subject: [PATCH] DLPX-63281 PANIC at zfeature.c:411:feature_do_action() (#44) --- include/sys/dsl_pool.h | 1 + module/zfs/dsl_pool.c | 26 ++++++++++++++++++++++++++ module/zfs/spa.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+) diff --git a/include/sys/dsl_pool.h b/include/sys/dsl_pool.h index 63ba3509a532..2b80f0a7995b 100644 --- a/include/sys/dsl_pool.h +++ b/include/sys/dsl_pool.h @@ -165,6 +165,7 @@ void dsl_free_sync(zio_t *pio, dsl_pool_t *dp, uint64_t txg, void dsl_pool_create_origin(dsl_pool_t *dp, dmu_tx_t *tx); void dsl_pool_upgrade_clones(dsl_pool_t *dp, dmu_tx_t *tx); void dsl_pool_upgrade_dir_clones(dsl_pool_t *dp, dmu_tx_t *tx); +void dsl_pool_sync_bookmark_featureflags(dsl_pool_t *dp, dmu_tx_t *tx); void dsl_pool_mos_diduse_space(dsl_pool_t *dp, int64_t used, int64_t comp, int64_t uncomp); void dsl_pool_ckpoint_diduse_space(dsl_pool_t *dp, diff --git a/module/zfs/dsl_pool.c b/module/zfs/dsl_pool.c index 864376c1e5f8..9adbedbab140 100644 --- a/module/zfs/dsl_pool.c +++ b/module/zfs/dsl_pool.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -1074,6 +1075,31 @@ dsl_pool_upgrade_dir_clones(dsl_pool_t *dp, dmu_tx_t *tx) upgrade_dir_clones_cb, tx, DS_FIND_CHILDREN | DS_FIND_SERIALIZE)); } +static int +sync_bookmark_featureflags_cb(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg) +{ + dmu_tx_t *tx = arg; + for (dsl_bookmark_node_t *dbn = avl_first(&ds->ds_bookmarks); + dbn != NULL; dbn = AVL_NEXT(&ds->ds_bookmarks, dbn)) { + if (dbn->dbn_phys.zbm_flags & ZBM_FLAG_HAS_FBN || + dbn->dbn_phys.zbm_redaction_obj != 0) { + spa_feature_incr(dp->dp_spa, SPA_FEATURE_BOOKMARK_V2, + tx); + } + } + return (0); +} + +void +dsl_pool_sync_bookmark_featureflags(dsl_pool_t *dp, dmu_tx_t *tx) +{ + ASSERT(dmu_tx_is_syncing(tx)); + + VERIFY0(dmu_objset_find_dp(dp, dp->dp_root_dir_obj, + sync_bookmark_featureflags_cb, tx, DS_FIND_CHILDREN | + DS_FIND_SERIALIZE)); +} + void dsl_pool_create_origin(dsl_pool_t *dp, dmu_tx_t *tx) { diff --git a/module/zfs/spa.c b/module/zfs/spa.c index dc4b32effc17..46ec884a66a6 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -8026,6 +8026,39 @@ spa_sync_upgrades(spa_t *spa, dmu_tx_t *tx) if (lz4_en && !lz4_ac) spa_feature_incr(spa, SPA_FEATURE_LZ4_COMPRESS, tx); + + /* + * Unfortunately, BOOKMARK_V2 was added as a dependency of + * REDACTION_BOOKMARKS and BOOKMARK_WRITTEN after they were + * already in the wild. As a result, any pool that already + * has those features that is upgraded to a version with + * BOOKMARK_V2 will crash if the number of bookmarks ever goes + * below the amount that existed when the pool was upgraded. + * This code iterates over all the bookmarks in the system and + * increments the V2 feature once for each bookmark that uses + * either BOOKMARK_WRITTEN or REDACTION_BOOKMARKS. It's not + * just the sum of the two, because some bookmarks will have + * both REDACTION and WRITTEN, and some could have either + * without the other. + * + * This logic will only execute once because, going forwards, + * any time BOOKMARK_WRITTEN or REDACTION_BOOKMARKS is + * incremented, BOOKMARK_V2 will be as well. As a result, its + * count should never dip back to zero (maaking in inactive + * but enabled) while the other two are active. + */ + boolean_t bv2_en = spa_feature_is_enabled(spa, + SPA_FEATURE_BOOKMARK_V2); + boolean_t bv2_ac = spa_feature_is_active(spa, + SPA_FEATURE_BOOKMARK_V2); + + boolean_t dep_ac = spa_feature_is_active(spa, + SPA_FEATURE_BOOKMARK_WRITTEN) || spa_feature_is_active(spa, + SPA_FEATURE_REDACTION_BOOKMARKS); + + if (bv2_en && !bv2_ac && dep_ac) { + dsl_pool_sync_bookmark_featureflags(dp, tx); + } } /*