Wait for the txg that finished a scan to sync

dsl_scan_done() marks the scan DSS_FINISHED in syncing context, and
right before that it asks vdev_dtl_reassess() to update the DTLs and
dirty the config, which the same txg writes out later in its
spa_sync().  spa_activity_in_progress() looks at the state alone:

    scanning = (scn->scn_phys.scn_state == DSS_SCANNING);
    ...
    *in_progress = (scanning && !paused && ...);

so "zpool wait -t resilver" and "zpool wait -t scrub" return while the
labels of that txg are still being written; spa_vdev_resilver_done()
notifies the waiters from the async thread, which can run before the
sync ends, so a waiter is not even guaranteed to sleep.

A command issued right after the wait then reads a pool whose labels
are mid-rewrite.  ZTS vdev_zaps_004_pos fails on this: it runs "zdb
-PC" straight after "zpool wait -t resilver" and the pool fails to
open, which on FreeBSD ends the test with

    zdb: can't open 'testpool': Device not configured

A dtrace of the attach in a FreeBSD VM, with a 100 ms write delay on
the disks to widen the window, showed "zpool wait" returning 10 ms
after dsl_scan_done() and seconds before the spa_sync() of that txg
had ended.

Record the txg the scan was finished in and keep reporting the scan as
in progress until that txg has synced, the way 8e5bb1d4b ("Wait for
the initialize and trim threads to exit") made those waits cover the
whole operation.  This waits for the txg already in flight, not for a
new one, and only for scans: the rebuild predicate above it is left
alone, since zpool_wait_rebuild gives "zpool wait" two seconds to exit
and a full txg under its injected I/O delay does not fit.  spa_sync()
notifies the waiters at its end so that they look again once the txg
is on disk.

Verified in VMs on FreeBSD 14.3, FreeBSD 15.1 and Linux: the same
dtrace now shows "zpool wait" returning after the spa_sync() of the
final txg, and the zpool_wait, zpool_wait/scan and vdev_zaps groups
pass.  The race itself does not reproduce on an idle machine, so the
CI failures below are explained by this timeline rather than
reproduced.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: George Melikov <[email protected]>
Closes #19066

include/sys/dsl_scan.h

@@ -108,6 +108,11 @@ typedef struct dsl_errorscrub_phys {
  *			the scan but have not yet been processed (i.e deferred
  *			frees) are accounted for.
  *
+ * scn_finished_txg -	the txg dsl_scan_done() marked the scan finished in.
+ *			That happens in syncing context, ahead of the config
+ *			and label writes of the same txg, so the scan is still
+ *			reported as in progress until this txg has synced.
+ *
  * This structure also maintains information about deferred frees which are
  * a special kind of traversal. Deferred free can exist in either a bptree or
  * a bpobj structure. The scn_is_bptree flag will indicate the type of
@@ -118,6 +123,7 @@ typedef struct dsl_scan {
 	struct dsl_pool *scn_dp;
 	uint64_t scn_restart_txg;
 	uint64_t scn_done_txg;
+	uint64_t scn_finished_txg;
 	uint64_t scn_sync_start_time;
 	uint64_t scn_issued_before_pass;
 

module/zfs/dsl_scan.c

@@ -1312,6 +1312,12 @@ dsl_scan_done(dsl_scan_t *scn, boolean_t complete, dmu_tx_t *tx)
 		scn->scn_phys.scn_state = complete ? DSS_FINISHED :
 		    DSS_CANCELED;
 		scn->scn_phys.scn_end_time = gethrestime_sec();
+		/*
+		 * The new state, and the config and labels updated above,
+		 * reach disk when this txg syncs.  Note it so that
+		 * "zpool wait" does not return before then.
+		 */
+		scn->scn_finished_txg = tx->tx_txg;
 		spa->spa_scrub_started = B_FALSE;
 
 		/*
@@ -1345,6 +1351,7 @@ dsl_scan_done(dsl_scan_t *scn, boolean_t complete, dmu_tx_t *tx)
 		scn->scn_phys.scn_state = complete ? DSS_FINISHED :
 		    DSS_CANCELED;
 		scn->scn_phys.scn_end_time = gethrestime_sec();
+		scn->scn_finished_txg = tx->tx_txg;
 	}
 
 	spa_notify_waiters(spa);

module/zfs/spa.c

@@ -11388,6 +11388,13 @@ spa_sync(spa_t *spa, uint64_t txg)
 	spa->spa_ubsync = spa->spa_uberblock;
 	spa_config_exit(spa, SCL_CONFIG, FTAG);
 
+	/*
+	 * An activity that ended in this txg is only over for a reader of
+	 * the pool now that the txg is on disk, so let the waiters look
+	 * again (see spa_activity_in_progress()).
+	 */
+	spa_notify_waiters(spa);
+
 	spa_handle_ignored_writes(spa);
 
 	/*
@@ -11898,13 +11905,25 @@ spa_activity_in_progress(spa_t *spa, zpool_wait_activity_t activity,
 		zfs_fallthrough;
 	case ZPOOL_WAIT_SCRUB:
 	{
-		boolean_t scanning, paused, is_scrub;
+		boolean_t scanning, paused, is_scrub, finishing;
 		dsl_scan_t *scn =  spa->spa_dsl_pool->dp_scan;
 
 		is_scrub = (scn->scn_phys.scn_func == POOL_SCAN_SCRUB);
 		scanning = (scn->scn_phys.scn_state == DSS_SCANNING);
 		paused = dsl_scan_is_paused_scrub(scn);
-		*in_progress = (scanning && !paused &&
+
+		/*
+		 * dsl_scan_done() marks the scan finished in syncing
+		 * context, ahead of the config and label writes that the
+		 * same txg carries, so the scan is not over for anyone
+		 * reading the pool until that txg has synced.  Keep
+		 * reporting it as in progress until then, the way the
+		 * initialize and trim waits cover the whole operation.
+		 */
+		finishing = (scn->scn_finished_txg != 0 &&
+		    spa_last_synced_txg(spa) < scn->scn_finished_txg);
+
+		*in_progress = ((scanning || finishing) && !paused &&
 		    is_scrub == (activity == ZPOOL_WAIT_SCRUB));
 		break;
 	}