Github/littlefs
1
Fork 0
Code Actions Activity

Fixed mroot death returning LFS_ERR_CORRUPT -> LFS_ERR_NOSPC

Browse Source 
It's counter-intuitive, but no top-level API should return
LFS_ERR_CORRUPT. Instead, if we can't make progress because of a corrupt
block, we should return LFS_ERR_NOSPC. This makes it easier for users to
write code that is well behaved even when a device is end-of-life.

It's up to our mroot extension algorithm to make sure this case can't be
reached in normal operation unless the device is _actually_ at
end-of-life.

Because mroot extension is a bit of a special case, we weren't
converting these corrupt errors to nospc errors consistently. This is
fixed now, along with a couple more hopefully-useful logging statements.

Found while playing around with test_exhaustion + block_recycles=-1.
This should assert on bad wear-leveling, but LFS_ERR_CORRUPT was
unexpected. Added an explicit test because this is an easy thing to let
split through:

- test_badblocks_mrootanchor_wear

Code changes were surprisingly minimal, I wonder if constants are being
swapped out somewhere low-level?

           code          stack
  before: 33766           2600
  after:  33770 (+0.0%)   2600 (+0.0%)
This commit is contained in:
Christopher Haster
2024-06-05 00:02:32 -05:00
parent 26f3034813
commit 76ffb0e7b6
2 changed files with 67 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+19 -2
View File
@@ -6668,9 +6668,12 @@ relocate:;
err = lfsr_mdir_swap__(lfs, &mdir_, mdir, true);
if (err) {
// bad prog? try another block
// bad prog? can't do much here, mdir stuck
if (err == LFS_ERR_CORRUPT) {
goto relocate;
LFS_DEBUG("Stuck mdir 0x{%"PRIx32",%"PRIx32"}",
mdir->rbyd.blocks[0],
mdir->rbyd.blocks[1]);
return LFS_ERR_NOSPC;
}
return err;
}
@@ -7211,6 +7214,13 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
lfsr_mdir_t mrootanchor_;
err = lfsr_mdir_swap__(lfs, &mrootanchor_, &mrootchild, true);
if (err) {
// bad prog? can't do much here, mroot stuck
if (err == LFS_ERR_CORRUPT) {
LFS_DEBUG("Stuck mroot 0x{%"PRIx32",%"PRIx32"}",
mrootanchor_.rbyd.blocks[0],
mrootanchor_.rbyd.blocks[1]);
return LFS_ERR_NOSPC;
}
goto failed;
}
@@ -7228,6 +7238,13 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE);
LFS_ASSERT(err != LFS_ERR_NOENT);
// bad prog? can't do much here, mroot stuck
if (err == LFS_ERR_CORRUPT) {
LFS_DEBUG("Stuck mroot 0x{%"PRIx32",%"PRIx32"}",
mrootanchor_.rbyd.blocks[0],
mrootanchor_.rbyd.blocks[1]);
return LFS_ERR_NOSPC;
}
goto failed;
}
}
tests/test_badblocks.toml
+48 -1
View File
@@ -5095,7 +5095,7 @@ code = '''
## other corner cases
# test formatting with 0 or 1 bad, this should just error
[cases.test_badblocks_mrootanchor]
[cases.test_badblocks_mrootanchor_format]
defines.ERASE_CYCLES = 0xffffffff
defines.BADBLOCKS = [0x1, 0x2, 0x3]
defines.BADBLOCK_BEHAVIOR = [
@@ -5119,6 +5119,53 @@ code = '''
lfsr_format(&lfs, CFG) => LFS_ERR_CORRUPT;
'''
# test blocks 0 or 1 going bad, this should just error
[cases.test_badblocks_mrootanchor_wear]
defines.ERASE_CYCLES = 0xffffffff
defines.BADBLOCKS = [0x1, 0x2]
defines.BADBLOCK_BEHAVIOR = [
'LFS_EMUBD_BADBLOCK_PROGERROR',
'LFS_EMUBD_BADBLOCK_ERASEERROR',
'LFS_EMUBD_BADBLOCK_READERROR',
'LFS_EMUBD_BADBLOCK_PROGNOOP',
'LFS_EMUBD_BADBLOCK_ERASENOOP',
]
# we need prog checking to detect read errors
defines.CHECK_PROGS = 'BADBLOCK_BEHAVIOR >= LFS_EMUBD_BADBLOCK_READERROR'
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
if (BADBLOCKS & 0x1) {
lfs_emubd_setwear(CFG, 0, 0xffffffff) => 0;
}
if (BADBLOCKS & 0x2) {
lfs_emubd_setwear(CFG, 1, 0xffffffff) => 0;
}
lfsr_mount(&lfs, CFG) => 0;
for (lfs_size_t i = 0;; i++) {
// this should eventually fail
assert(i <= BLOCK_COUNT);
int err = lfsr_mkdir(&lfs, "hi");
assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) {
break;
}
err = lfsr_remove(&lfs, "hi");
assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) {
break;
}
}
lfsr_unmount(&lfs) => 0;
'''