Prevent overrecycling on bad progs
This tweaks lfsr_mdir_commit_ to avoid overrecycling if we encounter a
bad prog (LFS_ERR_CORRUPT). This avoids compacting to the same block
twice, which risks an undetected prog error and breaks internal
invariants.
Note we still overrecycle if the relocation reason is a recycle
overflow.
---
This is an alternative solution to the previous overrecycling + shrub +
ckprog bug: Just make sure we don't compact to the same block twice!
After all, if we just got a bad prog, why are we trying to prog again?
(There are actually some arguments for multiple prog attempts, bus
errors for example, but I don't think that's a great excuse for littlefs
attempting multiple progs without user input.)
Even though this adds logic to lfsr_mdir_commit_, it ends up saving
code since we can drop the shrub discard pass:
code stack ctx
before: 37088 2304 636
after: 37056 (-0.1%) 2304 (+0.0%) 636 (+0.0%)
Not that we _really_ care about this quantity of code. The real
motivation is 1. lowering the risk of a missed prog error, and
2. maintaining the never-compact-same-block invariant in case there
are other invariant-dependent bugs lurking around.
This commit is contained in:
@@ -8422,7 +8422,7 @@ static int lfsr_mdir_commit_(lfs_t *lfs, lfsr_mdir_t *mdir,
|
|||||||
swap:;
|
swap:;
|
||||||
// can't commit, can we compact?
|
// can't commit, can we compact?
|
||||||
bool relocated = false;
|
bool relocated = false;
|
||||||
bool overrecycled = false;
|
bool overrecyclable = true;
|
||||||
|
|
||||||
// check if we're within our compaction threshold
|
// check if we're within our compaction threshold
|
||||||
lfs_ssize_t estimate = lfsr_mdir_estimate__(lfs, mdir, start_rid, end_rid,
|
lfs_ssize_t estimate = lfsr_mdir_estimate__(lfs, mdir, start_rid, end_rid,
|
||||||
@@ -8440,6 +8440,7 @@ swap:;
|
|||||||
err = lfsr_mdir_swap__(lfs, &mdir_, mdir, false);
|
err = lfsr_mdir_swap__(lfs, &mdir_, mdir, false);
|
||||||
if (err) {
|
if (err) {
|
||||||
if (err == LFS_ERR_NOSPC || err == LFS_ERR_CORRUPT) {
|
if (err == LFS_ERR_NOSPC || err == LFS_ERR_CORRUPT) {
|
||||||
|
overrecyclable &= (err != LFS_ERR_CORRUPT);
|
||||||
goto relocate;
|
goto relocate;
|
||||||
}
|
}
|
||||||
return err;
|
return err;
|
||||||
@@ -8450,7 +8451,7 @@ swap:;
|
|||||||
relocate:;
|
relocate:;
|
||||||
// needs relocation? bad prog? ok, try allocating a new mdir
|
// needs relocation? bad prog? ok, try allocating a new mdir
|
||||||
err = lfsr_mdir_alloc__(lfs, &mdir_, mdir->mid, relocated);
|
err = lfsr_mdir_alloc__(lfs, &mdir_, mdir->mid, relocated);
|
||||||
if (err && !(err == LFS_ERR_NOSPC && !overrecycled)) {
|
if (err && !(err == LFS_ERR_NOSPC && overrecyclable)) {
|
||||||
return err;
|
return err;
|
||||||
}
|
}
|
||||||
relocated = true;
|
relocated = true;
|
||||||
@@ -8461,7 +8462,8 @@ relocate:;
|
|||||||
LFS_WARN("Overrecycling mdir %"PRId32" 0x{%"PRIx32",%"PRIx32"}",
|
LFS_WARN("Overrecycling mdir %"PRId32" 0x{%"PRIx32",%"PRIx32"}",
|
||||||
lfsr_dbgmbid(lfs, mdir->mid),
|
lfsr_dbgmbid(lfs, mdir->mid),
|
||||||
mdir->rbyd.blocks[0], mdir->rbyd.blocks[1]);
|
mdir->rbyd.blocks[0], mdir->rbyd.blocks[1]);
|
||||||
overrecycled = true;
|
relocated = false;
|
||||||
|
overrecyclable = false;
|
||||||
|
|
||||||
err = lfsr_mdir_swap__(lfs, &mdir_, mdir, true);
|
err = lfsr_mdir_swap__(lfs, &mdir_, mdir, true);
|
||||||
if (err) {
|
if (err) {
|
||||||
@@ -8476,16 +8478,6 @@ relocate:;
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// discard any committed shrubs, we need to do this explicitly
|
|
||||||
// when overrecycling
|
|
||||||
for (lfsr_omdir_t *o = lfs->omdirs; o; o = o->next) {
|
|
||||||
if (lfsr_o_isbshrub(o->flags)
|
|
||||||
&& ((lfsr_bshrub_t*)o)->shrub_.blocks[0]
|
|
||||||
== mdir_.rbyd.blocks[0]) {
|
|
||||||
((lfsr_bshrub_t*)o)->shrub_.blocks[0] = -1;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
compact:;
|
compact:;
|
||||||
#ifdef LFS_DBGMDIRCOMMITS
|
#ifdef LFS_DBGMDIRCOMMITS
|
||||||
LFS_DEBUG("Compacting mdir %"PRId32" 0x{%"PRIx32",%"PRIx32"} "
|
LFS_DEBUG("Compacting mdir %"PRId32" 0x{%"PRIx32",%"PRIx32"} "
|
||||||
@@ -8497,7 +8489,7 @@ compact:;
|
|||||||
|
|
||||||
// don't copy over gcksum if relocating
|
// don't copy over gcksum if relocating
|
||||||
lfsr_srid_t start_rid_ = start_rid;
|
lfsr_srid_t start_rid_ = start_rid;
|
||||||
if (relocated && !overrecycled) {
|
if (relocated) {
|
||||||
start_rid_ = lfs_smax(start_rid_, -1);
|
start_rid_ = lfs_smax(start_rid_, -1);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -8507,6 +8499,7 @@ compact:;
|
|||||||
LFS_ASSERT(err != LFS_ERR_RANGE);
|
LFS_ASSERT(err != LFS_ERR_RANGE);
|
||||||
// bad prog? try another block
|
// bad prog? try another block
|
||||||
if (err == LFS_ERR_CORRUPT) {
|
if (err == LFS_ERR_CORRUPT) {
|
||||||
|
overrecyclable &= relocated;
|
||||||
goto relocate;
|
goto relocate;
|
||||||
}
|
}
|
||||||
return err;
|
return err;
|
||||||
@@ -8522,13 +8515,14 @@ compact:;
|
|||||||
LFS_ASSERT(err != LFS_ERR_RANGE);
|
LFS_ASSERT(err != LFS_ERR_RANGE);
|
||||||
// bad prog? try another block
|
// bad prog? try another block
|
||||||
if (err == LFS_ERR_CORRUPT) {
|
if (err == LFS_ERR_CORRUPT) {
|
||||||
|
overrecyclable &= relocated;
|
||||||
goto relocate;
|
goto relocate;
|
||||||
}
|
}
|
||||||
return err;
|
return err;
|
||||||
}
|
}
|
||||||
|
|
||||||
// consume gcksumdelta if relocated
|
// consume gcksumdelta if relocated
|
||||||
if (relocated && !overrecycled) {
|
if (relocated) {
|
||||||
lfs->gcksum_d ^= mdir->gcksumdelta;
|
lfs->gcksum_d ^= mdir->gcksumdelta;
|
||||||
}
|
}
|
||||||
// update mdir
|
// update mdir
|
||||||
@@ -8696,9 +8690,9 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
|
|||||||
// order the split compacts so that that mdir containing our mid
|
// order the split compacts so that that mdir containing our mid
|
||||||
// is committed last, this is a bit of a hack but necessary so
|
// is committed last, this is a bit of a hack but necessary so
|
||||||
// shrubs are staged correctly
|
// shrubs are staged correctly
|
||||||
bool l = lfsr_mrid(lfs, mdir->mid) < split_rid;
|
bool l = (lfsr_mrid(lfs, mdir->mid) < split_rid);
|
||||||
|
|
||||||
bool relocated = false;;
|
bool relocated = false;
|
||||||
split_relocate:;
|
split_relocate:;
|
||||||
// alloc and compact into new mdirs
|
// alloc and compact into new mdirs
|
||||||
err = lfsr_mdir_alloc__(lfs, &mdir_[i^l],
|
err = lfsr_mdir_alloc__(lfs, &mdir_[i^l],
|
||||||
|
|||||||
Reference in New Issue
Block a user