From eecffc96eae89bf61f7530284c55baf92cf0c268 Mon Sep 17 00:00:00 2001 From: George Amanakis Date: Mon, 29 Aug 2022 21:38:12 +0200 Subject: [PATCH] Fix a race condition in dsl_dataset_sync() when activating features The zio returned from arc_write() in dmu_objset_sync() uses zio_nowait(). However we may reach the end of dsl_dataset_sync() which checks if we need to activate features in the filesystem without knowing if that zio has even run through the ZIO pipeline yet. In that case we will flag features to be activated in dsl_dataset_block_born() but dsl_dataset_sync() has already completed its run and those features will not actually be activated. Mitigate this by moving the feature activation code in dsl_dataset_sync_done(). Also add new ASSERTs in dsl_scan_visitbp() checking if a block contradicts any filesystem flags. Signed-off-by: George Amanakis --- include/sys/dsl_dataset.h | 1 + module/zfs/dmu_send.c | 1 + module/zfs/dsl_dataset.c | 23 ++++++++++++----------- module/zfs/dsl_scan.c | 20 ++++++++++++++++++++ 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/include/sys/dsl_dataset.h b/include/sys/dsl_dataset.h index 3450527af7e0..b0d8c7994c07 100644 --- a/include/sys/dsl_dataset.h +++ b/include/sys/dsl_dataset.h @@ -372,6 +372,7 @@ int dsl_dataset_rename_snapshot(const char *fsname, const char *oldsnapname, const char *newsnapname, boolean_t recursive); int dsl_dataset_snapshot_tmp(const char *fsname, const char *snapname, minor_t cleanup_minor, const char *htag); +boolean_t zfeature_active(spa_feature_t f, void *arg); blkptr_t *dsl_dataset_get_blkptr(dsl_dataset_t *ds); diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index 7f8de23f0e29..33beb04b19b1 100644 --- a/module/zfs/dmu_send.c +++ b/module/zfs/dmu_send.c @@ -493,6 +493,7 @@ dmu_dump_write(dmu_send_cookie_t *dscp, dmu_object_type_t type, uint64_t object, (bp != NULL ? BP_GET_COMPRESS(bp) != ZIO_COMPRESS_OFF && io_compressed : lsize != psize); if (raw || compressed) { + ASSERT(bp != NULL); ASSERT(raw || dscp->dsc_featureflags & DMU_BACKUP_FEATURE_COMPRESSED); ASSERT(!BP_IS_EMBEDDED(bp)); diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index 57a58f88cec5..0584a356f9ac 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -1039,7 +1039,7 @@ dsl_dataset_has_owner(dsl_dataset_t *ds) return (rv); } -static boolean_t +boolean_t zfeature_active(spa_feature_t f, void *arg) { switch (spa_feature_table[f].fi_type) { @@ -2121,16 +2121,6 @@ dsl_dataset_sync(dsl_dataset_t *ds, zio_t *zio, dmu_tx_t *tx) } dmu_objset_sync(ds->ds_objset, zio, tx); - - for (spa_feature_t f = 0; f < SPA_FEATURES; f++) { - if (zfeature_active(f, ds->ds_feature_activation[f])) { - if (zfeature_active(f, ds->ds_feature[f])) - continue; - dsl_dataset_activate_feature(ds->ds_object, f, - ds->ds_feature_activation[f], tx); - ds->ds_feature[f] = ds->ds_feature_activation[f]; - } - } } /* @@ -2303,6 +2293,17 @@ dsl_dataset_sync_done(dsl_dataset_t *ds, dmu_tx_t *tx) ASSERT(!dmu_objset_is_dirty(os, dmu_tx_get_txg(tx))); dmu_buf_rele(ds->ds_dbuf, ds); + + for (spa_feature_t f = 0; f < SPA_FEATURES; f++) { + if (zfeature_active(f, + ds->ds_feature_activation[f])) { + if (zfeature_active(f, ds->ds_feature[f])) + continue; + dsl_dataset_activate_feature(ds->ds_object, f, + ds->ds_feature_activation[f], tx); + ds->ds_feature[f] = ds->ds_feature_activation[f]; + } + } } int diff --git a/module/zfs/dsl_scan.c b/module/zfs/dsl_scan.c index 2cc77eab6275..f971aa211e0c 100644 --- a/module/zfs/dsl_scan.c +++ b/module/zfs/dsl_scan.c @@ -2038,6 +2038,26 @@ dsl_scan_visitbp(blkptr_t *bp, const zbookmark_phys_t *zb, return; } + /* + * Check if this block contradicts any filesystem flags. + */ + spa_feature_t f = SPA_FEATURE_LARGE_BLOCKS; + if (BP_GET_LSIZE(bp) > SPA_OLD_MAXBLOCKSIZE) + ASSERT(dsl_dataset_feature_is_active(ds, f)); + + f = zio_checksum_to_feature(BP_GET_CHECKSUM(bp)); + if (f != SPA_FEATURE_NONE) + ASSERT(dsl_dataset_feature_is_active(ds, f)); + + f = zio_compress_to_feature(BP_GET_COMPRESS(bp)); + if (f != SPA_FEATURE_NONE) + ASSERT(dsl_dataset_feature_is_active(ds, f)); + + if (bp->blk_birth <= scn->scn_phys.scn_cur_min_txg) { + scn->scn_lt_min_this_txg++; + return; + } + if (bp->blk_birth <= scn->scn_phys.scn_cur_min_txg) { scn->scn_lt_min_this_txg++; return;