Github/littlefs
1
Fork 0
Code Actions Activity

trv: Attempted to drop fixgrm calls from lfs3_trv_read

Browse Source 
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%)
This commit is contained in:
Christopher Haster
2025-12-06 13:15:16 -06:00
parent 124afb3034
commit d56cd2c140
2 changed files with 12 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs3.c
-15
View File
@@ -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;
tests/test_trvs.toml
+12 -29
View File
@@ -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);