Illumos 6450 - scrub/resilver unnecessarily traverses snapshots

6450 scrub/resilver unnecessarily traverses snapshots created
after the scrub started
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Richard Elling <Richard.Elling@RichardElling.com>
Approved by: Richard Lowe <richlowe@richlowe.net>

References:
  https://www.illumos.org/issues/6450
  https://github.com/illumos/illumos-gate/commit/38d6103

Ported-by: kernelOfTruth kerneloftruth@gmail.com
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

diff --git a/module/zfs/dsl_scan.c b/module/zfs/dsl_scan.c
index 2b95561a9d24348e6124f7a96035f40957f9199a..166d0c1eb6e2cbc593cc41d8ff3f0e7513538301 100644 (file)
--- a/module/zfs/dsl_scan.c
+++ b/module/zfs/dsl_scan.c
@@ -872,7 +872,16 @@ dsl_scan_ds_destroyed(dsl_dataset_t *ds, dmu_tx_t *tx)
 
        if (scn->scn_phys.scn_bookmark.zb_objset == ds->ds_object) {
                if (ds->ds_is_snapshot) {
-                       /* Note, scn_cur_{min,max}_txg stays the same. */
+                       /*
+                        * Note:
+                        *  - scn_cur_{min,max}_txg stays the same.
+                        *  - Setting the flag is not really necessary if
+                        *    scn_cur_max_txg == scn_max_txg, because there
+                        *    is nothing after this snapshot that we care
+                        *    about.  However, we set it anyway and then
+                        *    ignore it when we retraverse it in
+                        *    dsl_scan_visitds().
+                        */
                        scn->scn_phys.scn_bookmark.zb_objset =
                            dsl_dataset_phys(ds)->ds_next_snap_obj;
                        zfs_dbgmsg("destroying ds %llu; currently traversing; "
@@ -912,9 +921,6 @@ dsl_scan_ds_destroyed(dsl_dataset_t *ds, dmu_tx_t *tx)
                        zfs_dbgmsg("destroying ds %llu; in queue; removing",
                            (u_longlong_t)ds->ds_object);
                }
-       } else {
-               zfs_dbgmsg("destroying ds %llu; ignoring",
-                   (u_longlong_t)ds->ds_object);
        }
 
        /*
@@ -1068,6 +1074,46 @@ dsl_scan_visitds(dsl_scan_t *scn, uint64_t dsobj, dmu_tx_t *tx)
 
        VERIFY3U(0, ==, dsl_dataset_hold_obj(dp, dsobj, FTAG, &ds));
 
+       if (scn->scn_phys.scn_cur_min_txg >=
+           scn->scn_phys.scn_max_txg) {
+               /*
+                * This can happen if this snapshot was created after the
+                * scan started, and we already completed a previous snapshot
+                * that was created after the scan started.  This snapshot
+                * only references blocks with:
+                *
+                *      birth < our ds_creation_txg
+                *      cur_min_txg is no less than ds_creation_txg.
+                *      We have already visited these blocks.
+                * or
+                *      birth > scn_max_txg
+                *      The scan requested not to visit these blocks.
+                *
+                * Subsequent snapshots (and clones) can reference our
+                * blocks, or blocks with even higher birth times.
+                * Therefore we do not need to visit them either,
+                * so we do not add them to the work queue.
+                *
+                * Note that checking for cur_min_txg >= cur_max_txg
+                * is not sufficient, because in that case we may need to
+                * visit subsequent snapshots.  This happens when min_txg > 0,
+                * which raises cur_min_txg.  In this case we will visit
+                * this dataset but skip all of its blocks, because the
+                * rootbp's birth time is < cur_min_txg.  Then we will
+                * add the next snapshots/clones to the work queue.
+                */
+               char *dsname = kmem_alloc(MAXNAMELEN, KM_SLEEP);
+               dsl_dataset_name(ds, dsname);
+               zfs_dbgmsg("scanning dataset %llu (%s) is unnecessary because "
+                   "cur_min_txg (%llu) >= max_txg (%llu)",
+                   dsobj, dsname,
+                   scn->scn_phys.scn_cur_min_txg,
+                   scn->scn_phys.scn_max_txg);
+               kmem_free(dsname, MAXNAMELEN);
+
+               goto out;
+       }
+
        if (dmu_objset_from_ds(ds, &os))
                goto out;