DLPX-63281 PANIC at zfeature.c:411:feature_do_action() (openzfs#44)

sdimitro · Apr 10, 2019 · ddb01c1 · ddb01c1
1 parent aedc12e
commit ddb01c1
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 0 deletions.
diff --git 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
@@ -32,6 +32,7 @@
 #include <sys/dsl_dir.h>
 #include <sys/dsl_synctask.h>
 #include <sys/dsl_scan.h>
+#include <sys/dsl_bookmark.h>
 #include <sys/dnode.h>
 #include <sys/dmu_tx.h>
 #include <sys/dmu_objset.h>
@@ -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
@@ -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);
+		}
 	}
 
 	/*