Fixed weakened wear-leveling (mdir aliasing) due to rev changes
This is a common pitfall with mdirs that has only ended up in the
codebase ~3(?) times. Naively using a power-of-two recycle counter for
relocations ends up aliasing mdir blocks such that only one block is
actually wear-leveled.
I'm not entirely sure it was intentional, but the previous
double-increment during needsrelocation checks made mdirs relocate one
recycle early, avoiding this aliasing issue.
However, the double-increment had other issues. The most glaring is that
it would always trigger two relocations back-to-back due to the mismatch
between counter cycles and overflow checks. Sort of defeating the
purpose of wear-leveling the two mdir blocks separately...
---
What we really want is a counter that's always coprime with 2. Such as
our old friend mod (2^n)-1.
Unfortunately mod (2^n)-1 counters don't really have any great
optimization trick. They show up all the time when code relies on the
multiplicative cycle of a 2^n finite-field, but despite this, all of
the implementations I've seen rely on a simple branch to handle the
one extra state.
I'm not sure this is the best implementation, but adding 2 and
subtracting 1 on non-overflow seems to minimize resulting code cost.
Presumably because the compiler is able to deduplicate this with the
needsrelocation check. Though we're only talking about a handful of
bytes.
Code changes:
code stack ctx
before: 35152 2136 660
after: 35164 (+0.0%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38388 2144 776
gbmap after: 38400 (+0.0%) 2144 (+0.0%) 776 (+0.0%)
code stack ctx
preerase before: 38928 2168 796
preerase after: 38940 (+0.0%) 2168 (+0.0%) 796 (+0.0%)
---
We don't have great tests for this, as it's difficult to create
rigorous checks for our best-effort dynamic wear-leveling. But,
surprisingly enough, this _was_ caught by
test_exhaustion_spam_uzd_fuzz's doubling-disk-doubles-lifetime check!
Though only with LFS3_YES_GBMAP=1:
LFS3_YES_GBMAP=1 \
TESTS=tests/test_exhaustion.toml \
make test-runner -j \
&& ./scripts/test.py test_exhaustion_spam_uzd_fuzz -O- -j \
| grep lifetime
This commit is contained in:
@@ -7798,7 +7798,10 @@ static inline bool lfs3_rev_needsrelocation(lfs3_t *lfs3, uint32_t rev) {
|
||||
}
|
||||
|
||||
// does out recycle counter overflow?
|
||||
uint32_t rev_ = rev + (1 << (28-lfs3_smax(lfs3->recycle_bits, 0)));
|
||||
uint32_t rev_ = rev + (2 << (28-lfs3_smax(lfs3->recycle_bits, 0)));
|
||||
// ^ note the +2! this ensures we overflow after
|
||||
// an odd number of recycles, otherwise we'd
|
||||
// only ever relocate one block in mdirs
|
||||
return (rev_ >> 28) != (rev >> 28);
|
||||
}
|
||||
#endif
|
||||
@@ -7806,7 +7809,15 @@ static inline bool lfs3_rev_needsrelocation(lfs3_t *lfs3, uint32_t rev) {
|
||||
#ifndef LFS3_RDONLY
|
||||
static inline uint32_t lfs3_rev_inc(lfs3_t *lfs3, uint32_t rev) {
|
||||
// increment recycle counter/revision
|
||||
return rev + (1 << (28-lfs3_smax(lfs3->recycle_bits, 0)));
|
||||
//
|
||||
// this mess increments by 2 unless we _don't_ overflow, in effect
|
||||
// it implements a mod (2^n)-1 counter, the goal is to avoid
|
||||
// multiple needsrelocation triggers (see above)
|
||||
uint32_t rev_ = rev + (2 << (28-lfs3_smax(lfs3->recycle_bits, 0)));
|
||||
if ((rev_ >> 28) == (rev >> 28)) {
|
||||
rev_ -= 1 << (28-lfs3_smax(lfs3->recycle_bits, 0));
|
||||
}
|
||||
return rev_;
|
||||
}
|
||||
#endif
|
||||
|
||||
@@ -15174,18 +15185,17 @@ static int lfs3_init(lfs3_t *lfs3, uint32_t flags,
|
||||
|
||||
// find the number of bits to use for recycle counters
|
||||
//
|
||||
// Add 1, to include the initial erase, multiply by 2, since we
|
||||
// alternate which metadata block we erase each compaction, and limit
|
||||
// to 28-bits so we always have some bits to determine the most recent
|
||||
// revision.
|
||||
// Add 1 for the initial erase, and multiply by 2 since we alternate
|
||||
// which metadata block we erase each compaction.
|
||||
//
|
||||
// We're currently limited to 20-bits to keep space for
|
||||
// perturb/low-effort debug bits, though this could be relaxed
|
||||
// in the future (though >28 bits may cause problems since we need
|
||||
// some bits to tell revisions apart).
|
||||
//
|
||||
#ifndef LFS3_RDONLY
|
||||
if (lfs3->cfg->block_recycles != -1) {
|
||||
lfs3->recycle_bits = lfs3_min(
|
||||
lfs3_nlog2(2*(lfs3->cfg->block_recycles+1)+1)-1,
|
||||
28);
|
||||
// we're currently limited to 20-bits to keep space for
|
||||
// perturb/low-effort debug bits, though this could be relaxed
|
||||
// in the future
|
||||
lfs3->recycle_bits = lfs3_nlog2(2*(lfs3->cfg->block_recycles+1)+1)-1;
|
||||
LFS3_ASSERT(lfs3->recycle_bits <= 20);
|
||||
} else {
|
||||
lfs3->recycle_bits = -1;
|
||||
|
||||
Reference in New Issue
Block a user