I mean, what does the single mptr representation get us anyways?
A slightly smaller filesystem when metadata doesn't _quite_ fit in the
mroot, but fits in a single mdir? The only difference between the two is
the filesystem config, which really shouldn't be _that_ large:
mmoss: msprout: mtree:
.--------. .--------. .--------.
.| mroot | .| mroot | .| mroot |
|| config | || config | || config |
|| dir / | || mptr | || mptr |
|'--------' |'---|----' |'---|----'
'--------' '----|---' '----|---'
v v
.--------. .--------.
.| mdir | | mtree |
|| mptr | | mptr |
|| | | | |
|'--------' '---|----'
'--------' v
.--------.
.| mdir |
|| mptr |
|| |
|'--------'
'--------'
Unlike the theoretical mshrub representation, which we've already given
up on, msprouts _don't_ require a significant code cost to implement.
But if they offer little value, why keep them around? Every code cost is
code cost.
---
This drops msprout support, saving a bit of code:
code stack ctx
before: 38508 2624 640
after: 38340 (-0.4%) 2624 (+0.0%) 640 (+0.0%)
This also clears the LFSR_RCOMPAT_MSPROUT flag, allowing for msprouts to
be possibly reintroduced in the future.
But I don't really see any motivation for msprouts, even if you don't
care about code size, so I suspect this will just be a dead bit from now
on...
littlefs is intentionally designed to not rely on noise, even with cksum
collisions (hello, perturb bit!). So it makes sense for this to be an
optional feature, even if it's a small one.
Disabling revision count noise by default also helps with testing. The
whole point of revision count noise is to make cksum collisions less
likely, which is a bit counterproductive when that's something we want
to test!
This doesn't really change the revision count encoding:
vvvvrrrr rrrrrrnn nnnnnnnn nnnnnnnn
'-.''----.----''---------.--------'
'------|---------------|---------- 4-bit relocation revision
'---------------|---------- recycle-bits recycle counter
'---------- pseudorandom noise (optional)
I considered moving the recycle-bits down when we're not adding noise,
but the extra logic just isn't worth making the revision count a bit
more human-readable.
---
This saves a small bit of code in the default build, at the cost of some
code for the runtime checks in the LFS_NOISY build. Though I'm hoping
future config work will let users opt-out of these runtime checks:
code stack ctx
before: 38548 2624 640
default after: 38508 (-0.1%) 2624 (+0.0%) 640 (+0.0%)
LFS_NOISY after: 38568 (+0.1%) 2624 (+0.0%) 640 (+0.0%)
Honestly the thing I'm more worried about is using one of our precious
mount flags for this... There's not that many bits left!
A bit ugly, but this seems necessary in order to allow defines that seem
reasonable to provide together, such as -DLFS_NO_LOG and -DLFS_NO_DEBUG.
This is useful for allowing define combinations that also work in older
builds.
This no longer includes LFS_NO_ASSERT, but at least 2 defines is better
than 5 right?
LFS_NO_LOG => LFS_NO_DEBUG
LFS_NO_INFO
LFS_NO_WARN
LFS_NO_ERROR
The benefit is a much more obvious name, and avoids confusion when mixed
with other feature aliases (LFS_LITTLER + LFS_BIGGEST?).
To help with debugging. These all seem useful, though the exact output
will probably be worth messing around with:
- LFS_DEBUGRBYDFETCHES - Debug every rbyd fetch
- LFS_DEBUGRBYDCOMMITS - Debug every rbyd commit
- LFS_DEBUGBTREEFETCHES - Debug every btree/bshrub fetch (though we
currently don't fetch bshrubs...)
- LFS_DEBUGBTREECOMMITS - Debug every btree/bshrub commit
- LFS_DEBUGMDIRFETCHES - Debug every mdir fetch
- LFS_DEBUGMDIRCOMMITS - Debug every mdir commit
- LFS_DEBUGALLOCS - Debug every block allocation
Let's see if you can match these to each debug output:
lfs.c:2942:debug: Fetched rbyd 0xe.d80 w77, eoff 3536, cksum 862283c6
lfs.c:4233:debug: Committed rbyd 0xe.dd0 w78, eoff 3616, cksum 38ae1347
lfs.c:4950:debug: Fetched btree 0x9f.806 w2048, cksum 7fb89b1b
lfs.c:6609:debug: Committed btree 0x9f.806 w2048, cksum 7fb89b1b
lfs.c:6603:debug: Committed bshrub 0x{0,1}.b06 w1747
lfs.c:7290:debug: Fetched mdir -1 0x{1,0}.8f w0, cksum 7846be7a
lfs.c:9022:debug: Committed mdir 0 0x{0,1}.a10 w2, cksum 4d2ccb29
lfs.c:10083:debug: Allocated block 0x8f, lookahead 125/253/256
Also tweaked LFSR_DEBUGRBYDBALANCE to be a bit more readable when
LFS_DEBUGRBYDFETCHES is enabled, and tweaked the out-of-space error
message to show the same lookahead info as LFS_DEBUGALLOCS:
lfs.c:10101:error: No more free space (lookahead 0/0/256)
^ ^ ^
lookahead remaining --' | |
ckpoint remaining ------' |
block count ---------------'
No code changes.
Like LFS_LITTLER, this is a simple alias for existing defines. In this
case it enables all opt-in features, which is useful for testing:
LFS_BIGGEST => LFS_CKPROGS
LFS_CKFETCHES
LFS_CKPARITY
LFS_CKDATACKSUMS
LFS_GC
This lowers the risk of forgetting a feature when running tests, which
has already happend a couple times.
The name comes from the idea of adding other tiers of features (
LFS_LITTLER, LFS_LITTLERER, LFS_BIGGER, etc), but I'm not entirely sure
what these would all look like.
This is a simple alias for a number of existing defines:
LFS_LITTLER => LFS_NO_DEBUG
LFS_NO_INFO
LFS_NO_WARN
LFS_NO_ERROR
LFS_NO_ASSERT
This is mainly to make CI/code measurements easier. I suspect most
systems already make these zero (or at least minimal) cost in release
mode, but when building with our Makefile we need some way to disable
the simple printf/assert mapping.
This probably deserves a better name, but at least this is something.
This adds LFS_INFO and limits LFS_DEBUG to opt-in debug output.
Note that while LFS_DEBUG seems to be only used in LFS_DEBUGRBYDBALANCE,
it can also be useful for adding additional logging while debugging.
Having separate levels here is useful for filtering. Users should be
able to expect LFS_INFO output to not bog down a system, while LFS_DEBUG
can be a free-for-all dumping ground of debug info.
This also converges to the common 4 levels of logging found in other
systems, which was an intentional non-goal, but it's interesting to see
how each level serves a purpose.
Now with both height (alt-height) and bheight (black-height):
lfs.c:2970:debug: rbyd 0x9.c05: height 6-11, bheight 6-6
It's interesting to note this highlighted a minor mistake in our assumed
rbyd bounds.
Originally, our rbyd algorithm generated trees with height bounded by
2*bheight+1, but now that we have range removals, our height can get up
to 2*bheight+2 due to how diverged paths are stitched together.
This name helps avoid the mistake the that LFS_ASSERTRBYDBALANCE is
cheap/free like other asserts, which is very much not true.
LFS_DEBUGRBYDBALANCE is expensive and should only be used for testing.
Aside from cleaning up the mess of debug statements/commented code, this
also includes a bit of fiddling with the append logic to try to make
things a bit more readable and minimize code cost:
code stack ctx
before: 38784 2624 640
after: 38548 (-0.6%) 2624 (+0.0%) 640 (+0.0%)
Now we can better compare before and after the balance rework:
code stack ctx
before rbyd-balance-rework: 38440 2624 640
after rbyd-balance-rework: 38548 (+0.3%) 2624 (+0.0%) 640 (+0.0%)
Though it's worth emphasizing that maintaining strictly balanced rbyds
is well worth the extra code cost, since it's sort of what the rest of
the filesystem is built on.
The issue with relying solely on the jump > branch hack to disambiguate
recolorable prunes is that suddenly we're back in ambiguous territory
when we encounter post-split yellow nodes. We need to prioritize yellow
prunes or else post-split weights can become ambiguous.
This was the whole reason we added a 3rd color!
Fortunately we can still prioritize yellow nodes by trying both
recolorable prunes first, and only falling back to non-recolorable
prunes if we still have unreachable alts.
Ends up with a bit of code duplication, but gets things working again.
---
Turns out this was all we needed to get our rbyd operations perfectly
balanced! Now all test_rbyd tests are passing:
test_rbyd+balance before: 1671/385878 failed
test_rbyd+balance after: 0/385878 failed (-100.0%)
And after running the full test suite, can confirm _all_ tests are
passing with LFS_ASSERTRBYDBALANCE. So I think we have some pretty
decent confidence our rbyd algorithm maintains balance, even with range
removals:
test+balance before: 20614/631541 failed
test+balance after: 0/631541 failed (-100.0%)
It was an open question if this was even possible, so it's nice to see
some evidence balanced range removals are not a problem.
---
The current implementation is a bit hacky, so we do take a hit to code
size. Though we may be able to claw this back after cleaning things up:
code stack ctx
before: 38580 2624 640
after: 38784 (+0.5%) 2624 (+0.0%) 640 (+0.0%)
What a funny bug.
I was wondering how jump ended up == branch, it turns out this was a
simple typo in the diverged-stitching rework.
What's extra humorous is I think the original code was trying to set
branch_ = jump, which wouldn't have worked. So the typo actually got
diverged-stitching to a more workable state than if there was no typo.
It just then broke the y-split logic.
Not randomly changing the branch when stitching is the correct fix.
---
This doesn't actually reduce any test failures since it's replacing the
previous jump >= branch "fix", which was really just a bandaid:
test_rbyd+balance before: 1671/385878 failed
test_rbyd+balance after: 1671/385878 failed (+0.0%)
Saves one ld/st though:
code stack ctx
before: 38584 2624 640
after: 38580 (-0.0%) 2624 (+0.0%) 640 (+0.0%)
Now that we have this jump > branch trick in our back pocket, we can
undo most of the y-pruning/diverged-pruning separation.
In theory all we need is to conditionally recolor based on if jump is >
branch.
---
Well, in theory at least. This seems to increase the number of test
failures, but it's unclear if this is due to jump > branch being an
incorrect assumption or some other bug exposed by the change in logic:
test_rbyd+balance before: 300/385878 failed
test_rbyd+balance after: 1671/385878 failed (+457.0%)
But it does save code:
code stack ctx
before: 38880 2624 640
after: 38584 (-0.8%) 2624 (+0.0%) 640 (+0.0%)
Idea!
The problem isn't so much that y-pruning recolors and diverged-pruning
doesn't recolor, but that we are unconditionally recoloring based on
the assumed yellow node direction.
Consider all possible yellow prune permutations:
<y <r
.-------'| .-------'|
| <r prune y | |
| .----'| => | .--'
| | <b | <b
| | .-'| | .-'|
1 2 3 4 1 2 3 4 x
<r
.----------'|
prune r | |
=> | .--'
| <b
| .-'|
1 2 3 4 x
<b
.----------'|
prune b | |
=> | .--------'
| |
| |
1 2 3 4 x
Note these all maintain the color balance of our tree, but we only need
to recolor the yellow alt when it's the black alt we're pruning.
But how do we know if it's the black alt we're pruning? Well, we can use
the same trick we used in unflipping y-splits. The black alt is the only
alt where jump >= the current branch.
This is true because we always split yellow nodes as soon as we see
them.
---
This idea is promising, but needs a bit of work.
So, uh, please ignore the additional test failures:
test_rbyd+balance before: 312/385878 failed
test_rbyd+balance after: 300/385878 failed (-3.8%)
Curiously, these are now actual test failures and not just balance
failures. I can't decide if that's a good sign or not.
Code changes:
code stack ctx
before: 38860 2624 640
after: 38880 (+0.1%) 2624 (+0.0%) 640 (+0.0%)
The conflicts between y-pruning and diverged-pruning are proving
difficult to resolve...
This is a step backwards but also a work in progress:
test_rbyd+balance before: 156/385878 failed
test_rbyd+balance after: 312/385878 failed (+100.0%)
Code changes:
code stack ctx
before: 38644 2624 640
after: 38860 (+0.6%) 2624 (+0.0%) 640 (+0.0%)
I'm not sure this is the correct solution, or why exactly we end up with
jump == branch... but the stitching alt was causing our branch-based
unflipping in the y-split logic to fail.
Changing this logic to jump >= branch seems to solve this issue, though
more investigation is needed into why...
But the good news is this small tweak solves half our current test
failures:
test_rbyd+balance before: 300/385878 failed
test_rbyd+balance after: 156/385878 failed (-48.0%)
Code changes:
code stack ctx
before: 38644 2624 640
after: 38644 (+0.0%) 2624 (+0.0%) 640 (+0.0%)
The main idea here is we need to unconditionally descend down red/yellow
alts to check if they're post-split yellow nodes before pruning, since
yellow nodes should be recolored to maintain the color balance of our
tree.
To make this work:
- Added separate y-pruning logic before our diverged logic, this starts
to look a bit like previous incarnations of this function.
- Our non-y-pruning logic is all now gated behind the if-diverging
check, so we shouldn't be non-y-pruning at all unless we've diverged.
It may be worth rewriting this to use the trimming logic directly,
instead of reachability.
- Moving non-y-pruning logic behind the if-diverging check ended up
causing problems for the stitching node, which is still a bit of a
special case. A hacky goto solves this for now at the risk of
velociraptors... https://xkcd.com/292
It's hacky, but the goal right now is to just get something working.
This may seem like a lot of changes for only a couple more tests
passing, but progress is progress:
test_rbyd+balance before: 306/385878 failed
test_rbyd+balance after: 300/385878 failed (-2.0%)
Code changes:
code stack ctx
before: 38528 2624 640
after: 38644 (+0.3%) 2624 (+0.0%) 640 (+0.0%)
In our pruning logic we eagerly prune root alts and red alts as soon as
we notice they're unreachable.
On paper this is fine, but it's hiding post-split yellow alts from being
recolored to maintain the color balance of the tree...
This is a work in progress...
Test changes:
test_rbyd+balance before: 273/385878 failed
test_rbyd+balance after: 306/385878 failed (+12.1%)
Code changes:
code stack ctx
before: 38536 2624 640
after: 38528 (-0.0%) 2624 (+0.0%) 640 (+0.0%)
- Since the diverged stitching alt is no longer a special case, we
should preserve its color in order to preserve the tree balance.
The fact that this works (ignoring balance) is a great sign.
- It seems we need to collapse both-diverging alts outside of our
diverging logic? I think this helps cover cases where all three alts
in a yellow node diverge.
This got a few more tests passing:
test_rbyd+balance before: 306/385878 failed
test_rbyd+balance after: 273/385878 failed (-10.8%)
Code changes
code stack ctx
before: 38408 2624 640
after: 38536 (+0.3%) 2624 (+0.0%) 640 (+0.0%)
Found with LFS_ASSERTRBYDBALANCE, the way we were introducing a new alt
to stitch together diverged trunks was preventing unreachable diverged
trunks from being pruned.
This isn't a hard error, since we end up appending a null tag to
terminate the tree, but like our compaction balance issue, the
terminating null tag leads to balance issues down the road:
.---> a .---> a
.---r-b-> b .---r-b-> b
| .---> c | .---> c
.---r-b-r-b-> d .---r-b-r-b-> d
| .---b---b-> e .---y-r---b---b-> e
r-b---------> null => | .-------------> f
| | .-----------> g
| | | .---------> h
b-y-r-b---------> i
'--------.--------'
unbalanced :(
Fortunately, after fiddling around with the algorithm a bit, it turns
out we don't really need a special case for the stitching alt as long as
we adjust the weight a bit.
This allows pruning of the stitching alt and avoids this unexpected
balance issue, while also simplifying how we stitch diverged trunks. Win
win.
Goes to show LFS_ASSERTRBYDBALANCE will probably be quite a valuable
assertion, assuming the remaining balance issues are solvable.
---
Test changes, this gets at least all non-weighted/non-range test_rbyd
tests passing with balance asserts:
test_rbyd+balance before: 351/385878 failed
test_rbyd+balance after: 306/385878 failed (-12.8%)
Ran with:
$ DEBUG=1 \
TESTS=tests/test_rbyd.toml \
CFLAGS=-DLFS_ASSERTRBYDBALANCE \
make test-runner -j \
&& ./scripts/test.py -j -B -k
While even saving code:
code stack ctx
before: 38440 2624 640
after: 38408 (-0.1%) 2624 (+0.0%) 640 (+0.0%)
---
Also please excuse the mess, this is going to be a rough series of
commits...
This is _not_ free, so an opt-in define is needed.
This is also expected to fail right now due to balance issues with
diverging yellow nodes in range-removals, but it's the first step
towards trying to fix said balance issues.
I've been trying to avoid adding debug machinery to lfs.c itself, since
it tends to hurt readability, but I really don't know how you could
assert rbyd balance out-of-tree short of trying to fetch every block in
emubd somehow...
Oh well, finding and asserting rbyd balance in lfsr_rbyd_fetch at least
doesn't require that much code.
No code changes, thanks to const-propagation.
altas, and to a lesser extend altns, are just too problematic for our
rbyd-append algorithm.
Main issue is these break our "narrowing" invariant, where each alt only
ever decreases the bounds.
I wanted to use altas to simplify lfsr_rbyd_appendcompaction, but
decided it wasn't worth it. Handling them correctly would require adding
a number of special cases to lfsr_rbyd_appendrat, adding complexity to
an already incredibly complex function.
---
Fortunately, we don't really need altns/altas on-disk, but we _do_ need
a way to mark alts as unreachable internally in order to know when we
can collapse alts when recoloring (at this point bounds information is
lost).
I was originally going to use the alt's sign bit for this, but it turns
out we already have this information thanks to setting jump=0 to assert
that an alt is unreachable. So no explicit flag needed!
This ends up saving a surprising amount of code for what is only a
couple lines of changes:
code stack ctx
before: 38512 2624 640
after: 38440 (-0.2%) 2624 (+0.0%) 640 (+0.0%)
In lfsr_mtree_traverse, we were explicitly not checking btree nodes when
ckfetches was enabled. This was an attempt to avoid double-checking btree
nodes when using both ckfetches and ckmeta.
Unfortunately this misses errors introduced _after_ fetch in btrees we
keep around in RAM. We still want to find these when doing an explicit
ckmeta scan!
---
Removing this special case _does_ mean we will be back to redundantly
checking non-cached btree nodes, but I think this is a case of better
safe than sorry.
In the future we may cache btree nodes more aggressively depending on
available RAM, and it would be a shame if that accidentally leads to a
weaker filesystem.
Worst case we can always revisit this ckfetches/ckmeta double-fetching
in the future if it become a bottleneck.
Found by our test_ck_spam_fwrite_fuzz test.
This actually saves a tiny bit of code, but at the cost of redundantly
checking non-cached btree nodes:
code stack ctx
default before: 38512 2624 640
default after: 38512 (+0.0%) 2624 (+0.0%) 640 (+0.0%)
ckfetches before: 38766 2656 640
ckfetches after: 38762 (-0.0%) 2656 (+0.0%) 640 (+0.0%)
This fixes a minor balance issue with rbyd commits after compaction,
reducing the worst case rbyd height by ~1/2.
I've been noticing some strange balance issues in our rbyds for a while,
it turns out our algorithms for compaction and commit weren't playing
nicely with each other.
The problem is always terminating with altles, and subtleties around
incomplete trees (non-powers-of-two).
When we terminate with altles, we end up with a dangling null tag:
rbyd log: rbyd tree:
data a <. .---> a
data b <--. .---r-b-> b
data c <----. | .---> c
data d <------. .---r-b-r-b-> d
data e <--------. | .---b---b-> e
altrle a <. | | | | r-b---------> null
altble b -|-' | | |
null | | | |
altrle c <--.-' | |
altble d -|-|---' |
null | | |
altble e <----.---'
null | | |
altrle b <. | |
altble d -|-' |
null | |
altble e <--.-'
null | |
altrle d -' |
altble e ---'
null
Which is fine, we're allowed to terminate rbyds with a null tag.
The problem is that from the rbyd's view of the world, that null tag has
the same balance as the rest of the entire tree. If we continue to
append tags (which is a very common thing to do), we end up with a
lopsided tree:
rbyd log: rbyd tree:
data a <. .---> a
data b <--. .---r-b-> b
data c <----. | .---> c
data d <------. .---r-b-r-b-> d
data e <--------. | .---b---b-> e
altrle a <. | | | | r-b---------> null
altble b -|-' | | | v
null | | | | .---> a
altrle c <--.-' | | .---r-b-> b
altble d -|-|---' | | .---> c
null | | | .---r-b-r-b-> d
altble e <----.---' | .---b---b-> e
null | | | r-b---------> f
altrle b <. | | v
altble d -|-' | .---> a
null | | .---r-b-> b
altble e <--.-' | .---> c
null | | .-----r-b-r-b-> d
altrle d | | | .-----b---b-> e
altble e | | | | .---------> f
null | | y-r-b---------> g
altrle d | | v
altble e | | .---> a
data f <--. .---r-b-> b
altrle d <. | | .---> c
altrle e -|-| .---r-b-r-b-> d
altble f | | .-y-r---b---b-> e
data g <----. | .-----------> f
altble e | | | | | .---------> g
altrle f | | | b-r-b---------> h
altble g | | | v
data h <------. .---> a
altble e -' | | | .---r-b-> b
altrle f ---' | | | .---> c
altrle g -----' | .---r-b-r-b-> d
altble h -------' .---y-r---b---b-> e
data i | .-------------> f
| | .-----------> g
| | | .---------> h
b-y-r-b---------> i
'---- h=4 --------'
Fortunately the solution is relatively simple. If we terminate each
layer of the compaction with an altgt, instead of an altle, the null
tag becomes unreachable:
rbyd log: rbyd tree:
data a <. .---> a
data b <--. .---r-b-> b
data c <----. | .---> c
data d <------. .---r-b-r-b-> d
data e <--------. r-b---b---b-> e
altrle a <. | | | |
altble b -|-' | | |
null | | | |
altrle c <--.-' | |
altble d -|-|---' |
null | | |
altbgt d <----.---'
null | | |
altrle b <. | |
altble d -|-' |
null | |
altbgt d <--.-'
null | |
altrle d -' |
altbgt d ---'
null
This preserves the balance of the tree:
rbyd log: rbyd tree:
data a <. .---> a
data b <--. .---r-b-> b
data c <----. | .---> c
data d <------. .---r-b-r-b-> d
data e <. | | | r-b---b---b-> e
altrle a -| | | | v
altble b -|-' | | .---> a
null | | | .---r-b-> b
altrle c <--.-' | | .---> c
altble d -|-|---' .-r-b-r-b-> d
null | | | .-> e
altbgt d | | b---b---b-> f
null | | v
altrle b <. | .---> a
altble d -|-' .---r-b-> b
null | | .---> c
altbgt d | .-r-b-r-b-> d
null | | .---> e
altrle d | | | .-> f
altbgt d | b---b-r-b-> g
null | v
altble d | .---> a
altbn d | .-----r-b-> b
altble e | | .---> c
data f <|-. .-r-b---r-b-> d
altble d | | | .-----> e
altbn | | | | .---> f
altrle e | | | | | .-> g
altble f | | b---b-y-r-b-> h
data g <----. v
altble d | | | .---> a
altbn | | | .---r-b-> b
altrle e <--. | | .---> c
altrle f -|-| | .-r-b-r-b-> d
altble g | | | | .---> e
data h <------. | .-y-r-> f
altble d -' | | | | | .---> g
altble f ---' | | | | | .-> h
altrle g -----' | b---b-r-b-> i
altble h -------'
data i '---- h=3 --'
As a plus, this also makes it a bit easier to see compaction layers
without decoding jumps, which is nice for debugging.
---
It's worth noting the subtlety around when use use altgts vs altles
here. We need to use altles in all but the last tag, in order to know
what the largest tag was in each subtree when building the next layer.
Fortunately, the last tag is the only tag where we need an altgt in
order to make the null tag unreachable.
This adds a bit of code, but preserving rbyd balance after compaction is
well worth it:
code stack ctx
before: 38500 2624 640
after: 38512 (+0.0%) 2624 (+0.0%) 640 (+0.0%)
This just exposes the gcksum to the user, but exposing the gcksum allows
the user to store it externally for an extra layer of protection against
filesystem corruption.
As far as I'm aware this is the only real way to protect against global
rollback issues, which is a problem for any filesystem with logs (aka
any powerloss-resilient filesystem).
This required a comically small amount of code:
code stack ctx
before: 38492 2624 640
after: 38500 (+0.0%) 2624 (+0.0%) 640 (+0.0%)
This is a bit on the edge of what is a reasonable to shove in
lfs_util.h. But it's useful enough that we were giving it its own
function anyways, and moving it to lfs_util.h allows it to be
potentially overriden with cube-specific optimizations (expanded
terms?).
No code changes.
This drops the last lfsr_gdelta_* function, which were really just a
bunch of somewhat-quirky mem operations.
Moving this to lfs_util.h also allows users to override it with
hardware-specific tricks, though I think hardware tricks for lfs_memlen
will be quite rare. Reverse-order memory optimizations are pretty
uncommon...
This could also be done with a theoretical memrcchr, if one existed:
p = memrcchr(buffer, 0, size);
return (p) ? p - buffer : 0;
But I figured this use case is so niche we might as well just limit it
to c=0 (and avoid questions about memrchr).
No code changes.
This doesn't really make sense if you don't have pmul hardware, but if
you _do_ have pmul hardware, a pmul-based crc32c significantly
out-performs a table-based approach with less code cost.
Measured with GCC 11 using -Os -mcpu=cortex-m55 in QEMU, _not_
cycle-accurate:
code stack ins ld/st branch
naive 48 12 221192 4099 36865
small-table 124 12 49160 12291 4097
big-table 1064 8 32776 8195 4097
pmul-naive-1x32 152 52 622603 5123 68609
pmul-tuned-8x16 316 72 6364 266 783
Unfortunately the m55 is a bit of a weird case, with only 16-bit pmuls,
but the ability to do 8 pmuls simultaneously, so it doesn't start
out-performing a table-based approach until leveraging SIMD. (All I want
is a reg<->reg word-sized pmul, but I guess that is a hard ask...)
Still, this is a useful proof-of-concept and may be useful for different
hardware.
We really shouldn't have two names for the same thing, it just makes
things more confusing, even if the public name doesn't quite match the
internal usage. Especially now that we internally rely on these being
the same flag.
This renames LFS_i_UNTIDY -> LFS_I_MKCONSISTENT and drops the untidy/
mktidy naming internally.
No code changes.
lfs->seed was already just the rough xor of all mdir cksums, so
replacing it with our gcksum doesn't really do anything but make its
definition more rigorous.
That and save both code and ctx:
code stack ctx
before: 38560 2624 644
after: 38492 (-0.2%) 2624 (+0.0%) 640 (-0.6%)
This finally gets rid of the annoying grm re-xoring we needed to do in
lfsr_mdir_commit.
If we're waiting until the last minute to append gdeltas, we might as
well wait to the last minute to encode/xor gdeltas. This avoids any
redundant xors because of failed mdir commits.
Note though, we still need our *_d copies to collect gdelta from any
dropped mdirs.
Also dropped lfsr_gdelta_xor and lfsr_is_zero. These were just small
wrappers over memxor and memcchr that add more noise than anything else.
This saves a nice bit of code/stack:
code stack ctx
before: 38588 2648 644
after: 38560 (-0.1%) 2624 (-0.9%) 644 (+0.0%)
It's not the most intuitive, but we always know if an mdir commit will
be the last commit/atomic before attempting the commit. This means we
can wait to append/flush gdelta until the last commit, which simplifies
handling of gdelta state in lfsr_mdir_commit a bit.
We were already doing this for the gcksum, this just changes the grm to
match.
This also results in gdeltas being pushed upwards when relocating, which
is probably a good thing? On one hand this encourages deduplication of
gdeltas, on the other hand it does result in gdeltas getting pushed into
mroots that don't need them (grm in mrootanchor?).
This ends up saving a bit of code, which is nice:
code stack ctx
before: 38612 2648 644
after: 38588 (-0.1%) 2648 (+0.0%) 644 (+0.0%)
Syncing mdirs got a bit awkward with the addition of the gcksumdelta, we
can't just copy the rbyd component. So added lfsr_mdir_sync to abstract
out the copying of any relevent mdir state.
Note this does _not_ include the mid, which may be different for
different files in the same mdir.
The code deduplication ended up saving a bit of code, which is nice:
code stack ctx
before: 38668 2648 644
after: 38612 (-0.1%) 2648 (+0.0%) 644 (+0.0%)
This tweaks lfsr_rbyd_appendcksum_ to take the canonical cksum as an
argument, allowing the caller to append any non-tree tags they'd like
before finalizing the commit.
This lets us calculate our gcksumdelta directly in lfsr_mdir_commit__,
which simplifies things a bit and avoids a redundant cksum copy.
The code cost mostly cancels out due to the extra gcksumdelta append
needed in lfsr_formatinited, but it's worth it for the code
simplification:
code stack ctx
before: 38664 2648 644
after: 38668 (+0.0%) 2648 (+0.0%) 644 (+0.0%)
These were mostly just overlooked. We should use LFS_WARN on unexpected
filesystem state and LFS_ERROR if we can no longer proceed:
- ckprog mismatch: LFS_DEBUG -> LFS_WARN
- stuck mdir: LFS_DEBUG -> LFS_ERROR
- mdir cksum mismatch: LFS_DEBUG -> LFS_ERROR
This changes lfsr_mtree_traverse + CKMETA to check on-disk mdir
checksums against all open mdirs and the mroot (which is always "open").
This is basically free (a bit more CPU work, but no disk accesses), and
makes lfsr_mtree_traverse a bit more robust against rollback issues. It
also lets us drop the obshrub-specific mdir cksum check.
This adds a bit of code, but it's probably worth it to catch rollback
issues earlier:
code stack ctx
before: 38572 2640 644
after: 38664 (+0.2%) 2648 (+0.3%) 644 (+0.0%)
It doesn't really make sense to write to disk/trace files with multiple
threads, the result usually ends up clobbered and useless.
If we only pass disk/trace files to the first thread, the result is at
at least useable, even if it only represents 1/j tests.
This is actually quite a nice way to sample filesystem images in
multithreaded tests.
As a side effect, this also changes test.py/bench.py to no longer pass
-d/--disk or -t/--trace to runner queries, which is probably a good
thing? These should be ignored in queries anyways.
It's an error to check the gcksum if our traversal ends up
dirty/mutated, since it may be contain cksums from different filesystem
states.
We already avoid clearing the CKMETA flag if this happens, so no change
needed there. lfsr_fs_gc will continue to traverse until this flag is
cleared. (Though running a ckmeta scan to completion before any mutation
_would_ be more robust, hmm...)
Found by our test_traversal_mutation test.
Code changes were zero after tweaking lfsr_mtree_traverse to better
deduplicate ckmeta/ckdata eot conditions:
code stack ctx
before: 38572 2640 644
after: 38572 (+0.0%) 2640 (+0.0%) 644 (+0.0%)
- In lfsr_mtree_traverse, we traverse open file bshrubs/btrees before
we validate the gcksum, which means bugs/asserts can slip through
before we have a chance to detect something is wrong.
To work around this, I've added an explicit mdir cksum check right
before we start traversing an open mdir's bshrubs/btrees. If an open
mdir doesn't match the on-disk state, the on-disk state must contain
an error (or the RAM, but that's a different story and wayyy out of
scope).
It might be better to rearrange lfsr_mtree_traverse to check gcksums
first, but this will require another look at our traversal clobbering
logic.
- For a similar reason, ckfetches can't detect open bshrub/btree
corruption as is. As its name suggests, ckfetches only checks fetches,
so any corruption after we've fetched bshrubs/btrees in lfsr_file_open
will go undetected.
Fortunately this just means we need a full ckmeta-scan in
test_ck_spam* tests that keep open files.
In real use, full ckmeta-scans should be preferred anyways. Limiting
these scans to mtreeonly was just an attempt to better stress btree
ckfetches.
At least we're still testing ckmeta+mtreeonly+ckfetches in
test_ck_spam_dir_fuzz and test_ck_spam_file_fuzz.
This gets the test_ck_spam* tests running under all of the current
interesting ck-modes.
Code changes:
code stack ctx
before: 38560 2640 644
after 38572 (+0.0%) 2640 (+0.0%) 644 (+0.0%)
Now that gcksums are working and we can detect rollback issues, it's
worth revisiting our most aggressive bit-error tests.
Unfortunately, I think due to focusing on ckprogs, these were a bit less
ready-to-go than I had hoped. We still have the read-hole, so the sort
of errors we can expect to detect is a bit limited.
Still, managed to come up with some schemes that I think are
interesting:
- ckprogs - Limited to catching bit-errors during progs, but these tests
work great.
- ckdata - Limited to manual bit-errors, but can detect both metdata +
data errors.
- ckmeta+ckfetches - Limited to manual bit-errors, ckmeta detects
mtree errors, while ckfetches detects btree + data errors.
- ckmeta+ckdatacksums - Limited to manual bit-errors, ckmeta detects
metadata errors, while ckdatacksums detects data errors.
To make testing manual bit-errors a bit easier, and to avoid
reimplementing the bit randomizer in emubd, I added
LFS_EMUBD_BADBLOCK_MANUAL and lfs_emubd_flip to let the tests manually
control when bits flip.
---
Unfortunately open files are proving to be an issue for these tests,
since we don't really expect corrupted metadata after lfsr_file_open (
assuming no read-hole).
For now I've limited these new ck-modes to the tests without open files,
but we should probably revisit this.
This adds a check that the on-disk gcksum matches the in-RAM gcksum
in lfsr_mtree_traverse, so ckmeta/ckdata scans should now be able to
at least detect global-rollback issues that occur while mounted.
This also moves the LFS_I_CKMETA/CKDATA flag clearing logic from
lfsr_mtree_gc -> lfsr_mtree_traverse. There's no reason to not clear
these flags if we've made a successful traversal. We weren't actually
calling lfsr_mtree_traverse with the right flags for this to matter, but
it does let us drop an explicit flag clear in lfsr_fs_ck.
---
These changes were a part of adding the harder versions of our ckmeta/
ckdata tests, where we flip individual bits instead of clobbering the
entire block. These are more realistic errors and stress our gcksum
system.
Recalculating the gcksum required another gcksum copy in
lfsr_traversal_t, which adds a bit of code and ctx to our incremental-gc
build:
code stack ctx
default before: 38428 2640 644
default after: 38560 (+0.3%) 2640 (+0.0%) 644 (+0.0%)
gc before: 38484 2640 788
gc after: 38616 (+0.3%) 2640 (+0.0%) 792 (+0.5%)
Unfortunately we can't easily abuse the copies in lfs_t since
multiple traversals may be open at once.
This was quite a puzzle.
The problem: How do we detect corrupt mdirs?
Seems like a simple question, but we can't just rely on mdir cksums. Our
mdirs are independently updateable logs, and logs have this annoying
tendency to "rollback" to previously valid states when corrupted.
Rollback issues aren't littlefs-specific, but what _is_ littlefs-
specific is that when one mdir rolls back, it can disagree with other
mdirs, resulting in wildly incorrect filesystem state.
To solve this, or at least protect against disagreeable mdirs, we need
to somehow include the state of all other mdirs in each mdir commit.
---
The first thought: Why not use gstate?
We already have a system for storing distributed state. If we add the
xor of all of our mdir cksums, we can rebuild it during mount and verify
that nothing changed:
.--------. .--------. .--------. .--------.
.| mdir 0 | .| mdir 1 | .| mdir 2 | .| mdir 3 |
|| | || | || | || |
|| gdelta | || gdelta | || gdelta | || gdelta |
|'-----|--' |'-----|--' |'-----|--' |'-----|--'
'------|-' '------|-' '------|-' '------|-'
'--.------' '--.------' '--.------' '--.------'
cksum | cksum | cksum | cksum |
| | v | v | v |
'---------> xor -------> xor -------> xor -------> gcksum
| v v v =?
'---------> xor -------> xor -------> xor ---> gcksum
Unfortunately it's not that easy. Consider what this looks like
mathematically (g is our gcksum, c_i is an mdir cksum, d_i is a
gcksumdelta, and +/-/sum is xor):
g = sum(c_i) = sum(d_i)
If we solve for a new gcksumdelta, d_i:
d_i = g' - g
d_i = g + c_i - g
d_i = c_i
The gcksum cancels itself out! We're left with an equation that depends
only on the current mdir, which doesn't help us at all.
Next thought: What if we permute the gcksum with a function t before
distributing it over our gcksumdeltas?
.--------. .--------. .--------. .--------.
.| mdir 0 | .| mdir 1 | .| mdir 2 | .| mdir 3 |
|| | || | || | || |
|| gdelta | || gdelta | || gdelta | || gdelta |
|'-----|--' |'-----|--' |'-----|--' |'-----|--'
'------|-' '------|-' '------|-' '------|-'
'--.------' '--.------' '--.------' '--.------'
cksum | cksum | cksum | cksum |
| | v | v | v |
'---------> xor -------> xor -------> xor -------> gcksum
| | | | .--t--'
| | | | '-> t(gcksum)
| v v v =?
'---------> xor -------> xor -------> xor ---> t(gcksum)
In math terms:
t(g) = t(sum(c_i)) = sum(d_i)
In order for this to work, t needs to be non-linear. If t is linear, the
same thing happens:
d_i = t(g') - t(g)
d_i = t(g + c_i) - t(g)
d_i = t(g) + t(c_i) - t(g)
d_i = t(c_i)
This was quite funny/frustrating (funnistrating?) during development,
because it means a lot of seemingly obvious functions don't work!
- t(g) = g - Doesn't work
- t(g) = crc32c(g) - Doesn't work because crc32cs are linear
- t(g) = g^2 in GF(2^n) - g^2 is linear in GF(2^n)!?
Fortunately, powers coprime with 2 finally give us a non-linear function
in GF(2^n), so t(g) = g^3 works:
d_i = g'^3 - g^3
d_i = (g + c_i)^3 - g^3
d_i = (g^2 + gc_i + gc_i + c_i^2)(g + c_i) - g^3
d_i = (g^2 + c_i^2)(g + c_i) - g^3
d_i = g^3 + gc_i^2 + g^2c_i + c_i^3 - g^3
d_i = gc_i^2 + g^2c_i + c_i^3
---
Bleh, now we need to implement finite-field operations? Well, not
entirely!
Note that our algorithm never uses division. This means we don't need a
full finite-field (+, -, *, /), but can get away with a finite-ring (+,
-, *). And conveniently for us, our crc32c polynomial defines a ring
epimorphic to a 31-bit finite-field.
All we need to do is define crc32c multiplication as polynomial
multiplication mod our crc32c polynomial:
crc32cmul(a, b) = pmod(pmul(a, b), P)
And since crc32c is more-or-less just pmod(x, P), this lets us take
advantage of any crc32c hardware/tables that may be available.
---
Bunch of notes:
- Our 2^n-bit crc-ring maps to a 2^n-1-bit finite-field because our crc
polynomial is defined as P(x) = Q(x)(x + 1), where Q(x) is a 2^n-1-bit
irreducible polynomial.
This is a common crc construction as it provides optimal odd-bit/2-bit
error detection, so it shouldn't be too difficult to adapt to other
crc sizes.
- t(g) = g^3 is not the only function that works, but it turns out to be
a pretty good one:
- 3 and 2^(2^n-1)-1 are coprime, which means our function t(g) = g^3
provides a one-to-one mapping in the underlying fields of all crc
rings of size 2^(2^n).
We know 3 and 2^(2^n-1)-1 are coprime because 2^(2^n-1)-1 =
2^(2^n)-1 (a Fermat number) - 2^(2^n-1) (a power-of-2), and 3
divides Fermat numbers >=3 (A023394) and is not 2.
- Our delta, when viewed as a polynomial in g: d(g) = gc^2 + g^2c +
c^3, has degree 2, which implies there are at most 2 solutions or
1-bit of information loss in the underlying field.
This is optimal since the original definition already had 2
solutions before we even chose a function:
d(g) = t(g + c) - t(g)
d(g) = t(g + c) - t((g + c) - c)
d(g) = t((g + c) + c) - t(g + c)
d(g) = d(g + c)
Though note the mapping of our crc-ring to the underlying field
already represents 1-bit of information loss.
- If you're using a cryptographic hash or other non-crc, you should
probably just use an equal sized finite-field.
Though note changing from a 2^n-1-bit field to a 2^n-bit field does
change the math a bit, with t(g) = g^7 being a better non-linear
function:
- 7 is the smallest odd-number coprime with 2^n-1, a Fermat number,
which makes t(g) = g^7 a one-to-one mapping.
3 humorously divides all 2^n-1 Fermat numbers.
- Expanding delta with t(g) = g^7 gives us a 6 degree polynomial,
which implies at most 6 solutions or ~3-bits of information loss.
This isn't actually the best you can do, some exhaustive searching
over small fields (<=2^16) suggests t(g) = g^(2^(n-1)-1) _might_ be
optimal, but that's a heck of a lot more multiplications.
- Because our crc32cs preserve parity/are epimorphic to parity bits,
addition (xor) and multiplication (crc32cmul) also preserve parity,
which can be used to show our entire gcksum system preserves parity.
This is quite neat, and means we are guaranteed to detect any odd
number of bit-errors across the entire filesystem.
- Another idea was to use two different addition operations: xor and
overflowing addition (or mod a prime).
This probably would have worked, but lacks the rigor of the above
solution.
- You might think an RS-like construction would help here, where g =
sum(c_ia^i), but this suffers from the same problem:
d_i = g' - g
d_i = g + c_ia^i - g
d_i = c_ia^i
Nothing here depends on anything outside of the current mdir.
- Another question is should we be using an RS-like construction anyways
to include location information in our gcksum?
Maybe in another system, but I don't think it's necessary in littlefs.
While our mdir are independently updateable, they aren't _entirely_
independent. The location of each mdir is stored in either the mtree
or a parent mdir, so it always gets mixed into the gcksum somewhere.
The only exception being the mrootanchor which is always at the fixed
blocks 0x{0,1}.
- This does _not_ catch "global-rollback" issues, where the most recent
commit in the entire filesystem is corrupted, revealing an older, but
still valid, filesystem state.
But as far as I am aware this is just a fundamental limitation of
powerloss-resilient filesystems, short of doing destructive
operations.
At the very least, exposing the gcksum would allow the user to store
it externally and prevent this issue.
---
Implementation details:
- Our gcksumdelta depends on the rbyd's cksum, so there's a catch-22 if
we include it in the rbyd itself.
We can avoid this by including it in the commit tags (actually the
separate canonical cksum makes this easier than it would have been
earlier), but this does mean LFSR_TAG_GCKSUMDELTA is not an
LFSR_TAG_GDELTA subtype. Unfortunate but not a dealbreaker.
- Reading/writing the gcksumdelta gets a bit annoying with it not being
in the rbyd. For now I've extended the low-level lfsr_rbyd_fetch_/
lfsr_rbyd_appendcksum_ to accept an optional gcksumdelta pointer,
which is a bit awkward, but I don't know of a better solution.
- Unlike the grm, _every_ mdir commit involves the gcksum, which means
we either need to propagate the gcksumdelta up the mroot chain
correctly, or somehow keep track of partially flushed gcksumdeltas.
To make this work I modified the low-level lfsr_mdir_commit__
functions to accept start_rid=-2 to indicate when gcksumdeltas should
be flushed.
It's a bit of a hack, but I think it might make sense to extend this
to all gdeltas eventually.
The gcksum cost both code and RAM, but I think it's well worth it for
removing an entire category of filesystem corruption:
code stack ctx
before: 37796 2608 620
after: 38428 (+1.7%) 2640 (+1.2%) 644 (+3.9%)
Indention has changed quite a bit for most of these, so I figured it's a
good time to go back through and clean things up.
Also tried to adopt indention rules a bit better, adding indention after
string continuation. This was adopted in the scripts but not in the
actual code.
The gcksum isn't actually implemented yet, I mostly just wanted to
measure this code cost separately:
code stack ctx
before: 37768 2608 620
after: 37796 (+0.1%) 2608 (+0.0%) 620 (+0.0%)
I may be procrastinating a little bit...
Now that we perturb commit cksums with the odd-parity zero, the q-bit no
longer serves a purpose other than extra debug info. But this is a
double-edged sword, because redundant info just means another thing that
can go wrong.
For example, should we assert? If the q-bit doesn't reflect the
previous-perturb state it's a bug, but the only thing that would break
would be the q-bit itself. And if we don't assert what's the point of
keeping the q-bit around?
Dropping the q-bit avoids answering this question and saves a bit of
code:
code stack ctx
before: 37772 2608 620
after: 37768 (-0.0%) 2608 (+0.0%) 620 (+0.0%)
Most of littlefs's metadata is encoded in leb128s now, with the
exception of tags (be16, sort of), revision counts (le32), cksums
(le32), and flags.
It makes sense for tags to be a special case, these are written and
rewritten _everywhere_, but less so for flags, which are only written to
the mroot and updated infrequently.
We might as well save a bit of code by reusing our le32 machinery.
---
This changes lfsr_format to just write out compat flags as le32s, saving
a tiny bit of code at the cost of a tiny bit of disk usage (the real
benefit being a tiny bit of code simplification):
code stack ctx
before: 37792 2608 620
after: 37772 (-0.1%) 2608 (+0.0%) 620 (+0.0%)
Compat already need to handle trailing zeros gracefully, so this doesn't
change anything at mount time.
Also had to switch from enums to #defines thanks to C's broken enums.
Wooh. We already use #defines for the other flags for this reason.
Christopher Haster