From d56cd2c1406c89cb4538a715f068eefac39de147 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 6 Dec 2025 13:15:16 -0600 Subject: [PATCH] trv: Attempted to drop fixgrm calls from lfs3_trv_read Will revert. The idea here is that fixgrm isn't really a traversal operation. It's convenient, but in an effort to simplify things, dropping fixgrm from lfs3_trv_read makes sense. But dropping fixgrm seems to cause more problems than it's worth. --- Note test_trvs is currently failing because attempting to remove an orphaned stickynote in the grm queue without calling fixgrm breaks things. It's probably fixable, but why? If we keep the implied fixgrm it's not possible to trigger a remove without a clean grm queue. And we want to keep our grm queue clean anyways to prevent a full fixorphan scan. Code changes: code stack ctx before: 35164 2136 660 after: 35112 (-0.1%) 2136 (+0.0%) 660 (+0.0%) code stack ctx gbmap before: 38088 2136 776 gbmap after: 38040 (-0.1%) 2136 (+0.0%) 776 (+0.0%) --- lfs3.c | 15 --------------- tests/test_trvs.toml | 41 ++++++++++++----------------------------- 2 files changed, 12 insertions(+), 44 deletions(-) diff --git a/lfs3.c b/lfs3.c index 62229c6d..cd848b93 100644 --- a/lfs3.c +++ b/lfs3.c @@ -16812,21 +16812,6 @@ 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) { - 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; diff --git a/tests/test_trvs.toml b/tests/test_trvs.toml index c274f3e2..c249c202 100644 --- a/tests/test_trvs.toml +++ b/tests/test_trvs.toml @@ -7611,29 +7611,28 @@ code = ''' lfs3_trv_read(&lfs3, &trv, &tinfo) => LFS3_ERR_NOENT; lfs3_trv_close(&lfs3, &trv) => 0; - // we should have cleaned up all grms/orphans - assert(lfs3.grm.queue[0] == 0); - assert(lfs3.grm.queue[1] == 0); + // we should have cleaned up all orphans (maybe not grms) assert(!(lfs3.flags & LFS3_I_MKCONSISTENT)); // which means there shouldn't be that many files left assert(lfs3.mtree.r.weight <= (2 << lfs3.mbits)); - assert(file1.b.h.mdir.r.weight <= 3); - assert(file2.b.h.mdir.r.weight <= 3); + assert(file1.b.h.mdir.r.weight <= 4); + assert(file2.b.h.mdir.r.weight <= 4); // and we should be marked as consistent lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( LFS3_IFDEF_YES_REVDBG(LFS3_I_REVDBG, 0) | LFS3_IFDEF_YES_REVNOISE(LFS3_I_REVNOISE, 0) - | (((!LOOKAHEAD || ORPHANS > 0) + | ((ORPHANS > 0) ? LFS3_I_MKCONSISTENT : 0) + | (((!LOOKAHEAD || ORPHANS >= 100) && (!GBMAP || ORPHANS >= 100)) ? LFS3_I_LOOKAHEAD : 0) | LFS3_I_COMPACT // note ckdata implies ckmeta - | (((!CKMETA && !CKDATA) || ORPHANS > 0) ? LFS3_I_CKMETA : 0) - | ((!CKDATA || ORPHANS > 0) ? LFS3_I_CKDATA : 0) + | (((!CKMETA && !CKDATA) || ORPHANS >= 100) ? LFS3_I_CKMETA : 0) + | ((!CKDATA || ORPHANS >= 100) ? LFS3_I_CKDATA : 0) | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_I_GBMAP, -1) : 0))); // check we can still read the files @@ -7783,9 +7782,6 @@ code = ''' lfs3_trv_read(&lfs3, &trv, &tinfo) => LFS3_ERR_NOENT; lfs3_trv_close(&lfs3, &trv) => 0; - // we should be able to clean up grms - assert(lfs3.grm.queue[0] == 0); - assert(lfs3.grm.queue[1] == 0); // if we introduce actual orphans, me _must not_ clear the orphan flag if (ORPHANS >= 3) { assert(lfs3.flags & LFS3_I_MKCONSISTENT); @@ -7970,9 +7966,7 @@ code = ''' lfs3_trv_read(&lfs3, &trv, &tinfo) => LFS3_ERR_NOENT; lfs3_trv_close(&lfs3, &trv) => 0; - // we should have cleaned up all grms/orphans - assert(lfs3.grm.queue[0] == 0); - assert(lfs3.grm.queue[1] == 0); + // we should have cleaned up all orphans (maybe not grms) assert(!(lfs3.flags & LFS3_I_MKCONSISTENT)); // which means there shouldn't be that many files left @@ -8155,9 +8149,7 @@ code = ''' lfs3_trv_read(&lfs3, &trv, &tinfo) => LFS3_ERR_NOENT; lfs3_trv_close(&lfs3, &trv) => 0; - // we should have cleaned up all grms/orphans - assert(lfs3.grm.queue[0] == 0); - assert(lfs3.grm.queue[1] == 0); + // we should have cleaned up all orphans (maybe not grms) assert(!(lfs3.flags & LFS3_I_MKCONSISTENT)); // which means there shouldn't be that many files left @@ -8349,9 +8341,7 @@ code = ''' lfs3_trv_read(&lfs3, &trv, &tinfo) => LFS3_ERR_NOENT; lfs3_trv_close(&lfs3, &trv) => 0; - // we should have cleaned up all grms/orphans - assert(lfs3.grm.queue[0] == 0); - assert(lfs3.grm.queue[1] == 0); + // we should have cleaned up all orphans (maybe not grms) assert(!(lfs3.flags & LFS3_I_MKCONSISTENT)); // which means there shouldn't be that many files left @@ -8543,9 +8533,7 @@ code = ''' lfs3_trv_read(&lfs3, &trv, &tinfo) => LFS3_ERR_NOENT; lfs3_trv_close(&lfs3, &trv) => 0; - // we should have cleaned up all grms/orphans - assert(lfs3.grm.queue[0] == 0); - assert(lfs3.grm.queue[1] == 0); + // we should have cleaned up all orphans (maybe not grms) assert(!(lfs3.flags & LFS3_I_MKCONSISTENT)); // which means there shouldn't be that many files left @@ -8736,9 +8724,7 @@ code = ''' } lfs3_trv_read(&lfs3, &trv, &tinfo) => LFS3_ERR_NOENT; - // we should have cleaned up all grms/orphans - assert(lfs3.grm.queue[0] == 0); - assert(lfs3.grm.queue[1] == 0); + // we should have cleaned up all orphans (maybe not grms) assert(!(lfs3.flags & LFS3_I_MKCONSISTENT)); // which means there shouldn't be that many files left @@ -8963,9 +8949,6 @@ code = ''' lfs3_trv_read(&lfs3, &trv, &tinfo) => LFS3_ERR_NOENT; lfs3_trv_close(&lfs3, &trv) => 0; - // we should be able to clean up grms - assert(lfs3.grm.queue[0] == 0); - assert(lfs3.grm.queue[1] == 0); // if we introduce actual orphans, me _must not_ clear the orphan flag if (ORPHANS >= 3) { assert(lfs3.flags & LFS3_I_MKCONSISTENT);