Github/littlefs
1
Fork 0
Code Actions Activity

Reverted gc-restart on flag change

Browse Source 
Thinking about this more, we probably don't want to entangle
lfsr_fs_mkconsistent/ckmeta/etc and lfsr_fs_gc:

- lfsr_fs_ckmeta/ckdata are readonly and don't need to clobber
  traversals. The system can make more progress if these use separate
  states.

- We already need a bit of code to force traversals to restart for
  lfsr_fs_ckmeta/ckdata, so these already aren't simple wrappers.

- lfsr_fs_mkconsistent should also probably not invalidate gc traversals
  when the filesystem is already consistent. It is called by... checks
  notes... every function that writes to disk.

  This could be fixed in lfsr_fs_mkconsistent, but it'd be pretty close
  to just calling lfsr_mtree_gc...

- We don't really benefit from reusing the gc traversal state.
  lfsr_fs_mkconsistent/ckmeta/etc aren't on the stack hot-path, so the
  stack usage is more-or-less free (though I realize this depends on
  what functions are called in a given system).

- Calling lfsr_fs_gc can actually be a detriment for code size when
  considering link-time-gc (not related to fs-gc), since it will drag in
  the function when we don't need the traversal-invalidation features.

- Calling lfsr_fs_gc vs lfsr_mtree_gc shouldn't really be a significant
  code size difference. We should probably look into lfsr_mtree_gc,
  which is called from many places, instead of tangling everything
  together...

So this commit reverts gc-restarts and brings back gc masking on flag
change.

At the very least, moving all the code around led to a bit of code
savings:

                      code          stack
  before gc-restart: 36316           2680
  gc-restart:        36068 (-0.7%)   2680 (+0.0%)
  after gc-restart:  36240 (-0.2%)   2680 (+0.0%)
This commit is contained in:
Christopher Haster
2024-07-17 03:09:17 -05:00
parent 1cd6a6873a
commit 6cf78527b4
2 changed files with 650 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+90 -24
View File
@@ -12928,32 +12928,97 @@ failed:;
return err;
}
static int lfsr_fs_fixorphans(lfs_t *lfs) {
// LFS_T_MKCONSISTENT really just removes orphans
lfsr_traversal_t t = LFSR_TRAVERSAL(
LFS_T_MTREEONLY | LFS_T_MKCONSISTENT);
while (true) {
int err = lfsr_mtree_gc(lfs, &t,
NULL, NULL);
if (err) {
if (err == LFS_ERR_NOENT) {
break;
}
return err;
}
}
return 0;
}
// prepare the filesystem for mutation
int lfsr_fs_mkconsistent(lfs_t *lfs) {
// leave this up to lfsr_fs_gc
return lfsr_fs_gc(lfs, -1, LFS_GC_MTREEONLY | LFS_GC_MKCONSISTENT);
// fix pending grms
if (lfsr_grm_count(lfs) > 0) {
if (lfsr_grm_count(lfs) == 2) {
LFS_DEBUG("Fixing grm %"PRId32".%"PRId32" %"PRId32".%"PRId32,
lfsr_mid_bid(lfs, lfs->grm.mids[0]) >> lfs->mdir_bits,
lfsr_mid_rid(lfs, lfs->grm.mids[0]),
lfsr_mid_bid(lfs, lfs->grm.mids[1]) >> lfs->mdir_bits,
lfsr_mid_rid(lfs, lfs->grm.mids[1]));
} else if (lfsr_grm_count(lfs) == 1) {
LFS_DEBUG("Fixing grm %"PRId32".%"PRId32,
lfsr_mid_bid(lfs, lfs->grm.mids[0]) >> lfs->mdir_bits,
lfsr_mid_rid(lfs, lfs->grm.mids[0]));
}
int err = lfsr_fs_fixgrm(lfs);
if (err) {
return err;
}
}
// fix orphaned files
//
// this must happen after fixgrm, since removing orphaned files risks
// outdating the grm
//
if (lfsr_f_hasorphans(lfs->flags)) {
LFS_DEBUG("Fixing orphans...");
int err = lfsr_fs_fixorphans(lfs);
if (err) {
return err;
}
}
return 0;
}
// check the filesystem for metadata errors
int lfsr_fs_ckmeta(lfs_t *lfs) {
// we want a full traversal, so make sure no gc is currently running
if (lfsr_omdir_isopen(lfs, &lfs->gc.o.o)) {
lfsr_omdir_close(lfs, &lfs->gc.o.o);
// we leave this up to lfsr_mtree_gc
lfsr_traversal_t t = LFSR_TRAVERSAL(LFS_T_CKMETA);
while (true) {
int err = lfsr_mtree_gc(lfs, &t,
NULL, NULL);
if (err) {
if (err == LFS_ERR_NOENT) {
break;
}
return err;
}
}
// leave this up to lfsr_fs_gc
return lfsr_fs_gc(lfs, -1, LFS_GC_CKMETA);
return 0;
}
// check the filesystem for metadata + data errors
int lfsr_fs_ckdata(lfs_t *lfs) {
// we want a full traversal, so make sure no gc is currently running
if (lfsr_omdir_isopen(lfs, &lfs->gc.o.o)) {
lfsr_omdir_close(lfs, &lfs->gc.o.o);
// we leave this up to lfsr_mtree_gc
lfsr_traversal_t t = LFSR_TRAVERSAL(LFS_T_CKMETA | LFS_T_CKDATA);
while (true) {
int err = lfsr_mtree_gc(lfs, &t,
NULL, NULL);
if (err) {
if (err == LFS_ERR_NOENT) {
break;
}
return err;
}
}
// leave this up to lfsr_fs_gc
return lfsr_fs_gc(lfs, -1, LFS_GC_CKMETA | LFS_GC_CKDATA);
return 0;
}
// perform any pending janitorial work
@@ -13015,22 +13080,23 @@ int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags) {
// checkpoint the allocator to maximize any lookahead scans
lfs_alloc_ckpoint(lfs);
// flags mismatch? restart traversal
if (lfsr_omdir_isopen(lfs, &lfs->gc.o.o)
&& (flags != (lfs->gc.o.o.flags & (
LFS_T_MTREEONLY
| LFS_T_MKCONSISTENT
| LFS_T_LOOKAHEAD
| LFS_T_COMPACT
| LFS_T_CKMETA
| LFS_T_CKDATA)))) {
lfsr_omdir_close(lfs, &lfs->gc.o.o);
}
// start a new traversal?
if (!lfsr_omdir_isopen(lfs, &lfs->gc.o.o)) {
lfs->gc = LFSR_TRAVERSAL(flags);
lfsr_omdir_open(lfs, &lfs->gc.o.o);
// existing traversal?
} else {
// note that we mask flags (except mtreeonly)! if you change flags
// mid-traversal, the result is equivalent to the worst-case set
// of flags
lfs->gc.o.o.flags &= ~(
LFS_GC_MKCONSISTENT
| LFS_GC_LOOKAHEAD
| LFS_GC_COMPACT
| LFS_GC_CKMETA
| LFS_GC_CKDATA
) | flags;
}
// progress gc