From 8a58954828ee79ac21c9c9effe996b647989b8d8 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 20 Oct 2025 15:35:03 -0500 Subject: [PATCH] trv: Reduced LFS3_t_CKPOINTED + LFS3_t_MUTATED -> LFS3_t_CKPOINTED This drops LFS3_t_MUTATED in favor of just using LFS3_t_CKPOINTED everywhere: 1. These meant roughly the same thing, with LFS3_t_MUTATED being a bit tighter at the cost of needing to be explicitly set. 2. The implicit setting of LFS3_t_CKPOINTED by lfs3_alloc_ckpoint -- a function that already needs to be called before mutation -- means we have one less thing to worry about. Implicit properties like LFS3_t_CKPOINTED are great for building a reliable system. Manual flags like LFS3_t_MUTATED, not so much. 3. Why use two flags when we can get away with one? The only downside is we may unnecessarily clobber gc/traversal work when we don't actually mutate the filesystem. Failed file open calls are a good example. However this tradeoff seems well worth it for an overall simpler + more reliable system. --- Saves a bit of code: code stack ctx before: 37220 2352 688 after: 37160 (-0.2%) 2352 (+0.0%) 688 (+0.0%) code stack ctx gbmap before: 40184 2368 856 gbmap after: 40132 (-0.1%) 2368 (+0.0%) 856 (+0.0%) --- lfs3.c | 58 ++++++++++---------------------------------- lfs3.h | 5 ++-- scripts/dbgflags.py | 5 ++-- tests/test_trvs.toml | 7 ++++-- 4 files changed, 22 insertions(+), 53 deletions(-) diff --git a/lfs3.c b/lfs3.c index fc9164f6..cfc6136f 100644 --- a/lfs3.c +++ b/lfs3.c @@ -7476,10 +7476,6 @@ static inline bool lfs3_t_isdirty(uint32_t flags) { return flags & LFS3_t_DIRTY; } -static inline bool lfs3_t_ismutated(uint32_t flags) { - return flags & LFS3_t_MUTATED; -} - static inline bool lfs3_t_isckpointed(uint32_t flags) { return flags & LFS3_t_CKPOINTED; } @@ -7668,27 +7664,6 @@ static void lfs3_handle_clobber(lfs3_t *lfs3, const lfs3_handle_t *h) { } #endif -// mark all traversals as mutated -#ifndef LFS3_RDONLY -static void lfs3_fs_mutate(lfs3_t *lfs3) { - for (lfs3_handle_t *h_ = lfs3->handles; h_; h_ = h_->next) { - if (lfs3_o_type(h_->flags) == LFS3_type_TRV) { - // mark as dirty + mutated, self-mutating traversals should - // clear the dirty bit - h_->flags |= LFS3_t_DIRTY | LFS3_t_MUTATED; - } - } -} -#endif - -// mark as mutated and clobber any traversals referencing a handle -#ifndef LFS3_RDONLY -static void lfs3_handle_mutate(lfs3_t *lfs3, const lfs3_handle_t *h) { - lfs3_handle_clobber(lfs3, h); - lfs3_fs_mutate(lfs3); -} -#endif - /// Global-state things /// @@ -9783,9 +9758,6 @@ static int lfs3_mdir_commit_(lfs3_t *lfs3, lfs3_mdir_t *mdir, // update any gstate changes lfs3_fs_commitgdelta(lfs3); - // mark all traversals as mutated - lfs3_fs_mutate(lfs3); - // we may have touched any number of mdirs, so assume uncompacted // until lfs3_fs_gc can prove otherwise lfs3->flags |= LFS3_I_COMPACTMETA; @@ -10595,8 +10567,7 @@ eot:; // compare gcksum with in-RAM gcksum if ((lfs3_t_isckmeta(mtrv->b.h.flags) || lfs3_t_isckdata(mtrv->b.h.flags)) - && !lfs3_t_isdirty(mtrv->b.h.flags) - && !lfs3_t_ismutated(mtrv->b.h.flags) + && !lfs3_t_isckpointed(mtrv->b.h.flags) && mtrv->gcksum != lfs3->gcksum) { LFS3_ERROR("Found gcksum mismatch, cksum %08"PRIx32" (!= %08"PRIx32")", mtrv->gcksum, @@ -10609,14 +10580,12 @@ eot:; if ((lfs3_t_isckmeta(mtrv->b.h.flags) || lfs3_t_isckdata(mtrv->b.h.flags)) && !lfs3_t_ismtreeonly(mtrv->b.h.flags) - && !lfs3_t_isdirty(mtrv->b.h.flags) - && !lfs3_t_ismutated(mtrv->b.h.flags)) { + && !lfs3_t_isckpointed(mtrv->b.h.flags)) { lfs3->flags &= ~LFS3_I_CKMETA; } if (lfs3_t_isckdata(mtrv->b.h.flags) && !lfs3_t_ismtreeonly(mtrv->b.h.flags) - && !lfs3_t_isdirty(mtrv->b.h.flags) - && !lfs3_t_ismutated(mtrv->b.h.flags)) { + && !lfs3_t_isckpointed(mtrv->b.h.flags)) { lfs3->flags &= ~LFS3_I_CKDATA; } @@ -10650,7 +10619,7 @@ static lfs3_stag_t lfs3_mtree_gc(lfs3_t *lfs3, lfs3_mgc_t *mgc, && !lfs3_t_isckpointed(mgc->t.b.h.flags)) { lfs3_alloc_ckpoint_(lfs3); // keep our own ckpointed flag clear - mgc->t.b.h.flags &= ~LFS3_t_CKPOINTED; + mgc->t.b.h.flags &= ~LFS3_t_CKPOINTED & ~LFS3_t_DIRTY; } #endif @@ -10687,7 +10656,7 @@ static lfs3_stag_t lfs3_mtree_gc(lfs3_t *lfs3, lfs3_mgc_t *mgc, } // keep our own ckpointed flag clear - mgc->t.b.h.flags &= ~LFS3_t_CKPOINTED; + mgc->t.b.h.flags &= ~LFS3_t_CKPOINTED & ~LFS3_t_DIRTY; } #endif } @@ -10825,7 +10794,7 @@ eot:; // was compaction successful? note we may need multiple passes if // we want to be sure everything is compacted if (lfs3_t_compactmeta(mgc->t.b.h.flags) - && !lfs3_t_ismutated(mgc->t.b.h.flags)) { + && !lfs3_t_isckpointed(mgc->t.b.h.flags)) { lfs3->flags &= ~LFS3_I_COMPACTMETA; } #endif @@ -11113,11 +11082,11 @@ static inline void lfs3_alloc_ckpoint_(lfs3_t *lfs3) { // set ckpoint = disk size lfs3->lookahead.ckpoint = lfs3->block_count; - // mark all traversals as ckpointed - // TODO should this be a function? non-clobbering? + // mark all traversals as ckpointed + dirty, self-mutating + // traversals should clear the dirty bit for (lfs3_handle_t *h = lfs3->handles; h; h = h->next) { if (lfs3_o_type(h->flags) == LFS3_type_TRV) { - h->flags |= LFS3_t_CKPOINTED; + h->flags |= LFS3_t_CKPOINTED | LFS3_t_DIRTY; } } } @@ -14395,7 +14364,7 @@ lfs3_ssize_t lfs3_file_write(lfs3_t *lfs3, lfs3_file_t *file, } // clobber entangled traversals - lfs3_handle_mutate(lfs3, &file->b.h); + lfs3_handle_clobber(lfs3, &file->b.h); // mark as unsynced in case we fail file->b.h.flags |= LFS3_o_UNSYNC; @@ -14539,7 +14508,7 @@ int lfs3_file_flush(lfs3_t *lfs3, lfs3_file_t *file) { #ifndef LFS3_RDONLY // clobber entangled traversals - lfs3_handle_mutate(lfs3, &file->b.h); + lfs3_handle_clobber(lfs3, &file->b.h); int err; // flush our cache @@ -15045,7 +15014,7 @@ int lfs3_file_truncate(lfs3_t *lfs3, lfs3_file_t *file, lfs3_off_t size_) { } // clobber entangled traversals - lfs3_handle_mutate(lfs3, &file->b.h); + lfs3_handle_clobber(lfs3, &file->b.h); // mark as unsynced in case we fail file->b.h.flags |= LFS3_o_UNSYNC; @@ -15138,7 +15107,7 @@ int lfs3_file_fruncate(lfs3_t *lfs3, lfs3_file_t *file, lfs3_off_t size_) { } // clobber entangled traversals - lfs3_handle_mutate(lfs3, &file->b.h); + lfs3_handle_clobber(lfs3, &file->b.h); // mark as unsynced in case we fail file->b.h.flags |= LFS3_o_UNSYNC; @@ -17560,7 +17529,6 @@ static int lfs3_trv_rewind_(lfs3_t *lfs3, lfs3_trv_t *trv) { lfs3_mgc_init(&trv->gc, trv->gc.t.b.h.flags & ~LFS3_t_DIRTY - & ~LFS3_t_MUTATED & ~LFS3_t_CKPOINTED & ~LFS3_t_TSTATE); diff --git a/lfs3.h b/lfs3.h index 447979c0..49c48f57 100644 --- a/lfs3.h +++ b/lfs3.h @@ -344,10 +344,9 @@ enum lfs3_btype { #define LFS3_t_TSTATE 0x000f0000 // The current traversal state #define LFS3_t_BTYPE 0x00f00000 // The current block type #define LFS3_t_ZOMBIE 0x08000000 // File has been removed -#define LFS3_t_DIRTY 0x04000000 // Filesystem modified outside traversal -#define LFS3_t_MUTATED 0x02000000 // Filesystem modified during traversal +#define LFS3_t_DIRTY 0x04000000 // Filesystem ckpointed outside traversal #define LFS3_t_CKPOINTED \ - 0x01000000 // Filesystem ckpointed during traversal + 0x02000000 // Filesystem ckpointed during traversal // GC flags #ifndef LFS3_RDONLY diff --git a/scripts/dbgflags.py b/scripts/dbgflags.py index d0357591..d67b17f0 100755 --- a/scripts/dbgflags.py +++ b/scripts/dbgflags.py @@ -170,9 +170,8 @@ FLAGS = [ ('^_BTREE', 0x00200000, "Btype = btree" ), ('^_DATA', 0x00300000, "Btype = data" ), ('t_ZOMBIE', 0x08000000, "File has been removed" ), - ('t_DIRTY', 0x04000000, "Filesystem modified outside traversal" ), - ('t_MUTATED', 0x02000000, "Filesystem modified during traversal" ), - ('t_CKPOINTED', 0x01000000, "Filesystem ckpointed during traversal" ), + ('t_DIRTY', 0x04000000, "Filesystem ckpointed outside traversal" ), + ('t_CKPOINTED', 0x02000000, "Filesystem ckpointed during traversal" ), # Block allocator flags ('alloc_ERASE', 0x00000001, "Please erase the block" ), diff --git a/tests/test_trvs.toml b/tests/test_trvs.toml index 3cd2aa2f..cb102995 100644 --- a/tests/test_trvs.toml +++ b/tests/test_trvs.toml @@ -2360,8 +2360,11 @@ code = ''' ? LFS3_IFDEF_GBMAP(LFS3_I_REPOPGBMAP, -1) : 0) | LFS3_I_COMPACTMETA - | LFS3_I_CKMETA - | LFS3_I_CKDATA + // and maybe ckmeta/ckdata, note ckdata implies ckmeta + | (((!CKMETA && !CKDATA) || SIZE > FILE_CACHE_SIZE || SYNC) + ? LFS3_I_CKMETA : 0) + | ((!CKDATA || SIZE > FILE_CACHE_SIZE || SYNC) + ? LFS3_I_CKDATA : 0) | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_I_GBMAP, -1) : 0))); // check the file contents