t: Reverted most of LFS_T_MKCONSISTENT, just check for new grms/orphans

Checking for orphans + other traversal work turned out to mesh much
worse than originally thought:

- Adjusting mids and being able to drop mdirs mid-traversal complicates
  traversal quite a bit and has potential to hide difficult to reproduce
  bugs.

- Implementing incremental mkconsistent requires it's own separate state
  to detect mutation correctly since LFS_T_MKCONSISTENT and
  LFS_T_LOOKAHEAD are invalidated by slightly different things.

- If hasorphans=true, we're likely going to find orphans and clobber the
  traversal. So it's not really worth trying to opportunistically prove
  there are no orphans while doing other traversal operations.

- We don't really want to traverse the mroot/mtree during mkconsistent,
  which makes deduplicating these two functions a bit tricky. Doable,
  but annoying.

- grms don't involve traversals and are their own separate awkward step
  already.

Combine this with the fact that needing to scan for orphans should be
relatively rare in practice -- requiring either a powerloss or a
complicated set of file operations with at minimum 3 desynced files --
and parallel orphan checking starts to look like more trouble than it's
worth...

Instead, we now only check if the hasorphan bit has been set, and if it
has been we just call lfsr_fs_mkconsistent directly. This does a full
traversal in a single step, but at least makes it so traversal +
LFS_T_MKCONSISTENT in a background thread will do any necessary
janitorial work.

This saves a bit code:

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

lfs.c

@@ -8265,10 +8265,6 @@ 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_
@@ -8652,7 +8648,6 @@ 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,
@@ -8661,32 +8656,6 @@ again:;
         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
@@ -8788,11 +8757,6 @@ again:;
         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;
     }
@@ -12700,77 +12664,54 @@ 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_mid_t mid = 0;
+    lfsr_mdir_t mdir = {.mid=0};
-    while (mid < lfsr_mtree_weight(lfs)) {
+    while (mdir.mid < (lfsr_srid_t)lfsr_mtree_weight(lfs)) {
-        lfsr_mdir_t mdir;
+        int err = lfsr_mtree_lookup(lfs, LFSR_MID(lfs, mdir.mid, 0),
-        int err = lfsr_mtree_lookup(lfs, mid,
                 &mdir);
         if (err) {
             LFS_ASSERT(err != LFS_ERR_NOENT);
             return err;
         }
 
-        // clean up orphans
+        while (lfsr_mid_rid(lfs, mdir.mid)
-        err = lfsr_mdir_fixorphans(lfs, &mdir);
+                < (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;
+                }
                 return err;
             }
 
-        // incremend mid unless we dropped the mdir
+            // 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
         if (mdir.rbyd.weight > 0) {
-            mid += 1 << lfs->mdir_bits;
+            mdir.mid = lfsr_mid_bid(lfs, mdir.mid) + 1;
         }
     }
 
@@ -12915,34 +12856,14 @@ int lfsr_traversal_read(lfs_t *lfs, lfsr_traversal_t *t,
         struct lfs_tinfo *tinfo) {
     LFS_ASSERT(lfsr_omdir_isopen(lfs, &t->o.o));
 
-    // check for pending grms every step, just in case some other
+    // mkconsistencing?
-    // operation introduced new grms
+    //
-    if (lfsr_t_ismkconsistent(t->o.o.flags)
+    // check every step in case some other operation introduced a grm/orphan
-            && lfsr_grm_count(lfs) > 0) {
+    if (lfsr_t_ismkconsistent(t->o.o.flags)) {
-        if (lfsr_grm_count(lfs) == 2) {
+        int err = lfsr_fs_mkconsistent(lfs);
-            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) {
@@ -12994,16 +12915,9 @@ 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);
     }
 
@@ -13012,8 +12926,11 @@ 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
-    if (t->o.o.state < LFSR_TSTATE_MDIRS) {
+    } else 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;
@@ -13046,7 +12963,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 & ~LFS_F_MUTATED;
+    t->o.o.flags &= ~LFS_F_DIRTY;
     t->o.o.state = LFSR_TSTATE_MROOTANCHOR;
     t->o.o.mdir.mid = -1;
     t->o.o.mdir.rbyd.weight = 0;

lfs.h

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

tests/test_traversal.toml

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