From c58a48c02e2f0b67b829cd089ece701da96d635f Mon Sep 17 00:00:00 2001
From: Christopher Haster <geky@geky.net>
Date: Wed, 17 Jul 2024 15:38:36 -0500
Subject: [PATCH] gc: Consider ckmeta/ckdata successful even if we mutated the
 filesystem

Also moved ckmeta/ckdata progress into lfs->flags. We have the bits
available so we might as well use them instead of allocating bools on
the stack...

Whether or not to consider ckmeta/ckdata successful when the filesystem
has been mutated is a bit nuanced.

Initially, I thought we trigger a re-traversal, since we may have
introduced new blocks that haven't been checked. But think about it,
where did those blocks come from?

Any new blocks introduced by filesystem mutation will have just been
written. And if a write introduces corruption you probably have bigger
problems...

... Actually as I write this I realized mounting without ckprogs makes
this even more nuanced, but since ckmeta/ckdata is more intended for
data-at-rest error detection I'm going to keep the change for now.

If you want to catch write errors, you really should enable ckprogs.
This is only a problem for lfsr_fs_gc, and the use cases for
ckmeta/ckdata in lfsr_fs_gc will probably catch any write errors on the
next cycle anyways...

Code changes:

           code          stack
  before: 36208           2680
  after:  36244 (+0.1%)   2680 (+0.0%)
---
 lfs.c | 24 ++++++++++++++++++------
 lfs.h |  2 ++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/lfs.c b/lfs.c
index 1bac2b45..025f6a8d 100644
--- a/lfs.c
+++ b/lfs.c
@@ -5924,6 +5924,14 @@ static inline bool lfsr_f_hasorphans(uint32_t flags) {
     return flags & LFS_F_ORPHANS;
 }
 
+static inline bool lfsr_f_hasckedmeta(uint32_t flags) {
+    return flags & LFS_F_CKEDMETA;
+}
+
+static inline bool lfsr_f_hasckeddata(uint32_t flags) {
+    return flags & LFS_F_CKEDDATA;
+}
+
 // on-demand flags
 
 // needed in lfsr_fs_isinconsistent
@@ -13061,8 +13069,11 @@ int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags) {
         }
     }
 
+    // every time we call lfsr_fs_gc with ckmeta/ckdata, assume we want
+    // a new traversal
+    lfs->flags &= ~(LFS_F_CKEDMETA | LFS_F_CKEDDATA);
+
     // do we have any pending work?
-    bool cked = false;
     while ((lfs_off_t)steps > 0
             && ((lfsr_t_ismkconsistent(flags)
                     && lfsr_f_hasorphans(lfs->flags))
@@ -13071,9 +13082,9 @@ int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags) {
                 || (lfsr_t_iscompact(flags)
                     && lfsr_i_isuncompacted(lfs->flags))
                 || (lfsr_t_isckmeta(flags)
-                    && !cked)
+                    && !lfsr_f_hasckedmeta(lfs->flags))
                 || (lfsr_t_isckdata(flags)
-                    && !cked))) {
+                    && !lfsr_f_hasckeddata(lfs->flags)))) {
         // checkpoint the allocator to maximize any lookahead scans
         lfs_alloc_ckpoint(lfs);
 
@@ -13106,9 +13117,10 @@ int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags) {
         if (err == LFS_ERR_NOENT) {
             lfsr_omdir_close(lfs, &lfs->gc.o.o);
 
-            // consider our filesystem checked if we completed a traversal
-            // with no mutation
-            cked = !lfsr_f_ismutated(lfs->gc.o.o.flags);
+            // consider our filesystem checked if we complete at least
+            // one traversal
+            lfs->flags |= ((lfsr_t_isckmeta(flags)) ? LFS_F_CKEDMETA : 0)
+                    | ((lfsr_t_isckdata(flags)) ? LFS_F_CKEDDATA : 0);
         }
 
         // decrement steps
diff --git a/lfs.h b/lfs.h
index ed5ed342..8a30cadf 100644
--- a/lfs.h
+++ b/lfs.h
@@ -140,6 +140,8 @@ enum lfs_fsinfo_flags {
 
     // internally used flags
     LFS_F_ORPHANS       = 0x8000, // Filesystem may have untracked orphans
+    LFS_F_CKEDMETA      = 0x2000, // Filesystem metadata checked during gc
+    LFS_F_CKEDDATA      = 0x4000, // Filesystem data checked during gc
 };
 
 // File types