From 3c2828180957b0a11c0fe3364e9e37f0729dd306 Mon Sep 17 00:00:00 2001
From: Christopher Haster <geky@geky.net>
Date: Wed, 4 Mar 2026 00:45:55 -0600
Subject: [PATCH] trv: Reverted deduplicated mgc related lfs3_fs_fixgrm calls

This fixes a pretty egregious performance regression in bench_wt_many:

  NOR throughput              before       after
  bench_wt_seq+write         29405.7     29405.8 (+0.0%)
  bench_wt_random+write        957.3       957.3 (+0.0%)
  bench_wt_logging+write      2153.4      2153.4 (+0.0%)
  bench_wt_many+write          453.6      6855.4 (+1411.3%)
  bench_rt_seq+read       23939212.7  23939212.8 (+0.0%)
  bench_rt_random+read     6461004.1   6461004.1 (+0.0%)
  bench_rt_logging+read       4163.9      4164.0 (+0.0%)
  bench_rt_many+read        392380.4    392368.5 (-0.0%)

  NAND throughput             before       after
  bench_wt_seq+write         22246.3     22246.3 (+0.0%)
  bench_wt_random+write       3637.2      3637.2 (+0.0%)
  bench_wt_logging+write     10977.0     10976.4 (-0.0%)
  bench_wt_many+write           68.2       448.1 (+557.0%)
  bench_rt_seq+read        2472748.7   2472748.8 (+0.0%)
  bench_rt_random+read      375952.9    375952.9 (+0.0%)
  bench_rt_logging+read      22312.2     22310.9 (-0.0%)
  bench_rt_many+read           860.7       896.8 (+4.2%)

But shows our benchmarks are working! I was very confused why littlefs3
was suddenly performing worse than littlefs2 out of seemingly nowhere.

The fundamental problem is that lfs3_mtree_gc unconditionally traverses
the filesystem. So if we call it every lfs3_fs_mkconsistent call, we
trigger a full filesystem traversal on _every_ write operation.

Not great!

The solution for now is to just revert this change. We need the
lfs3_t_ismkconsistent(lfs3->flags) check to avoid the traversal, and if
we don't call lfs3_mtree_gc we also need a check for
lfs3_grm_count(lfs3) > 0.

Not sure there's a better solution here.

---

Code changes minimal, worth fixing:

           code          stack          ctx
  before: 35256           2136          660
  after:  35292 (+0.1%)   2136 (+0.0%)  660 (+0.0%)
---
 lfs3.c | 76 +++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 25 deletions(-)

diff --git a/lfs3.c b/lfs3.c
index d033387a..e84b19ef 100644
--- a/lfs3.c
+++ b/lfs3.c
@@ -10179,7 +10179,6 @@ eot:;
 }
 
 // needed in lfs3_mtree_gc
-static int lfs3_fs_fixgrm(lfs3_t *lfs3);
 static int lfs3_mdir_mkconsistent(lfs3_t *lfs3, lfs3_mdir_t *mdir);
 static inline void lfs3_alloc_ckpoint_(lfs3_t *lfs3);
 static inline bool lfs3_alloc_canlookahead(const lfs3_t *lfs3);
@@ -10197,22 +10196,6 @@ static int lfs3_alloc_adoptgbmap(lfs3_t *lfs3,
 // mutation here
 static lfs3_stag_t lfs3_mtree_gc(lfs3_t *lfs3, lfs3_mgc_t *mgc,
         lfs3_bptr_t *bptr_) {
-    // check for pending grms every step, just in case some other
-    // operation introduced new grms
-    #ifndef LFS3_RDONLY
-    if (lfs3_t_ismkconsistent(mgc->t.h.flags)
-            && lfs3_grm_count(lfs3) > 0) {
-        // fix pending grms
-        uint32_t dirty = mgc->t.h.flags;
-        int err = lfs3_fs_fixgrm(lfs3);
-        if (err) {
-            return err;
-        }
-        // reset dirty flag
-        mgc->t.h.flags &= ~LFS3_t_DIRTY | dirty;
-    }
-    #endif
-
     // start of traversal?
     if (mgc->t.h.mdir.mid == LFS3_MID_MROOTANCHOR) {
         #ifndef LFS3_RDONLY
@@ -11709,6 +11692,9 @@ empty:;
 }
 #endif
 
+// needed in lfs3_remove
+static int lfs3_fs_fixgrm(lfs3_t *lfs3);
+
 #ifndef LFS3_RDONLY
 int lfs3_remove(lfs3_t *lfs3, const char *path) {
     // prepare our filesystem for writing
@@ -16591,15 +16577,9 @@ failed:;
 }
 #endif
 
-// prepare the filesystem for mutation
 #ifndef LFS3_RDONLY
-int lfs3_fs_mkconsistent(lfs3_t *lfs3) {
-    // filesystem must be writeable
-    LFS3_ASSERT(!lfs3_m_isrdonly(lfs3->flags));
-
-    // LFS3_T_MKCONSISTENT does most of the work:
-    // 1. fixes pending grms
-    // 2. fixes orphaned stickynotes
+static int lfs3_fs_fixorphans(lfs3_t *lfs3) {
+    // LFS3_T_MKCONSISTENT really just removes orphans
     lfs3_mgc_t mgc;
     lfs3_mgc_init(&mgc,
             LFS3_T_RDWR | LFS3_T_MTREEONLY | LFS3_T_MKCONSISTENT);
@@ -16615,6 +16595,36 @@ int lfs3_fs_mkconsistent(lfs3_t *lfs3) {
         }
     }
 
+    return 0;
+}
+#endif
+
+// prepare the filesystem for mutation
+#ifndef LFS3_RDONLY
+int lfs3_fs_mkconsistent(lfs3_t *lfs3) {
+    // filesystem must be writeable
+    LFS3_ASSERT(!lfs3_m_isrdonly(lfs3->flags));
+
+    // fix pending grms
+    if (lfs3_grm_count(lfs3) > 0) {
+        int err = lfs3_fs_fixgrm(lfs3);
+        if (err) {
+            return err;
+        }
+    }
+
+    // fix orphaned stickynotes
+    //
+    // this must happen after fixgrm, since removing orphaned
+    // stickynotes risks outdating the grm
+    //
+    if (lfs3_t_ismkconsistent(lfs3->flags)) {
+        int err = lfs3_fs_fixorphans(lfs3);
+        if (err) {
+            return err;
+        }
+    }
+
     // go ahead and checkpoint the allocator
     //
     // this isn't always needed, but redundant alloc ckpoints are noops,
@@ -17149,6 +17159,22 @@ int lfs3_trv_read(lfs3_t *lfs3, lfs3_trv_t *trv,
         return LFS3_ERR_BUSY;
     }
 
+    // check for pending grms every step, just in case some other
+    // operation introduced new grms
+    #ifndef LFS3_RDONLY
+    if (lfs3_t_ismkconsistent(trv->gc.t.h.flags)
+            && lfs3_grm_count(lfs3) > 0) {
+        // fix pending grms
+        uint32_t dirty = trv->gc.t.h.flags;
+        int err = lfs3_fs_fixgrm(lfs3);
+        if (err) {
+            return err;
+        }
+        // reset dirty flag
+        trv->gc.t.h.flags &= ~LFS3_t_DIRTY | dirty;
+    }
+    #endif
+
     // discard current block queue?
     if (lfs3_t_isstale(trv->gc.t.h.flags)) {
         trv->blocks[0] = -1;