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.
LFS_WCOMPAT_RDONLY seems generally useful for tools that just want to
mark a filesystem is read-only. This is a common flag that exists in
other filesystems (RO_COMPAT_READONLY in ext4 for example).
LFS_RCOMPAT_WRONLY, on the other hand, is a bit more of a joke, but
there could be some niche use cases for it (preventing double mounts?).
Fortunately, these flags require no extra code, and fall out naturally
from our wcompat/rcompat handling.
---
Originally, the idea was to also add LFS_F_RDONLY, to match LFS_M_RDONLY
and set the LFS_WCOMPAT_RDONLY flag during format.
But this doesn't really work with the current API, since lfsr_format
would just give you an empty filesystem you can't write to. Which is a
bit silly.
Maybe we should add something like lfsr_fs_mkrdonly in the future? This
is probably low-priority.
Mainly to add LFS_RCOMPAT_MSPROUT. It makes sense that a littlefs driver
may not want to support mroot-inlined mdirs, and this flag would be the
only way to indicate that. (Currently inlined mdir -> mtree is one way,
but this may not always be the case.)
This also makes space for a couple planned features:
LFS_RCOMPAT_NONSTANDARD 0x00000001 Non-standard filesystem format
LFS_RCOMPAT_WRONLY* 0x00000002 Reading is disallowed
LFS_RCOMPAT_GRM 0x00000004 May use a global-remove
LFS_RCOMPAT_MSPROUT 0x00000010 May use an inlined mdir
LFS_RCOMPAT_MLEAF 0x00000020 May use a single mdir pointer
LFS_RCOMPAT_MSHRUB 0x00000040 May use an inlined mtree
LFS_RCOMPAT_MTREE 0x00000080 May use an mdir btree
LFS_RCOMPAT_BSPROUT 0x00000100 Files may use inlined data
LFS_RCOMPAT_BLEAF 0x00000200 Files may use single block pointers
LFS_RCOMPAT_BSHRUB 0x00000400 Files may use inlined btrees
LFS_RCOMPAT_BTREE 0x00000800 Files may use btrees
*Planned
I've gone ahead and included rcompat flags we reserve but don't
currently use (LFS_RCOMPAT_MSHRUB). It seems like a good idea to make
these reservations explicit. Though we should still prohibit their use
until there is a good reason, in case we want to repurpose these flags
in the future.
Code changes minimal (larger literal? compiler noise?):
code stack ctx
before: 37788 2608 620
after: 37792 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
This is mainly to free up space for flags, we're pretty close to running
out of 32-bits with future planned features:
1. Reduced file type info from 8 -> 4 bits
We don't really need more than this, but it does mean type info is
no longer a simple byte load.
2. Moved most internal file-state flags into the next 4 bits
These are mostly file-type specific (except LFS_o_ZOMBIE), so we
don't need to worry too much about overlap.
3. Compacted ck-flags into 5 bits:
LFS_M_CKPROGS 0x00000800
LFS_M_CKFETCHES 0x00001000
LFS_M_CKPARITY 0x00002000
LFS_M_CKMETAREDUND* 0x00004000
LFS_M_CKDATACKSUMS 0x00008000
*Planned
Now that ck-flags are a bit more mature, it's pretty clear we'll
probably never have CKMETACKSUMS (ckcksums + small tag reads is
crazy expensive) or CKDATAREDUND (non-trivial parity fanout makes
this crazy expensives. So reserving bits for these just wastes bits.
This also moves things around so ck-flags no longer overlap with open
flags.
It's a tight fit, and I still think file-specific ck-flags are out-of-
scope, but this at least decreases flag ambiguity.
New jenga:
8 8 8 8
.----++----++----++----.
.-..-..-.-------.------.
o_flags: |t||f||t| | o |
|-||-||-|-------:--.---'
|-||-||-'--.----.------.
t_flags: |t||f|| t | | tstt |
'-''-'|----|----'------'
.----.|----|.--.:--:.--.
m_flags: | f || t ||c ||o ||m |
|----||-.--'|--|'--''--'
|----||-|---|--|.------.
f_flags: | f ||t| |c || f |
'----''-'---'--''------'
Fortunately no major code costs:
code stack ctx
before: 37792 2608 620
after: 37788 (-0.0%) 2608 (+0.0%) 620 (+0.0%)
dbgerr.py and dbgtag.py have proven to be incredibly useful for quick
debugging/introspection, so I figured why not have more of that.
My favorite part is being able to quickly see all flags set on an open
file handle:
(gdb) p file.o.o.flags
$2 = 24117517
(gdb) !./scripts/dbgflags.py o 24117517
LFS_O_WRONLY 0x00000001 Open a file as write only
LFS_O_CREAT 0x00000004 Create a file if it does not exist
LFS_O_EXCL 0x00000008 Fail if a file already exists
LFS_O_DESYNC 0x00000100 Do not sync or recieve file updates
LFS_o_REG 0x01000000 Type = regular-file
LFS_o_UNFLUSH 0x00100000 File's data does not match disk
LFS_o_UNSYNC 0x00200000 File's metadata does not match disk
LFS_o_UNCREAT 0x00400000 File does not exist yet
The only concern is if dbgflags.py falls out-of-sync often, I suspect
flag encoding will have quite a bit more churn than flags/tags. But we
can always drop this script in the future if this turns into a problem.
---
While poking around this also ended up with a bunch of other small
changes:
- Added LFS_*_MODE masks for consistency with other "type<->flag
embeddings"
- Added compat flag comments
- Adopted lowercase prefix for internal flags (LFS_o_ZOMBIE), though
not sure if I'll keep this yet...
- Tweaked dbgerr.py to also match ERR_ prefixes and to ignore case
Since we dropped lfsr_gc_setflags/setsteps, it was no longer possible to
set gc_flags to zero (perfectly valid and useful for system bringup/
testing things). Supporting gc_flags=0 means it's not possible to
provide a default, but this is probably ok as users need to opt-in to
LFS_GC anyways.
Note that at least gc_steps=0 doesn't make sense, so the default there
is reasonable.
Fixing this also highlighted that gc_flags/steps are no longer mutable,
making the comment in lfs_init out-of-date. Dropping these saves a bit
of lfs_t size, so that's nice.
And then testing also revealed that LFS_GC_CKDATA implying LFS_GC_CKDATA
means it should probably clear the LFS_I_CKMETA flag as well.
---
And here I thought this was going to be just a simple test-writing
exercise!
Code changes:
code stack ctx
default before: 37792 2608 620
default after: 37792 (-0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37896 2608 768
gc after: 37848 (-0.1%) 2608 (+0.0%) 760 (-1.0%)
This just makes lfsr_fs_stat and lfsr_fs_gc_ that much simpler, at the
risk of the duplicate state falling out-of-sync.
Some minor code savings:
code stack ctx
before: 37804 2608 620
after: 37792 (-0.0%) 2608 (+0.0%) 620 (+0.0%)
The argument for this flag is pretty brittle. Yes it's _technically_
possible to end up with a compactable filesystem during lfsr_format, but
it's pretty unlikely. And keeping LFS_F_COMPACT around means we'd always
need the lfsr_mtree_gc circuitry in lfsr_format, for such a niche
situation, that can be easily cleaned up in lfsr_mount.
So dropping for now.
No code changes, but this does mean one less feature to support:
code stack ctx
before: 37804 2608 620
after: 37804 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
Looking at future planned features, we're running into some real issues
fitting all these flags into 32 bits.
I think the only real use case for LFS_T_MTREEONLY is in
lfsr_traversal_t, where the depth of traversal can't be infered. So no
reason to keep this flag around in the other APIs.
No code changes:
code stack ctx
default before: 37804 2608 620
default after: 37804 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37940 2608 768
gc after: 37940 (+0.0%) 2608 (+0.0%) 768 (+0.0%)
- LFS_I_INCONSISTENT -> LFS_I_MKCONSISTENT
- LFS_I_CANLOOKAHEAD -> LFS_I_LOOKAHEAD
- LFS_I_UNCOMPACTED -> LFS_I_COMPACT
- LFS_I_CANCKMETA -> LFS_I_CKMETA
- LFS_I_CANCKDATA -> LFS_I_CKDATA
This just makes everything easier to read/pattern match, even if it's
a bit inaccurate english-wise. The imperative transformations were also
wildly inconsistent...
Looks like this was never updated after changing the -Y/--summary +
-c/--compare hack to its own -Q/--small-table flag. Fortunately a single
character fix.
Unrelated, but I was considering dropping the make *-diff rules, until
the different compile-time targets proved they are _very_ useful when
jumping around various commits/builds.
- lfsr_gc -> lfsr_fs_gc
- lfsr_gc_unck -> lfsr_fs_unck
lfsr_fs_unck is surprisingly still useful in non-gc builds, since we
still have ckmeta/ckdata state. These flags can still be queried with
lfsr_fs_stat and cleared with lfsr_fs_ckmeta/ckdata/lfsr_traversal_t, so
it seems useful to keep this function around.
It's also a relatively cheap function.
Though this does mean it deserves a rename. Dropping the gc prefix
hopefully makes it clearer this function is not entirely gc-specific.
And since we no longer have lfsr_gc_setflags/setsteps, it makes sense to
rename lfsr_gc back to lfsr_fs_gc, to be consistent with the other
filesystem-wide utilities.
Code changes, apparently lfsr_fs_unck costs 12 bytes:
code stack ctx
default before: 37792 2608 620
default after: 37804 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37938 2608 768
gc after: 37940 (+0.0%) 2608 (+0.0%) 768 (+0.0%)
LFS_GC_CKMETA and LFS_GC_CKDATA are a bit unique in that their work is
never really done.
Where LFS_GC_MKCONSISTENT/COMPACT can prove things about the system,
LFS_GC_CKMETA/CKDATA can't, because it's always possible for new
bit-errors to develop. Even _during_ an LFS_GC_CKMETA/CKDATA traversal.
But while this is technically true, it's not a very useful state of
things for our lfsr_gc API...
---
What we really want is some way to know if ckmeta/ckdata has completed
"recently" (for some definition of recently), and to let users indicate
when they need another ckmeta/ckdata scan.
To try to solve this:
1. Added LFS_I_CANCKMETA and LFS_I_CANCKDATA to indicate when lfsr_gc
has not checked metadata/data.
These are set during mount (unless mounting with
LFS_M_CKMETA/CKDATA), and cleared when either lfsr_gc completes or
lfsr_fs_ckmeta/data is called. Once cleared, littlefs will not reset
them on its own.
2. Added lfsr_gc_unck to allow users to explicitly reset LFS_I_CKMETA
and/or LFS_I_CKDATA, which will tell lfsr_gc to check metadata/data
again on the next call.
There is some subtlety around clobbering ongoing traversals, but a
mask and some tests should prevent this from being a problem.
Currently, lfsr_gc_unck also allows clearing of other gc flags, but
I'm not sure there's any real use-case for this...
Note that you can still get the previous behavior if you just call
lfsr_gc_unck after every lfsr_gc call.
This also changes info flag behavior slightly in default mode, with
LFS_I_CANCKMETA/CANCKDATA telling you if metadata/data has been checked
since mount. Which does seem useful? Maybe these flags deserve a better
name?
Code changes:
code stack ctx
default before: 37796 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
default after: 37792 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37896 2608 768
gc after: 37938 (+0.1%) 2608 (+0.0%) 768 (+0.0.%)
Now that you can provide gc_flags/gc_steps in lfs_config, I think it's a
bit more clear that _mutating_ the flags/steps is a niche feature, and
not worth implementing/testing.
It raises the question why not have a similar lfsr_setflags or
lfsr_file_setflags, and the answer there is it would be a pain-in-the-
ass to make sure all possible corner cases are covered.
It actually already was a pain-in-the-ass to test lfsr_gcsetflags/
setsteps... but just because we already did the work is not a good
reason for keeping complexity around.
---
Note that most of the use cases for lfsr_gc_setflags/setsteps can be
covered by either remounting the filesystem or through the
lfsr_traversal_t APIs directly.
The end result is a bit of code savings when incremental gc is enabled:
code stack ctx
default before: 37796 2608 620
default after: 37796 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37944 2608 768
gc after 37896 (-0.1%) 2608 (+0.0%) 768 (+0.0%)
Incremental gc, being stateful and not gc-able (ironic), was always
going to need to be conditionally compilable.
This moves incremental gc behind the LFS_GC define, so that we can focus
on the "default" costs. This cuts lfs_t in nearly half!
lfs_t with LFS_GC: 308
lfs_t without LFS_C: 168 (-45.5%)
This does save less code than one might expect though. We still need
most of the internal traversal/gc logic for things like block allocation
and orphan cleanup, so most of the savings is limited to the RAM storing
the incremental state:
code stack ctx
before: 37916 2608 768
after with LFS_CFG: 37944 (+0.1%) 2608 (+0.0%) 768 (+0.0%)
after without LFS_CFG: 37796 (-0.3%) 2608 (+0.0%) 620 (-19.3%)
On the flip side, this does mean most of the incremental gc
functionality is still availables in the lfsr_traversal_t APIs.
Applications with more advanced gc use-cases may actually benefit from
_not_ enabling the incremental gc APIs, and instead use the
lfsr_traversal_t APIs directly.
Christopher Haster