Github/littlefs
1
Fork 0
Code Actions Activity

trv: Added test_gc_nospc, fixed pcache bug and trv-repop-conflict bug

Browse Source 
This adds test_gc_nospc with more aggressive testing of gc/traversal
operations in low-space conditions. The original intention was to test
the new soft-ENOSPC traversal behavior, but instead it found a couple
unrelated bugs.

In my defense these involve some rather subtle filesystem interactions
and went unnoticed because we don't usually check data checksums:

1. lfs3_bd_flush had a rare chance where it could corrupt our
   prog-aligned pcksum when (1) we bypass the pcache, allowing any
   previous contents to stay there until flush/pcksum, and (2) some
   other failed prog, in this case failing repopgbmaps due to the
   low-space condition, leaves garbage in the pcache. When we flush
   we corrupt the pcksum even though the old data belongs to an
   unrelated block.

   This resulted in CKDATA failing, though the failed check is a false
   positive.

   As a workaround, lfs3_bd_prog and lfs3_bd_prognext now discard _any_
   unrelated pcache, even if bypassing the pcache. This should ensure
   consistent behavior in all cases. Note we do something similar for
   with the file cache in lfs3_file_write.

   This means progs may not complete unless lfs3_bd_flush is called, but
   I think we need to call lfs3_bd_flush in all cases anyways to ensure
   power-loss safe behavior.

   The end result should be a more reliable internal bd prog API.

2. On a successful traversal with LFS3_T_REPOPLOOKAHEAD and
   LFS3_T_REPOPGBMAP we adopt both the new gbmap and lookahead buffer.

   This is wrong! The lookahead buffer is not aware of the gbmap during
   the traversal, and _can't_ be aware as the gbmap changes during
   repopulation work. This is the whole reason we have the alloc
   ckpoints and the in-flight window.

   To fix, adopting the lookahead buffer is now conditional on _not_
   adopting a new gbmap.

   It makes the code a bit more messy, but this is the correct behavior.
   Populating both the gbmap and lookahead buffere requires at least two
   passes.

Code changes minimal:

                 code          stack          ctx
  before:       37248           2352          688
  after:        37260 (+0.0%)   2352 (+0.0%)  688 (+0.0%)

                 code          stack          ctx
  gbmap before: 40204           2368          856
  gbmap after:  40220 (+0.0%)   2368 (+0.0%)  856 (+0.0%)
This commit is contained in:
Christopher Haster
2025-10-19 13:33:04 -05:00
parent f892d299dd
commit 9e4bbdf0ad
2 changed files with 138 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs3.c
+35 -18
View File
@@ -410,8 +410,14 @@ static int lfs3_bd_prognext(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
while (true) {
// active pcache?
if (lfs3->pcache.block == block
&& lfs3->pcache.size != 0) {
if (lfs3->pcache.size != 0) {
// wait, wrong block? this must be a leftover pcache due to
// an error, discard
if (lfs3->pcache.block != block) {
lfs3_bd_droppcache(lfs3);
continue;
}
// fits in pcache?
if (off < lfs3->pcache.off + lfs3->cfg->pcache_size) {
// you can't prog backwards silly
@@ -465,8 +471,14 @@ static int lfs3_bd_prog(lfs3_t *lfs3, lfs3_block_t block, lfs3_size_t off,
lfs3_size_t size_ = size;
while (size_ > 0) {
// active pcache?
if (lfs3->pcache.block == block
&& lfs3->pcache.size != 0) {
if (lfs3->pcache.size != 0) {
// wait, wrong block? this must be a leftover pcache due to
// an error, discard
if (lfs3->pcache.block != block) {
lfs3_bd_droppcache(lfs3);
continue;
}
// fits in pcache?
if (off_ < lfs3->pcache.off + lfs3->cfg->pcache_size) {
// you can't prog backwards silly
@@ -10817,27 +10829,32 @@ dropped:;
eot:;
#ifndef LFS3_RDONLY
// was lookahead scan successful?
#ifndef LFS3_2BONLY
if (lfs3_t_isrepoplookahead(mgc->t.b.h.flags)
&& !lfs3_t_ismtreeonly(mgc->t.b.h.flags)
&& !lfs3_t_isckpointed(mgc->t.b.h.flags)) {
lfs3_alloc_adopt(lfs3, lfs3->lookahead.ckpoint);
}
#endif
// was gbmap repop successful?
#ifdef LFS3_GBMAP
if (lfs3_t_isrepopgbmap(mgc->t.b.h.flags)
// was gbmap scan successful?
//
// this is structured this way because only one repopulation
// scan can succeed at a time, if gbmap succeeds it invalidates the
// lookahead scan with the new gbmap
//
// gbmap takes priority because it actually writes to disk
if (LFS3_IFDEF_GBMAP(
lfs3_t_isrepopgbmap(mgc->t.b.h.flags)
&& lfs3_f_isgbmap(lfs3->flags)
&& lfs3_t_isrepopgbmap(lfs3->flags)
&& !lfs3_t_ismtreeonly(mgc->t.b.h.flags)
&& !lfs3_t_isckpointed(mgc->t.b.h.flags)
&& !lfs3_t_isnospc(mgc->t.b.h.flags)) {
&& !lfs3_t_isnospc(mgc->t.b.h.flags),
false)) {
#ifdef LFS3_GBMAP
lfs3_alloc_adoptgbmap(lfs3, &mgc->gbmap_, lfs3->lookahead.ckpoint);
}
#endif
// was lookahead scan successful?
} else if (lfs3_t_isrepoplookahead(mgc->t.b.h.flags)
&& !lfs3_t_ismtreeonly(mgc->t.b.h.flags)
&& !lfs3_t_isckpointed(mgc->t.b.h.flags)) {
lfs3_alloc_adopt(lfs3, lfs3->lookahead.ckpoint);
}
// was mkconsistent successful?
if (lfs3_t_ismkconsistent(mgc->t.b.h.flags)
&& !lfs3_t_isdirty(mgc->t.b.h.flags)
tests/test_gc.toml
+102
View File
@@ -2598,6 +2598,108 @@ code = '''
lfs3_unmount(&lfs3) => 0;
'''
# test that gc work doesn't break anything in low-space condiditions
[cases.test_gc_nospc]
defines.MKCONSISTENT = [false, true]
defines.REPOPLOOKAHEAD = [false, true]
defines.REPOPGBMAP = [false, true]
defines.COMPACTMETA = [false, true]
defines.CKMETA = [false, true]
defines.CKDATA = [false, true]
defines.GC_FLAGS = '''
((MKCONSISTENT) ? LFS3_GC_MKCONSISTENT : 0)
| ((REPOPLOOKAHEAD) ? LFS3_GC_REPOPLOOKAHEAD : 0)
| ((REPOPGBMAP) ? LFS3_IFDEF_GBMAP(LFS3_GC_REPOPGBMAP, -1) : 0)
| ((COMPACTMETA) ? LFS3_GC_COMPACTMETA : 0)
| ((CKMETA) ? LFS3_GC_CKMETA : 0)
| ((CKDATA) ? LFS3_GC_CKDATA : 0)
'''
# DON'T test with GC_STEPS=-1, it may never terminate!
defines.GC_STEPS = [1, 2, 10, 100, 1000]
# set compactmeta thresh to minimum
defines.GC_COMPACTMETA_THRESH = 'BLOCK_SIZE/2'
defines.SIZE = 'BLOCK_SIZE'
if = 'GBMAP || !REPOPGBMAP'
ifdef = 'LFS3_GC'
code = '''
lfs3_t lfs3;
lfs3_format(&lfs3,
LFS3_F_RDWR
| ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_F_GBMAP, -1) : 0),
CFG) => 0;
lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0;
uint32_t prng = 42;
for (uint32_t i = 0;; i++) {
// create a new file every gc cycle
lfs3_file_t file;
char name[256];
sprintf(name, "purseweb%03x", i);
int err = lfs3_file_open(&lfs3, &file, name,
LFS3_O_WRONLY | LFS3_O_CREAT | LFS3_O_EXCL);
assert(!err || err == LFS3_ERR_NOSPC);
if (err == LFS3_ERR_NOSPC) {
break;
}
uint8_t wbuf[SIZE];
for (lfs3_size_t j = 0; j < SIZE; j++) {
wbuf[j] = 'a' + (TEST_PRNG(&prng) % 26);
}
lfs3_ssize_t d = lfs3_file_write(&lfs3, &file, wbuf, SIZE);
assert(d == SIZE || d == LFS3_ERR_NOSPC);
if (d == LFS3_ERR_NOSPC) {
lfs3_file_close(&lfs3, &file) => 0;
break;
}
err = lfs3_file_close(&lfs3, &file);
assert(!err || err == LFS3_ERR_NOSPC);
if (err == LFS3_ERR_NOSPC) {
break;
}
// gc!
//
// gc should not error, but may be unable to make progress
lfs3_fs_gc(&lfs3) => 0;
}
// check the contents of the files that were written
for (int remount = 0; remount < 2; remount++) {
// remount?
if (remount) {
lfs3_unmount(&lfs3) => 0;
lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0;
}
// reset prng
uint32_t prng = 42;
// try to read
for (uint32_t i = 0;; i++) {
lfs3_file_t file;
char name[256];
sprintf(name, "purseweb%03x", i);
int err = lfs3_file_open(&lfs3, &file, name, LFS3_O_RDONLY);
assert(!err || err == LFS3_ERR_NOENT);
if (err == LFS3_ERR_NOENT) {
break;
}
uint8_t wbuf[SIZE];
for (lfs3_size_t j = 0; j < SIZE; j++) {
wbuf[j] = 'a' + (TEST_PRNG(&prng) % 26);
}
uint8_t rbuf[SIZE];
lfs3_file_read(&lfs3, &file, rbuf, SIZE) => SIZE;
assert(memcmp(rbuf, wbuf, SIZE) == 0);
lfs3_file_close(&lfs3, &file) => 0;
}
}
lfs3_unmount(&lfs3) => 0;
'''
# many/fuzz tests mixed with GC