t: Reverted reverted most of LFS_T_MKCONSISTENT

After thinking about this for a bit, there are some compelling
motivations for including an incremental LFS_T_MKCONSISTENT:

- Being able to run incremental LFS_T_MKCONSISTENT traversals in
  parallel with read-only operations is actually quite enticing.

  The only complicated part is maintaining the invalidatable traversal
  state, which already exists with lfsr_traversal_t (except the
  annoying LFS_F_MUTATED bit).

- While it's not really effective to combine LFS_T_MKCONSISTENT and
  LFS_T_LOOKAHEAD traversals, it _is_ possible to combine
  LFS_T_MKCONSISTENT with LFS_T_COMPACT, LFS_T_CKMETA,
  LFS_T_REPAIRMETA (future), etc.

  Really, LFS_T_LOOKAHEAD is the odd one out.

- Making LFS_T_MKCONSISTENT incremental means all filesystem-level
  traversals (except lfsr_mount) can be run incrementally. Which is a
  nice feature to have when O(n = entire fs) risks being very long
  running.

The main downside of LFS_T_MKCONSISTENT (and LFS_T_COMPACT, etc) is that
attempting to run it immediately after mount will likely recursively
trigger a lookahead scan to satisfy block allocation requests -- which
will block the current thread for the duration of the lookahead scan.
But this seems to be more a problem of LFS_T_LOOKAHEAD interacting with
other traversals poorly.

Fortunately, long term, the current plan is to replace the lookahead
buffer with an on-disk block map on disks where the lookahead scan is a
bottleneck. If this gets implemented the problem goes away.

So re-reverting this for now. Worst case we can always re-re-revert this
again in the future. There is already a working implementation, so might
as well see where it goes...

Supporting incremental LFS_T_MKCONSISTENT does add a bit of a code
cost, but there is still some room for deduplicating lfsr_mtree_gc +
lfsr_fs_mkconsistent, which may be interesting:

           code          stack
  before: 35232           2680
  after:  35480 (+0.7%)   2680 (+0.0%)

lfs.c

@@ -8265,6 +8265,10 @@ static inline bool lfsr_f_isdirty(uint32_t flags) {
     return flags & LFS_F_DIRTY;
 }
 
+static inline bool lfsr_f_ismutated(uint32_t flags) {
+    return flags & LFS_F_MUTATED;
+}
+
 
 
 // needed in lfsr_mtree_traverse_
@@ -8648,6 +8652,7 @@ static int lfsr_mtree_gc(lfs_t *lfs, lfsr_traversal_t *t,
     // lfsr_mtree_gc to work correctly
     LFS_ASSERT(lfsr_omdir_isopen(lfs, &t->o.o));
 
+again:;
     lfsr_tag_t tag;
     lfsr_bptr_t bptr;
     int err = lfsr_mtree_traverse(lfs, t,
@@ -8656,6 +8661,32 @@ static int lfsr_mtree_gc(lfs_t *lfs, lfsr_traversal_t *t,
         return err;
     }
 
+    // keep track of dirty flag before mutation
+    bool dirty = t->o.o.flags & LFS_F_DIRTY;
+
+    // mkconsistencing mdirs?
+    if (lfsr_t_ismkconsistent(t->o.o.flags)
+            && tag == LFSR_TAG_MDIR
+            && lfs->hasorphans) {
+        lfsr_mdir_t *mdir = (lfsr_mdir_t*)bptr.data.u.buffer;
+        int err = lfsr_mdir_fixorphans(lfs, mdir);
+        if (err) {
+            return err;
+        }
+
+        // did this drop our mdir?
+        if (mdir->mid != -1 && mdir->rbyd.weight == 0) {
+            t->o.o.flags &= ~LFS_F_ZOMBIE;
+            t->o.o.state = LFSR_TSTATE_MDIRS;
+
+            // downgrade any new dirty flags
+            t->o.o.flags |= (t->o.o.flags & LFS_F_DIRTY) ? LFS_F_MUTATED : 0;
+            t->o.o.flags &= ~LFS_F_DIRTY;
+            t->o.o.flags |= (dirty) ? LFS_F_DIRTY : 0;
+            goto again;
+        }
+    }
+
     // compacting mdirs?
     if (lfsr_t_iscompact(t->o.o.flags)
             && tag == LFSR_TAG_MDIR
@@ -8757,6 +8788,11 @@ static int lfsr_mtree_gc(lfs_t *lfs, lfsr_traversal_t *t,
         t->u.bt.rid = t->u.bt.bid;
     }
 
+    // downgrade any new dirty flags
+    t->o.o.flags |= (t->o.o.flags & LFS_F_DIRTY) ? LFS_F_MUTATED : 0;
+    t->o.o.flags &= ~LFS_F_DIRTY;
+    t->o.o.flags |= (dirty) ? LFS_F_DIRTY : 0;
+
     if (tag_) {
         *tag_ = tag;
     }
@@ -12664,54 +12700,77 @@ static int lfsr_fs_fixgrm(lfs_t *lfs) {
     return 0;
 }
 
+static int lfsr_mdir_fixorphans(lfs_t *lfs, lfsr_mdir_t *mdir) {
+    // save the current mid
+    lfsr_mid_t mid = mdir->mid;
+
+    // iterate through mids looking for orphans
+    mdir->mid = LFSR_MID(lfs, mdir->mid, 0);
+    int err;
+    while (lfsr_mid_rid(lfs, mdir->mid) < (lfsr_srid_t)mdir->rbyd.weight) {
+        // is this mid open? well we're not an orphan then, skip
+        if (lfsr_omdir_ismidopen(lfs, mdir->mid)) {
+            mdir->mid += 1;
+            continue;
+        }
+
+        // is this mid marked as an orphan?
+        err = lfsr_mdir_lookup(lfs, mdir, LFSR_TAG_ORPHAN,
+                NULL);
+        if (err) {
+            if (err == LFS_ERR_NOENT) {
+                mdir->mid += 1;
+                continue;
+            }
+            goto failed;
+        }
+
+        // we found an orphaned file, remove
+        LFS_DEBUG("Fixing orphan %"PRId32".%"PRId32,
+                lfsr_mid_bid(lfs, mdir->mid) >> lfs->mdir_bits,
+                lfsr_mid_rid(lfs, mdir->mid));
+
+        err = lfsr_mdir_commit(lfs, mdir, LFSR_ATTRS(
+                LFSR_ATTR(LFSR_TAG_RM, -1, LFSR_DATA_NULL())));
+        if (err) {
+            goto failed;
+        }
+    }
+
+    // restore the current mid
+    mdir->mid = mid;
+    return 0;
+
+failed:;
+    // restore the current mid
+    mdir->mid = mid;
+    return err;
+}
+
 static int lfsr_fs_fixorphans(lfs_t *lfs) {
     // iterate through the filesystem and remove any orphaned files
     //
     // note this never takes longer than lfsr_mount
     //
-    lfsr_mdir_t mdir = {.mid=0};
+    lfsr_mid_t mid = 0;
-    while (mdir.mid < (lfsr_srid_t)lfsr_mtree_weight(lfs)) {
+    while (mid < lfsr_mtree_weight(lfs)) {
-        int err = lfsr_mtree_lookup(lfs, LFSR_MID(lfs, mdir.mid, 0),
+        lfsr_mdir_t mdir;
+        int err = lfsr_mtree_lookup(lfs, mid,
                 &mdir);
         if (err) {
             LFS_ASSERT(err != LFS_ERR_NOENT);
             return err;
         }
 
-        while (lfsr_mid_rid(lfs, mdir.mid)
+        // clean up orphans
-                < (lfsr_srid_t)mdir.rbyd.weight) {
+        err = lfsr_mdir_fixorphans(lfs, &mdir);
-            // is this mid open? well we're not an orphan then, skip
+        if (err) {
-            if (lfsr_omdir_ismidopen(lfs, mdir.mid)) {
+            return err;
-                mdir.mid += 1;
-                continue;
-            }
-
-            // is this mid marked as an orphan?
-            err = lfsr_mdir_lookup(lfs, &mdir, LFSR_TAG_ORPHAN,
-                    NULL);
-            if (err) {
-                if (err == LFS_ERR_NOENT) {
-                    mdir.mid += 1;
-                    continue;
-                }
-                return err;
-            }
-
-            // we found an orphaned file, remove
-            LFS_DEBUG("Fixing orphan %"PRId32".%"PRId32,
-                    lfsr_mid_bid(lfs, mdir.mid) >> lfs->mdir_bits,
-                    lfsr_mid_rid(lfs, mdir.mid));
-
-            err = lfsr_mdir_commit(lfs, &mdir, LFSR_ATTRS(
-                    LFSR_ATTR(LFSR_TAG_RM, -1, LFSR_DATA_NULL())));
-            if (err) {
-                return err;
-            }
         }
 
-        // increment mid unless we dropped the mdir
+        // incremend mid unless we dropped the mdir
         if (mdir.rbyd.weight > 0) {
-            mdir.mid = lfsr_mid_bid(lfs, mdir.mid) + 1;
+            mid += 1 << lfs->mdir_bits;
         }
     }
 
@@ -12856,14 +12915,34 @@ int lfsr_traversal_read(lfs_t *lfs, lfsr_traversal_t *t,
         struct lfs_tinfo *tinfo) {
     LFS_ASSERT(lfsr_omdir_isopen(lfs, &t->o.o));
 
-    // mkconsistencing?
+    // check for pending grms every step, just in case some other
-    //
+    // operation introduced new grms
-    // check every step in case some other operation introduced a grm/orphan
+    if (lfsr_t_ismkconsistent(t->o.o.flags)
-    if (lfsr_t_ismkconsistent(t->o.o.flags)) {
+            && lfsr_grm_count(lfs) > 0) {
-        int err = lfsr_fs_mkconsistent(lfs);
+        if (lfsr_grm_count(lfs) == 2) {
+            LFS_DEBUG("Fixing grm %"PRId32".%"PRId32" %"PRId32".%"PRId32,
+                    lfsr_mid_bid(lfs, lfs->grm.mids[0]) >> lfs->mdir_bits,
+                    lfsr_mid_rid(lfs, lfs->grm.mids[0]),
+                    lfsr_mid_bid(lfs, lfs->grm.mids[1]) >> lfs->mdir_bits,
+                    lfsr_mid_rid(lfs, lfs->grm.mids[1]));
+        } else if (lfsr_grm_count(lfs) == 1) {
+            LFS_DEBUG("Fixing grm %"PRId32".%"PRId32,
+                    lfsr_mid_bid(lfs, lfs->grm.mids[0]) >> lfs->mdir_bits,
+                    lfsr_mid_rid(lfs, lfs->grm.mids[0]));
+        }
+
+        // keep track of dirty flag before mutation
+        bool dirty = t->o.o.flags & LFS_F_DIRTY;
+
+        int err = lfsr_fs_fixgrm(lfs);
         if (err) {
             return err;
         }
+
+        // downgrade any new dirty flags
+        t->o.o.flags |= (t->o.o.flags & LFS_F_DIRTY) ? LFS_F_MUTATED : 0;
+        t->o.o.flags &= ~LFS_F_DIRTY;
+        t->o.o.flags |= (dirty) ? LFS_F_DIRTY : 0;
     }
 
     while (true) {
@@ -12915,9 +12994,16 @@ int lfsr_traversal_read(lfs_t *lfs, lfsr_traversal_t *t,
     }
 
 done:;
+    // was mkconsistent successful?
+    if (lfsr_t_ismkconsistent(t->o.o.flags)
+            && !lfsr_f_isdirty(t->o.o.flags)) {
+        lfs->hasorphans = false;
+    }
+
     // was a lookahead scan successful?
     if (lfsr_t_islookahead(t->o.o.flags)
-            && !lfsr_f_isdirty(t->o.o.flags)) {
+            && !lfsr_f_isdirty(t->o.o.flags)
+            && !lfsr_f_ismutated(t->o.o.flags)) {
         lfs_alloc_markfree(lfs);
     }
 
@@ -12926,11 +13012,8 @@ done:;
 
 static void lfsr_traversal_clobber(lfs_t *lfs, lfsr_traversal_t *t) {
     (void)lfs;
-    // not yet started?
-    if (t->o.o.state < LFSR_TSTATE_MROOTCHAIN) {
-        // do nothing
     // mroot/mtree? transition to mdir iteration
-    } else if (t->o.o.state < LFSR_TSTATE_MDIRS) {
+    if (t->o.o.state < LFSR_TSTATE_MDIRS) {
         t->o.o.state = LFSR_TSTATE_MDIRS;
         t->o.o.mdir.mid = 0;
         t->o.bshrub.u.bshrub.weight = 0;
@@ -12963,7 +13046,7 @@ static void lfsr_traversal_clobber(lfs_t *lfs, lfsr_traversal_t *t) {
 static int lfsr_traversal_rewind_(lfs_t *lfs, lfsr_traversal_t *t) {
     (void)lfs;
     // reset traversal
-    t->o.o.flags &= ~LFS_F_DIRTY;
+    t->o.o.flags &= ~LFS_F_DIRTY & ~LFS_F_MUTATED;
     t->o.o.state = LFSR_TSTATE_MROOTANCHOR;
     t->o.o.mdir.mid = -1;
     t->o.o.mdir.rbyd.weight = 0;

lfs.h

@@ -179,7 +179,8 @@ enum lfs_traversal_flags {
 //    LFS_T_REPAIRDATA    = 0x0c00, // Repair metadata + data blocks
 
     // internally used flags
-    LFS_F_DIRTY         = 0x1000, // Filesystem has been modified
+    LFS_F_DIRTY         = 0x1000, // Filesystem modified during traversal
+    LFS_F_MUTATED       = 0x4000, // Filesystem modified by traversal
 };
 
 

tests/test_traversal.toml

@@ -6964,10 +6964,13 @@ code = '''
     lfsr_traversal_read(&lfs, &t, &tinfo) => LFS_ERR_NOENT;
     lfsr_traversal_close(&lfs, &t) => 0;
 
-    // we should be able to clean up grms and orphans
+    // we should be able to clean up grms
     assert(lfs.grm.mids[0] == -1);
     assert(lfs.grm.mids[1] == -1);
-    assert(lfs.hasorphans == false);
+    // if we introduce actual orphans, me _must not_ clear the orphan flag
+    if (ORPHANS > 3) {
+        assert(lfs.hasorphans == true);
+    }
 
     // check we can still read the files
     for (int remount = 0; remount < 2; remount++) {
@@ -7836,10 +7839,13 @@ code = '''
     lfsr_traversal_read(&lfs, &t, &tinfo) => LFS_ERR_NOENT;
     lfsr_traversal_close(&lfs, &t) => 0;
 
-    // we should be able to clean up grms and orphans
+    // we should be able to clean up grms
     assert(lfs.grm.mids[0] == -1);
     assert(lfs.grm.mids[1] == -1);
-    assert(lfs.hasorphans == false);
+    // if we introduce actual orphans, me _must not_ clear the orphan flag
+    if (ORPHANS > 3) {
+        assert(lfs.hasorphans == true);
+    }
 
     // mdirs should have been compacted
     assert((file1.o.o.mdir.rbyd.eoff & 0x7fffffff) <= GC_COMPACT_THRESH);