May revisit this in the future, but this is the best solution I can
think of right now, that doesn't run into duplicate macro-argument
side-effect issues...
Statement expressions would be another solution, but that's even less
portable!
At the moment this is only used for functions that implement
LFSR_DATA_* and LFSR_RAT_* macros. These _need_ to be inlined to avoid a
large code-size explosion, and GCC seems to have issues with this.
For most of the other inlinable functions, relying on C99's inline +
compiler heuristics seems to be fine.
Code changes:
before: 36304 2576 640
no-forceinline: 36460 (+0.4%) 2664 (+3.4%) 640 (+0.0%)
yes-forceinline: 36300 (-0.0%) 2576 (+0.0%) 640 (+0.0%)
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.
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.
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.
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 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%)
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.
To clarify this only checks data reads, and to makes space for future
theoretical ck-operations:
- ckmetaredund - likely
- ckdataredund - unlikely, expensive
- ckmetacksums - unlikely, expensive
- ckdatacksums - implemented
This also tweaks the relevant mount/format/info flags a bit:
LFS_M_CKPROGS 0x00100000 Check progs by reading back progged data
LFS_M_CKFETCHES 0x00200000 Check block checksums before first use
LFS_M_CKPARITY 0x00400000 Check metadata tag parity bits
LFS_M_CKMETAREDUND+ 0x01000000 Check metadata redund blocks on reads
LFS_M_CKDATAREDUND* 0x02000000 Check data redund blocks on reads
LFS_M_CKMETACKSUMS* 0x04000000 Check metadata checksums on reads
LFS_M_CKDATACKSUMS 0x08000000 Check data checksums on reads
+Planned
*Hypothetical
No code changes.
Since we already need all the machinery to track ck info for ckparity, I
figured we might as well implement a full ckcksums option as well.
Ckcksums closes the checksum-read-hole by reading enough data to check a
relevant checksum on ever read, even if this ends up being significantly
more data than the initial request. This should always detect detectable
bit-errors, even if they occur between consecutive reads.
If this sounds naive, that's because it is. Performance will be awful.
To be clear, ckcksums should probably never be used in production. I
can't think of a use case that isn't better handled by either ECC in the
block device or the future-planned ckredund feature. Just look at the
runtime complexities:
small-reads rbyd-lookup rbyd-compaction
ckcksums: O(b^2) O(b log b) O(b^2 log b)
ckredund*: O(log_b(n) + xb) O(log b) O(b log b)
eccbd*: O(b) O(log b) O(b log b)
* theoretical
We've already seen that O(b^2) compactions turns a performance problem
into a tractability problem, so I think O(b^2 log b) compactions will be
a bit too much for most applications.
We can already seen this in our test_ck_ckcksums_* tests (which do pass
by the way!). Compare to test_ck_ckprogs_*, which is basically the same
set of tests:
test_ck_ckprogs_*: 6.08s
test_ck_ckcksums_*: 64.88s
Or consider test_rbyd with/without ckcksums:
test_rbyd: 12.21s
test_rbyd+ckcksums: 389.94s
Still, ckcksums is an interesting proof-of-concept, and does manage to
close the checksum-read-hole.
---
Like ckprogs/ckfetches/ckparity/etc, ckcksums is an opt-in feature,
requiring both 1. defining LFS_CKCKSUMS and 2. passing LFS_M_CKCKSUMS at
mount time.
Like ckparity, ckcksums requires a significant code and stack increase
to track ck info in lfsr_data_t:
code stack
before: 36416 2616
yes-ckcksums: 38872 (+6.7%) 3176 (+21.4%)
no-ckcksums: 36416 (+0.0%) 2616 (+0.0%)
It's interesting to note how this compares to all of the current
ck-modes, though each has their own set of tradeoffs:
code stack
default: 36416 2616
ckprogs: 36468 (+0.1%) 2616 (+0.0%)
ckfetches: 36666 (+0.7%) 2648 (+1.2%)
ckparity: 37996 (+4.3%) 3040 (+16.2%)
ckcksums: 38872 (+6.7%) 3176 (+21.4%)
---
Note that even though ckcksums is opt-in, it may still be worth removing
from the codebase in the future, for a couple reasons:
- Every feature, even if unused, adds developer/maintenance burden.
- Ck info is particularly messy with how it interacts with all
lfsr_data_t APIs. Though getting rid of ck info would also require
getting rid of ckparity.
- It's possible for a user to see ckcksums in the codebase,
misunderstand its tradeoffs, enable it, and get the impression that
littlefs itself is just unusably slow.
Ckparity is pretty flawed in littlefs, for several reasons. The biggest
one being that we can't even reliably detect single-bit errors.
But! It can still provide an extra layer of safety in a system where you
don't care about the extra code/stack cost.
And, for ckreads, performance cost...
Performance isn't a big problem for parity-checking. We can assume
metadata tags are going to relatively small (and can be controlled by
fragment_size). But for data checksums, ckreads risks O(b^2) when
performing many small reads, which can be a bit of a problem.
And since ckreads doesn't really prove anything interesting about the
system anymore, it makes sense to unbundle these two checks, rename
ckreads -> ckparity, and limit it to only checking parity bits.
This way, you can enable ckparity for a bit of extra safety, with a
code/stack cost hit, but without sacrificing performance.
---
I was hoping more code/stack savings, but since we still need to track
parity context in lfsr_data_t, and still need to intercept bd_read/cmp/
cpy calls that reference metadata, we end up needing to keep most of
the ck circuitry around:
code stack
default before: 36464 2672
default after: 36464 (+0.0%) 2672 (+0.0%)
code stack
ckparity before: 38036 3080
ckparity after: 38024 (-0.0%) 3080 (+0.0%)
We even end up still tracking checksum context for bptrs! Maybe we
should just go ahead and add ckcksums as a joke...
Ckfetches implements what might be your first idea on how to check
checksums in a filesystem: Check each block/mdir on first access
(fetch) to make sure the data is sound.
Unfortunately, there are two problems with this approach, both which
come from the fact that blocks are big and can't fit in RAM:
1. We still have a checksum-read hole.
We can't keep a whole block around in RAM, so reads after a fetch may
need to reread from disk, at which point new bit-errors may slip in
undetected.
This is especially problematic for traversing our rbyds, which
involves a lot of small reads in a block.
2. Ckfetches may have a surprisingly negative performance impact.
Consider the case of reading a large file with a bunch of small
reads. Because we don't cache blocks, each read may need a btree
lookup, and a full block fetch. On paper this can quickly end up
O(b^2), which is not great.
Though this is helped by the file buffer. It will be interesting to
benchmark and see if this theoretical O(b^2) translates to poor
performance in practice.
Note ckreads has this same performance issue.
Still, despite these problems, ckfetches may be useful for cases where
you just want an extra layer of safety, or don't care about the tiny
chance an error is introduced between a fetch an subsequent read.
---
Like ckprogs/ckreads, ckfetches is an opt-in feature, and requires both
1. defining LFS_CKFETCHES, and 2. passing LFS_M_CKFETCHES during mount.
This is a bit of a quick implementation to get testing in place, so the
code cost is probably higher than strictly necessary. If we can refactor
the code internally to avoid all the duplicate lfsr_rbyd_fetchck/
lfsr_bptr_ck calls, we can probably bring this down a bit:
code stack
before: 36428 2680
yes-ckfetches: 36848 (+1.2%) 2680 (+0.0%)
no-ckfetches: 36428 (+0.0%) 2680 (+0.0%)
Oh, and also added lfs_emubd_flipbit to allow tests to manually flip
bits themselves. LFS_EMUBD_BADBLOCK_PROGFLIP is quick to find the above
mentioned checksum-read hole.
This could be done manually with read+erase+prog, but no reason to make
it harder than it needs to be.
So just like ckreads, ckprogs is now opt-in, requiring both 1. defining
LFS_CKPROGS at compile-time, and 2. passing the LFS_M_CKPROGS flag
during lfsr_mount.
_Unlike_ ckreads, ckprogs is actually a very lightweight feature. So the
difference between compiling with/without ckprogs is really quite small:
code stack
before: 36480 2680
yes-ckprogs: 36480 (+0.0%) 2680 (+0.0%)
no-ckprogs: 36428 (-0.1%) 2680 (+0.0%)
It's almost not worth putting behind an ifdef if not for consistency
with ckreads.
This moves all ckread-related logic behind the new opt-in compile-time
LFS_CKREADS flag. So in order to use ckreads you need to 1. define
LFS_CKREADS at compile time, and 2. pass LFS_M_CKREADS during
lfsr_mount.
This was always the plan since, even if ckreads worked perfectly, it
adds a significant amount of baggage (stack mostly) to track the
ck context of all reads.
---
This is the first non-trivial opt-in define in littlefs, so more test
framework features!
test.py and build.py now support the optional ifdef attribute, which
makes it easy to indicate a test suite/case should not be compiled when
a feature is missing.
Also interesting to note is the addition of LFS_IFDEF_CKREADS, which
solves several issues (and general ugliness) related to #ifdefs in
expression. For example:
// does not compile :( (can't embed ifdefs in macros)
LFS_ASSERT(flags == (
LFS_M_CKPROGS
#ifdef LFS_CKREADS
| LFS_M_CKREADS
#endif
))
// does compile :)
LFS_ASSERT(flags == (
LFS_M_CKPROGS
| LFS_IFDEF_CKREADS(LFS_M_CKREADS, 0)));
---
This brings us way back down to our pre-ckread levels of code/stack:
code stack
before-ckreads: 36352 2672
ckreads: 38060 (+4.7%) 3056 (+14.4%)
after-ckreads: 36428 (+0.2%) 2680 (+0.3%)
Unfortunately, we do end up with a bit more code cost than where we
started. Mainly due to code moving around to support the ckread
infrastructure:
code stack
lfsr_bd_readtag: +52 (+23.2%) +8 (+10.0%)
lfsr_rbyd_fetch: +36 (+5.0%) +8 (+6.2%, cold)
lfs_toleb128: -12 (-25.0%) -4 (-20.0%, cold)
total: +76 (+0.2%) +8 (+0.3%)
But oh well. Note that some of these changes are good even without
ckreads, such as only parsing the last ecksum tag.
I've been scratching my head over our rbyd perturb scheme. It's gotten
rather clunky with needing to xor valid bits and whatnot.
But it's tricky with needing erased-state to be included in parity bits,
while at the same time excluded from our canonical checksum. If only
there was some way to flip the checksums parity without changing its
value...
Enter the crc32c odd-parity zero: 0xfca42daf!
This bends the definition of zero a bit, but it is one of two numbers in
our crc32c-ring with a very interesting property:
crc32c(m) == crc32c(m xor 0xfca42daf) xor 0xfca42daf // odd-p zero
crc32c(m) == crc32c(m xor 0x00000000) xor 0x00000000 // even-p zero
Recall that crc32c's polynomial, 0x11edc6f41, is composed of two
polynomials: 0x3, the parity polynomial, and 0xf5b4253f, a maximally
sized irreducible polynomial. Because our polynomial breaks down into
two smaller polynomials, our crc32c space turns out to not be a field,
but rather a ring containing two smaller sub-fields. Because these
sub-fields are defined by their polynomials, one is the 31-bit crc
defined by the polynomial 0xf5b4253f, while the other is the current
parity.
We can move in the parity sub-field without changing our position in the
31-bit crc sub-field by xoring with a number that is one in the parity
sub-field, but zero in the 31-bit crc sub-field.
This number happens to be 0xf5b4253f (0xfca42daf bit-reversed)!
(crcs being bit-reversed will never not be annoying)
So long story short, xoring any crc32c with 0xfca42daf will change its
parity but not its value.
---
An that's basically our new perturb scheme. If we need to perturb, xor
with 0xfca42daf to change the parity, and after calculating/validating
the checksum, xor with 0xfca42daf to get our canonical checksum.
Isn't that neat!
There was one small hiccup: At first I assumed you could continue
including the valid bits in the checksum, which would have been nice for
bulk checksumming. But this doesn't work because while valid bits cancel
out so the parity doesn't change, changing valid bits _does_ change the
underlying 31-bit crc, poisoning our checksum and making everything a
mess.
So we still need to mask out valid bits, which is a bit annoying.
But then I stumbled on the funny realization that by masking our valid
bits, we accidentally end up with a fully functional parity scheme.
Because valid bits _don't_ include the previous valid bit, we can figure
out the parity for not only the entire commit, but also each individual
tag:
80 03 00 08 6c 69 74 74 6c 65 66 73 80
^'----------------.---------------' ^
| | |
v + parity = v'
Or more simply:
80 03 00 08 6c 69 74 74 6c 65 66 73 80
'----------------.----------------' ^
| |
parity = v'
Double neat!
Some other notes:
- By keeping the commit checksum perturbed, but not the canonical
checksum, the perturb state is self-validating. We no longer need to
explicitly check the previous-perturb-bit (q) to avoid the perturb
hole we ran into previously.
I'm still keeping the previous-perturb-bit (q) around, since it's
useful for debugging. We still need to know the perturb state
internally at all times in order to xor out the canonical checksum
correctly anyways.
- Thanks to all of our perturb iterations, we now know how to remove the
valid bits from the checksum easily:
cksum ^= 0x00000080 & (tag >> 8)
This makes the whole omitting-valid-bits thing less of a pain point.
- It wasn't actually worth it to perturb the checksum when building
commits, vs manually flipping each valid bit, as this would have made
our internal appendattr API really weird.
At least the perturbed checksum made fetch a bit simpler.
Not sure exactly how to draw this with our perturb scheme diagrams,
maybe something like this?
.---+---+---+---. \ \ \ \
|v| tag | | | | |
+---+---+---+---+ | | | |
| commit | | | | |
| | +-. | | |
+---+---+---+---+ / | | | |
|v|qp-------------->p>p-->p .
+---+---+---+---+ | . . .
| cksum | | . . .
+---+---+---+---+ | . . .
| padding | | . . .
| | | . . .
+---+---+---+---+ | | | |
|v------------------' | | |
+---+---+---+---+ | | |
| commit | +-. | +- rbyd
| | | | | | cksum
+---+---+---+---+ / | +-. /
|v----------------------' | |
+-------+---+---+ / |
| cksum ----------------'
+---+---+---+---+
| padding |
| |
+---+---+---+---+
| erased |
| |
. .
. .
---
Code changes were minimal, saving a tiny bit of code:
code stack
before: 36368 2664
after: 36352 (-0.0%) 2672 (+0.3%)
There was a stack bump in lfsr_bd_readtag, but as far as I can tell it's
just compiler noise? I poked around a bit but couldn't figure out why it
changed...
We don't actually need these, all we need are utils defined for the
largest integer size we operate on, currently uint32_t.
Counterintuitively this should make it easier to adopt different integer
widths in the future.
Or maybe this will bite us when lfs_off_t >> lfs_size_t? Oh well, if
that's the case we can fix it then.
No code changes:
code stack
before: 33886 2560
after: 33886 (+0.0%) 2560 (+0.0%)
The ability of GCC is just insufficient for asserts-as-hints and at some
point it's not worth trying to workaround this.
Code with asserts should never be worse than code without asserts, so we
might as well just disable asserts completely when not debugging:
code stack
hint-assert (before): 33918 2592
no-assert (after): 33882 (-0.1%) 2560 (-1.2%)
Clang does no better here (targeting x86):
code
clang+hint-asserts (before): 51946
clang+no-asserts (after): 51663 (-0.5%)
Maybe in the future some builtin will let us force pure expressions. It
would be interesting to revisit assert driven optimizations at some
point.
The theory is that lfsr_opened_isopen is a relatively special case, and
the main cause of LFS_ASSERT side-effects. All other LFS_ASSERTs are
either limited to simple expressions, or small static-inline functions.
Unfortunately, even without lfsr_opened_isopen asserts, it seems
asserts-as-hints still results in worse code/stack costs:
code stack
no-assert (before): 33626 2552
hint-assert (after): 33670 (+0.1%) 2592 (+1.6%)
Digging around in the low-level assembly, it seems that what is
happening is the increased number of calls to static-inline functions is
causing the compiler to prefer to not-inline functions more often. Then,
even if calls would be eliminated as dead-code, the damage is done to
the containing function.
It's not entirely clear how this could be avoided. Maybe a separate
LFS_ASSERT for only pure expressions? This may not be worth trying to
solve outside of the compiler...
I realized the reason asserting on opened/closed file handles added so
much extra code was because our LFS_ASSERT macro doesn't properly
eliminate side-effects.
Consider this assert:
LFS_ASSERT(lfsr_opened_isopen(lfs, &dir->o));
Expanded:
((lfsr_opened_isopen(lfs, &dir->o))
? (void)0
: __builtin_unreachable());
Even though the compiler knows lfsr_opened_isopen must return true
here, it doesn't know what possible side-effects calling
lfsr_opened_isopen may have, and can't eliminate the function call.
This is quite a bit more obvious if you did something like:
LFS_ASSERT(lfsr_file_sync(&lfs, &file) == 0);
But since lfsr_opened_isopen is a static inline function, it gets a
little bit less clear. Even worse, whether or not the call is eliminated
probably depends if it's actually inlined and other compiler
optimization noise.
---
This commit effectively reverts LFS_ASSERT as a compiler hint, making
LFS_ASSERT an empty string if LFS_NO_ASSERT is defined.
This may not be the optimal solution, but it at least keeps the
programmer's intuition that anything in LFS_ASSERT has zero impact when
asserts are disabled. It would be nice if there was some way to tell the
compiler that an expression should have no side-effects, but as far as
I'm aware this is not currently possible.
Measurements show that just disabling asserts wins in both code and
stack over trying to leverage asserts-as-hints:
code stack
hint-assert (before): 33904 2584
no-assert (after): 33626 (-0.8%) 2552 (-1.2%)
At least both of these win over leaving asserts enabled, so
asserts-as-hints does eliminate _most_ code (measured here
with a simple assert-loop, since I assume that would have the smallest
code footprint):
code stack
loop-assert: 36874 2616
hint-assert (before): 33904 (-8.1%) 2584 (-1.2%)
no-assert (after): 33626 (-8.8%) 2552 (-2.4%)
Unfortunately asserts-as-hints was doing quite a bit of heavy lifting
at preventing overzealous GCC warnings. It took quite a few tweaks to
get GCC to shut up, and I'm still not entirely sure the best way to tell
GCC that some functions only return negative values. Currently I just
limit certain error checks to only check for negative values, which is
not great, but at least gets the code compiling again...
Added:
name builtin? string.h?
lfs_memcmp y y
lfs_memcpy y y
lfs_memmove y y
lfs_memset y y
lfs_memchr y
lfs_memcchr (I wish!)
lfs_memxor
lfs_strlen y
lfs_strcmp y
lfs_strcpy y
lfs_strchr y
lfs_strcchr
lfs_strspn y
lfs_strcspn y
The intention of these is _not_ to try anything better than the stdlib,
but to allow users/integrators to override these functions if string.h
or stdlib.h is not available.
Well... The original motivation was just to add lfs_memcchr to
lfs_utils.h, which is useful for checking if a memory is all zeros, but
then things got a bit out of hand... Oh well, flexibility is good right?
Things get a bit... delicate wrapping memcmp/memcpy/memmove/memset like
this. These functions are basically primitives in C, and the compiler
can get up to all sort of tricks eliding/folding these. Unfortunately,
even just wrapping these in static inline functions seems to create
problems, so I've just defaulted to #defining the relevant lfs_*
symbols.
Even weirder, GCC's __builtin_* variants seem to be worse, code-wise,
than the stdlib symbols. Maybe because these ignore -Os hints? For this
reason I've prioritized the string.h's symbols unless LFS_NO_STRINGH is
defined:
code stack
before: 33338 2640
static-inline: 33422 (+0.3%) 2648 (+0.3%)
builtins: 33402 (+0.2%) 2640 (+0.0%)
after: 33342 (+0.0%) 2640 (+0.0%)
Comparing the LFS_NO_STRINGH and LFS_NO_INTRINSICS builds, just for
curiosity:
code stack
default: 33342 2640
no-string.h: 33486 (+0.4%) 2640 (+0.0%)
no-intrinsics: 33514 (+0.5%) 2616 (-0.9%)
no-both: 33722 (+1.1%) 2624 (-0.6%)
The extra 4 bytes introduced seem to come from the added
lfs_gdelta_xor -> lfs_memxor indirection, not really sure why, maybe
compiler/instruction alignment noise?
Why not provide __builtin_* variants for all string.h symbols? To be
honest, because we really don't care about the performance of strlen/
strcpy/strspn in littlefs. And in environments where string.h is not
available it's likely __builtin_str* won't be as well.
Note the test/bench frameworks should stick with the stdlib symbols. By
default C code should assume these are always available, and this makes
it slightly more reliable to test with -DLFS_NO_STRINGH or
-DLFS_NO_INTRINSICS.
lfsr_attr_t attrs[a*d*b]; => lfsr_attr_t attrs[a];
lfs_size_t attr_count; lfs_size_t attr_count;
lfs_size_t attr_scratch; lfsr_data_t datas[d];
lfs_size_t data_count;
uint8_t buf[b];
lfs_size_t buf_size;
This mostly reverts the allocator scaffolding needed for the attr-list
arenas (LFS_ALIGNOF, etc). This is the main draw of this change, as it
would be nice to avoid a low-level arena implementation headaches unless
they prove to be worthwhile. Which they haven't really so far...
Unfortunately this comes with another code cost, I think due to the
number of counters needed to keep track of separate attr/data/buf
allocations. At least stack showed a slight improvement:
code stack
before: 33844 2824
after: 33872 (+0.1%) 2816 (-0.3%)
The idea is for cases where we need to incrementally allocate attrs +
context, to allocate from both sides of a statically allocated attr
array. This keeps all of the attr-list state in one place, simplifying
state allocation:
.---+---+---+---.
| attr |
+---+---+---+---+
| attr ----------.
+---+---+---+---+ |
| attr --------. |
+---+---+---+---+ | |
| | | | |
| v | | |
| | | |
| ^ | | |
| | | | |
+---+---+---+---+ | |
| data | <-' |
+---+---+---+---+ |
| encoded bptr | <---'
'---+---+---+---'
This is especially useful for the non-terminating tail-recursive
lfsr_btree_commit_, which needs to pass this state through a function
call.
Unfortunately, to make this work we needed to implement more-or-less a
full arena allocator, complete with annoying alignment handling. alignof
isn't even available in C99, so we needed a few more intrinsics:
- LFS_ALIGNOF(t) - Alignment of type t
- LFS_ALIGNEDSIZEOF(t) - Necessary size to force alignment for t
- LFS_MIN(a, b) - Compile-time min
- LFS_MAX(a, b) - Compile-time max
Technically only LFS_ALIGNOF was required, but the others are nice to
have. LFS_MIN/LFS_MAX is also useful anywhere you need to calculate
complicated compile-time sizes.
At least in C11 we get alignof, so we won't need compiler extensions/
hacks for this in the future...
---
Unfortunately this ended up a net-negative. Pushing up the code/stack
cost to near pre-cat levels:
code stack
before cat: 33856 2824
before scratch: 33812 (-0.1%) 2800 (-0.8%)
after: 33844 (-0.0%) 2824 (+0.0%)
I think the two main culprits are 1. the extra logic needed to calculate
alignment, and 2. wasted stack due to aligning scratch space up to the
nearest lfsr_attr_t.
We've been linking in this now-unused CRC table when we don't
need to be:
code stack
before: 33976 2824
after: 33856 (-0.4%) 2824 (+0.0%)
The only catch was it's use in lfs_emubd to provide optional checksums
when debugging. But the actual checksum doesn't matter, so this can be
migrated to crc32c.
We're using parity a lot more than popc now (actually, now that we don't
use CTZ skip-lists, do we use popc at all?), so it makes sense to the
compiler's __builtin_parity intrinsic when possible.
On some processors parity can be much cheaper than popc. Notably, the
8080 family just includes a parity flag in the set of carry flags that
are implicitly updated on most ALU operations. Though I think this
approach didn't scale, you don't really see parity flags on most >8-bit
architectures...
Unfortunately, ARM thumb, our test arch, does not have a popc or parity
instruction. I guess because thanks to implicit shifts in most
instructions, the tree-reduction solution is surprisingly cheap:
ea80 4010 eor.w r0, r0, r0, lsr #16
ea80 2010 eor.w r0, r0, r0, lsr #8
ea80 1010 eor.w r0, r0, r0, lsr #4
ea80 00c0 eor.w r0, r0, r0, lsr #2
ea80 0050 eor.w r0, r0, r0, lsr #1
f000 0001 and.w r0, r0, #1
Both popc and parity benefit from this (GCC 11):
code
__popcountsi2: 40
__paritysi2: 32 (-20.0%)
So, thumb is not an arch where we see much benefit:
code stack
before: 33908 2824
after: 33924 (+0.0%) 2824 (+0.0%)
Not really sure where the +16 bytes come from, we removed several masks,
so I guess it's just bool vs in compiler noise?
Still, this may be useful for other archs with parity instructions/
hardware.
- It didn't save code.
- An inlined buffer is potentially more useful, even if only marginally,
and, uh, unproven yet.
- Requiring lfs_toleb128 in a readonly implementation is a hard ask.
The idea is that we can save on the cost of calling lfs_toleb128
everywhere we commit leb128s, by lazily encoding during progdata.
I original thought this would have too many small problems, but:
1. We can actually implement slice surprisingly easily by just shifting
the internal word 7 bits. This emulates byte-level slicing in the
encoded leb128.
This enables read/cmp, so we can implement all of the lfsr_data_t
functions, though it does make lfs_toleb128 required for a readonly
implementation, which isn't great. Sufficient creativity with ifdefs
likely makes this a non-problem though.
2. There's really very limited use cases for non-leb128 inlined datas.
We can use it to encode the version and compatflags during
lfs_format, but that's about it. And lfs_format is definitely not on
the stack hot-path, so there's no reason to not use on-stack buffers
for these.
The original motivation for this change was noticing a surprising amount
of code savings related to lazy leb128 encoding in another lfsr_data_t
refactor. Unfortunately this savings does not seem reproducible:
code stack
before: 33864 2880
after: 33912 (+0.1%) 2888 (+0.3%)
But that's ok, this is closer to what I expected. The lfs_sizeleb128
call we need to predict the leb128 size is close to the same cost as
calling lfs_toleb128 so the savings isn't really that much.
So unsigned instead of signed. The original intention of using int32_t
was to hint that the sign bit should be reserved, but this may just
confuse if our leb128 encoding has a sign representation, which it does
not.
So:
x = (cond) ? yes : no;
Where there are always parentheses around the condition, even if not
required for disambiguity. Additional parentheses are always allowed,
but the parenthesized condition helps signal that a ternary operator is
coming earlier in the expression.
This style has grown on me as I think it helps code readability. It
reminds me of the required parentheses for if/while statements.
Might as well adopt codebase-wide.
While definitely winning cool points, uncrc32cs have a number of
problems:
1. uncrc32c is relatively unflexible, being limited to only CRC-related
checksums, and probably violating some properties of cryptographic
hashes if possible there.
2. Code savings are minimal, a reversed crc32c implementation is a only
a little less costly than the logic to save aligned CRCs, and since
it's not on the hot-path, the stack cost is ~zero.
3. uncrc32c may come with a high computation cost.
We aren't measuring this, but uncrc32c either operates at the
bit-level, or requires a second set of tables which is unreasonable
for littlefs's use case.
With uncrc32c you need to update the checksum based on every bit in
the extra unaligned data, up to prog_size. With flcksums it's just a
copy of a word, and prog_size has no impact.
So for now dropping uncrc32c, though this can always be reverted in the
future.
Checksumming unaligned data during block compaction is surprisingly
tricky. We don't know if our data will be aligned until after
a potentially unbounded number lookups, we need to write data into our
pcache as we go to avoid unnecessary lookups, but if we end up unaligned
we need to revert our checksum to the checksum of the aligned data.
The way I see it there are 4 options:
1. Calculate the checksum after writing data into the block.
This is the most expensive option, requiring a full second read of
the data to calculate the checksum. It is simple though.
2. Do a pass over the btree to figure out alignment before writing.
This at least only reads metadata twice, so is more efficient than
the 1st option.
3. Keep track of the aligned checksum on each flush, falling back to the
last flushed checksum if we need to correct alignment.
This solution is flexible though requires some extra state to track
multiple checksums.
4. Leverage the math behind CRCs to run the CRC backwards when we
truncate for alignment.
This works, though a bit inefficiently, but is strictly tied to
CRC-related checksums.
By inefficient I mean that we would likely be limited to a bit-level
"uncrc32c". It's possible to create nibble/byte tables for uncrc32c,
but this adds significant code cost for a relatively uncritical
function.
I was hopeful that we could leverage the existing tables in both
functions, but unfortunately it doesn't work out like that. You could
scan the crc32c table to find the constant to reverse, but this
requires ~16*2 or ~256 operations vs "naive" ~8 operations per byte.
This commit implements both 3 and 4, defaulting to 4 unless
LFS_NO_UNCRC32C is defined.
The current lfs_uncrc32c implementation is a simple bit-level
implementation, but does allow for crc32c truncation without any extra
state.
code stack
before: 32044 2880
uncrc32c: 32108 (+0.2%) 2880 (+0.0%)
flcksum: 32132 (+0.3%) 2880 (+0.0%)
First, realized the the LFS_UNREACHABLE logic was flipped after a
confusing test bug (damn double negatives). But also realized LFS_ASSERT
could be tweaked to "call" __builtin_unreachable() on assert failure to
act as a sort of compiler hint.
Turns out this hint saves a little bit of code, note both builds have
LFS_UNREACHABLE fixed:
code stack
without __builtin_unreachable: 28408 1928
with __builtin_unreachable: 28324 (-0.3%) 1920 (+0.0%)
Since __builtin_unreachable is a compiler extension, its usage respects
LFS_NO_INTRINSICS.
The main purpose of this change is to introduce LFSR_DATA_CAT, a
generalized way to concatenated various data references internally.
As a side-effect lfsr_data_t has been completely restructured. Now,
lfsr_data_t can be in one of 4 modes:
If the size field's sign bit=0, the lfsr_data_t points in-device. A new,
count field, determines the encoding:
sign(size)=0, count=0 => inlined:
.---+---+---+---.
| size |
|---+---+---+---|
|c=0| inlined d | note inlined data is just enough to hold
|---+ | one encoded leb128
| ata... |
'---------------'
sign(size)=1, count=1 => direct:
.---+---+---+---. .---+---+---+---.
| size | .>| data... |
|---+---+---+---| | | . |
|c=1| | | . . .
|---+---+---+---| | . . .
| direct ptr -----' . .
'---------------'
sign(size)=1, count>=2 => indirect:
.---+---+---+---. .---+---+---+---. .---+---+---+---.
| size | .>| size | .>| data... |
|---+---+---+---| | |---+---+---+---| | | . |
|c>1| | | |c=1| | | . . .
|---+---+---+---| | |---+---+---+---| | . . .
| indirect ptr ---' | direct ptr -----' . .
'---------------' '---------------' .---+---+---+---.
| size | .>| data... |
|---+---+---+---| | | . |
|c=1| | | . . .
|---+---+---+---| | . . .
| direct ptr -----' . .
'---+---+---+---'
| . |
| . |
. . .
. .
. .
note only one indirect layer is allowed due to no recursion
If the size field's sign bit=1, the lfsr_data_t points on-disk:
sign(size)=0 => on-disk:
.---+---+---+---. .....
| size | ..'' ''..
|---+---+---+---| : : :
| block ------+->| ..:|
|---+---+---+---| | |......( )::::::|
| off -------' |:::' : |
'---------------' :' : :
''.. :.''
'''''
My goal with this commit was to test the new implementation and see how
it would impact code/RAM size before adopting it in the actual file
handling code, and the results are... not great...
code stack
before: 24668 1840
after: 25552 (+3.5%) 1920 (+4.2%)
I think most of the new cost comes from the now correct handling of
read/cmp with concatentated datas, which previously would just assert.
This change gives us LFSR_DATA_CAT, so I will be working with it for
now, but this may be worth looking at again in the future. Maybe the
correct handling of read/cmp should just be reverted to an assert...
The idea here: Instead of having unique functionality for each
individual btree operation (push/set/pop/split), we treat btrees sort of
like rbyds, with a single commit entry point that operates on attr-lists.
This adds code cost, due to needing to parse the attr-list for properties
that can affect inlined btrees (tag changes mostly), but, in theory, comes
with some advantages:
1. A single btree commit entry point with all of the inlined/uninlining
logic should offer better chances for code deduplication, vs
spreading this logic out in each btree operation.
2. Higher-levels should know what the current weight of the branch is,
so we may be able to avoid the implicit math needed to calculate
deltas.
3. Higher-levels have more knowledge about the state of the btree in
general, so there may be other shortcuts. The mtree, for example,
only operates on weight=1 entries, which greatly simplifies a lot of
the related math.
Note that btrees still have strict limits in what's possible in an
attr-list. Btree operations can't cross leaf-rbyd boundaries for
example.
---
A notable omission in this change is the loss of reinlining btrees.
This wase dropped for a couple reasons. It may be worth adding back at a
later time, maybe after we actually have files implemented, but for now
does not seem worth it:
1. Reinlining adds code cost. Reinlining is more complex than you might
expect because we only reinline on compaction. And because we compact
before playing out our attr-list, we need to know if a commit makes
the btree inlinable before committing to the btree.
This is still doable with our attr-lists. We already derive the
change in tags, since we need this to know when to uninline. But it
adds a kind of complex bailing out of btree commits.
2. The benefits of reinlining may not be that great. In most systems, a
tree that is uninlined once is likely to be uninlined again. It's
only if there is a bigger state change in a system that it makes
sense to reinline.
Though, to be fair, waiting for compaction to reinline handled this
quite well. Only reinlining when all erased storage is used up...
3. Thanks to our roots did entry, our mtree can never reinline.
It would be nice to change this, but this would require explicit
handling in lfsr_mdir_commit. Future work?
4. Files are another can of worms, with more complex interactions with
inlinability thanks to (at least on paper right now) always having
inlined data even when uninlined.
If reinlining is valuable for files this can change during that work.
5. Even if files never support reinlinability, truncating files (via
either lfsr_file_truncate or LFSR_O_TRUNC) should give the file a
blank slate, effectively reinlining the file in that case.
---
The current implementation also changes the attr-list to be mutable so
we can adjust attr-list based on the current btree node. This is a
temporary hack! We should add the appropriate functionality to our rbyd
utilities to revert this eventually.
Composable parsing functions always feel a bit weird to me in C. I don't
know if this is because of something C lacks, such as multiple return
values, or if composable parsers are just inherently awkward to describe
in procedural languages because of the different levels of state.
But I think the API here is pretty ok. The main idea is that data
parsers can be added as functions in the lfsr_data_* namespace that take
lfsr_data_t as a mutable reference, updating the lfsr_data_t's internal
state as data is parsed.
In practice you only need a couple of primitives, bytes, le32s, leb128s,
that touch the internals of lfsr_data_t, and the other parsers can be
built using these.
This leverages the pointer-like abstraction of lfsr_data_t, and avoids
needing to keep track of offsets. And thanks to lfsr_data_t being
relatively cheap to make copies, this API is relatively flexible.
Some other tweaks:
- Signed leb128 overflow detection is moved up into lfs_fromleb128.
littlefs now assumes _all_ leb128s are 31-bits, which is useful for
leveraging the sign bit internally.
This also fixes the an issue in overflow detection in lfs_fromleb128
which wouldn't catch overflows in the last byte of a >32-bit leb128.
- Most lfsr_data_t functions now take a pointer. This offered a small
bit of code savings and feels more natural in C. Though most functions
that accept lfsr_data_t still take a copy. Most of these functions
would need to make a copy anyways now that the parsers are consuming,
and these copies avoid concerns about shared state.
At 3-words, lfsr_data_t is right at that boundary of questionable
reasonableness for copying, but copying is a very useful feature of
this struct.
This ends up with some decent code/stack savings:
code stack
before: 22118 2048
after: 21722 (-1.8%) 1992 (-2.7%)
The idea of this is:
1. Aside from the encoded size, our lfsr_data_t has space for 2 integers.
2. Our mdir addresses are exactly 2 leb128s.
3. We already need to be able to inject 1 leb128 for did entries.
So if we can cram our 2 leb128s inline into the lfsr_data_t, we should
be able to avoid the indirection, wasted space in lfsr_data_t, and
duplicate encoding costs for the mdir addresses.
Conveniently for us, there are exactly 2 unused bits in various fields,
thanks to our common 31-bit limits.
It's a bit awkward since we must assume our buffer pointer uses all
32-bits, but here are the current encodings:
00 = in-device buffer 10 = on-disk data
no leb128s no leb128s
.----+----+----+----. .----+----+----+----.
|0| size | |1| size |
|----+----+----+----| |----+----+----+----|
|0000000000000000000| |0| offset |
|----+----+----+----| |----+----+----+----|
| buffer | | block |
'----+----+----+----' '----+----+----+----'
01 = in-device buffer 11 = 2 leb128s
1 leb128
.----+----+----+----. .----+----+----+----.
|0| size | |1| size |
|----+----+----+----| |----+----+----+----|
|1| leb128 | |1| leb128 |
|----+----+----+----| |----+----+----+----|
| buffer | | leb128 |
'----+----+----+----' '----+----+----+----'
This encoding also presents a relatively nice code-path, since we can
treat the 2 leb128 case as an on-disk data reference with no size.
Unfortunately the initial measurements look, uh, really bad:
code stack
before: 22194 2048
after: 22426 (+1.0%) 2088 (+2.0%)
This needs more investigation, but from what I can tell so far the RAM
cost comes from the leb128 encoding buffer moving into the "hot path",
aka the deepest call stack in littlefs, which involves lfsr_data_read
as a part of mtree traversal as a part of block allocation.
I have no idea about the code cost though...
This turned out to be tricky.
At littlefs's core, we have the lfsr_rbyd_t struct. It is really
important this is as small as possible since littlefs creates many rbyd
copies in order to track state of metadata on disk.
Wrapping rbyd, we have the lfsr_btree_t struct, which can alternatively
contain a single inlined entry, accomplished by overlapping the width
field in both cases. And the lfsr_mdir_t struct, which tracks any redundant
blocks, and would be nice if the blocks lined up as neighbors so all blocks
involved in the mdir could be passed around as an array. Both of these
wrappers attempt to overlap fields of the lfsr_rbyd_t struct, which presents
a bit of a problem.
The solution here is to put the rbyd block field at the beginning of the
lfsr_rbyd_t struct, and use exactly 32-bits of padding in lfsr_btree_t
to overlap the width field even though it is not at the beginning of the
struct. To avoid inflating the lfsr_btree_t size, we sneak the inlined
size and tag into the overlapping padding. This will need special
handling if the size of these fields change, but saves a decent amount
of RAM:
lfsr_rbyd_t lfsr_btree_t lfsr_mdir_t
8b 8b 8b 8b
.----+----+----+----.
| mid.bid | mid.rid |
|----+----+----+----|
8b 8b 8b 8b 8b 8b 8b 8b | blocks |
.----+----+----+----. .----+----+----+----. | |
| block |..| tag |size|padd|.>| |
|----+----+----+----| |----+----+----+----| |----+----+----+----|
| weight |.>| weight | | weight |
|----+----+----+----| |----+----+----+----| |----+----+----+----|
| trunk | | inlined data | | trunk |
|----+----+----+----| | | | |----+----+----+----|
| off | | v | | off |
|----+----+----+----| | | |----+----+----+----|
| crc | | | | crc |
'----+----+----+----' '----+----+----+----' '----+----+----+----'
Also tried to reduce the amount of mdir usage in lfsr_mdir_commit by
better using only the arrays of relevant mdir blocks, to limited success.
The idea here is to combine the current mtree size with the theoretical
upper bound on the number of directories in a single mdir, assuming our
block size, to give us a heuristic for did truncation that does not
require any extra state.
- Each directory needs 1 name tag, 1 did tag, and 1 dstart
- Each tag needs ~2 alts with our current compaction strategy
- Each tag/alt encodes to a minimum of 4 bytes
- We can also assume ~1/2 block utilization due to our split threshold
This gives us ~3*3*4*2 or ~72 bytes per directory at minimum, or
rounding down, ~block_size/32 directories per mdir.
This is a nice number because for common NOR flash geometry,
4096/32 = 128, so a filesystem with a single mdir encodes dids in a
single byte.
The biggest benefit though is being able to drop the mlimit state from
the lfs_t struct.
---
Unfortunately, this change revealed several bugs.
It turns out __builtin_clz in GCC is undefined at 0, which caused our
lfs_nlog2 function to return incorrect values at 1. This was causing
our dids to all collide when the mtree was inlined, which was resolved
by the linear scanning that resolves dids, but was severely limiting
what exactly our tests covered.
Now that this is fixed (with a simple if statement in lfs_nlog2,
lfs_nlog2 now always has defined behavior, even at 0), several bugs
needed fixing:
- We update the rid based on attrs in lfsr_mdir_commit before updating
the mdir. If we have multiple attrs this causes the assert on
rid-in-bounds to trigger incorrectly. Just removed that assert for now.
- We needed to adjust second grms if they are affected by the fixing
of the first grm.
- Directory position updates are incorrectly updated if an unrelated
weight change occurs before an opened directory, but is not a part of
that opened directory.
This is NOT fixed yet, the current implementation is just broken
enough that I've just ripped it out for now (it was causing the
read_with_rms test to fail because pos backed up into the "."/".."
entries).
This needs some thinking to fix.
Because of that last, unfixed bug, tests are not all passing at the
moment. To pass testing -DSEEK=0 is needed to disable the failing tests.
This is an absurd optimization that stems from the observation that the
branch encoding for the inner-rbyds in a B-tree is enough information to
jump directly to the trunk of the rbyd without needing an lfsr_rbyd_fetch.
This results in a pretty ridiculous performance jump from O(m log_m(n/m))
to O(log(m) log_m(n/m)).
If the complexity analysis isn't impressive enough, look at some rough
benchmarking of read operations for 4KiB-block, 1K-entry B-trees:
12KiB ^ :: :. :: .: .: :. : .: :. : : .. : : . : .: : : :
| .:: .::.::.:: ::.::::::::::::.::::::::.::::::::::::.
| : :::':: ::'::'::':: :' :':: :'::::::::': ::::::': :
before | ::: ::' :' :' :: :' '' ' ' '' : : : '' ' ' '
| ::: ''
|:
0B :'------------------------------------------------------>
.17KiB ^ ............:::::::::::::::::::::::::::::
| . .....:::::''''''''' ' ' '
| .::::::::::::
after | :':''
|.::
.:'
0B :------------------------------------------------------->
0 1K
In order for this to work, the branch encoding did need to be tweaked
slightly. Before it stored block+off, now it stores block+trunk where
"trunk" is the offset of the entry point into the rbyd tree. Both off
and trunk are enough info to know when to stop fetching, if necessary,
but trunk allows lookups to jump directly into the branches rbyd tree
without a fetch.
With the change to trunk, lfsr_rbyd_fetch has also be extended to allow
fetching of any internal trunks, not just the last trunk in the commit.
This is very useful for dbgrbyd.py, but doesn't currently have a use in
littlefs itself. But it's at least valuable to have the feature available
in case it does become useful.
Note that two cases still requires the slower O(m log_m(n/m)) lookup
with lfsr_rbyd_fetch:
1. Name lookups, since we currently use a linear-search O(m) to find names.
2. Validating B-tree rbyd's, which requires a linear fetch O(m) to
validate the checksums. We will need to do this at least once
after mount.
It's also worth mentioning this will likely have a large impact on B-tree
traversal speed. Which is huge as I am expecting B-tree traversal to be
the main bottleneck once garbage-collection (or its replacement) is
involved.
This fixed two notable bugs:
1. Using "altle 0xfff0" to terminate unreachable rbyd trunks threw off
id calculations in lfsr_rbyd_fetch searches. We derive the tag's
id+weight from the lower bound calculated as the sum of all "altle"s
and an always-followed "altle 0xfff0" throws this off.
We _could_ derive the tag's id+weight from the upper bound, inverting
this relationship, but decided to revert back to using "altgt 0" to
terminate unreachable rbyd trunks.
Using the lower bound is more intuitive, and "altgt 0" has the
benifit of supporting variable-length tags if we ever need to adopt
those.
To avoid the previous issues around 0-tag holes (which was the original
motivation for altle 0xfff0), 0-tags are now automatically adjusted
in lfsr_rbyd_lookup, and avoided in lfsr_rbyd_append.
But note! if any implemention tries to look up 0-tags, this will
eventually break! See previous commits for more info.
2. Unfortunately, we can't combine branch updates and weight updates in
lfsr_btree_commit in the general case.
If our btree contains bname tags, the weight is attached to the
bname tag, separately from the branch tag.
Branch updates in lfsr_btree_commit need two separate attrs for the
weight and branch struct for this reason, which is unfortunate.
The amount of extra conditions to make bname+branch pairs work makes
me want to redesign the inner-nodes of the btrees, but I can't think
of a better way to approach the problem.
This does not work as is due to ambiguity with grows and insertions.
Before, these were disambiguated by seperate grow and attr tags. You
effectively grew the neighboring id before claiming its weight
as yours. But now that the attr itself creates the grow/insertion,
it's ambiguous which one is intended.
The main motivation for this was issues fitting a good tag encoding into
14-bits. The extra 2-bits (though really only 1 bit was needed) from
making this not a leb encoding opens up the space from 3 suptypes to
15 suptypes, which is nothing to shake a stick at.
The main downsides:
1. We can't rely on leb encoding for effectively-infinite extensions.
2. We can't shorten small tags (crcs, grows, shrinks) to one byte.
For 1., extending the leb encoding beyond 14-bits is already
unpalatable, because it would increase RAM costs in the tag
encoder/decoder,` which must assume a worst-case tag size, and would likely
add storage cost to every alt pointer, more on this in the next section.
The current encoding is quite generous, so I think it is unlikely we
will exceed the 16-bit encoding space. But even if we do, it's possible
to use a spare bit for an "extended" set of tags in the future.
As for 2., the lack of compression is a downside, but I've realized the
only tags that really matter storage-wise are the alt pointers. In any
rbyds there will be roughly O(m log m) alt pointers, but at most O(m) of
any other tags. What this means is that the encoding of any other tag is
in the noise of the encoding of our alt pointers.
Our alt pointers are already pretty densely packed. But because the
sparse key part of alt-pointers are stored as-is, the worst-case
encoding of in-tree tags likely ends up as the encoding of our
alt-pointers. So going up to 3-byte tags adds a surprisingly large
storage cost.
As a minor plus, le16s should be slightly cheaper to encode/decode. It
should also be slightly easier to debug tags on-disk.
tag encoding:
TTTTtttt ttttTTTv
^--------^--^^- 4+3-bit suptype
'---|- 8-bit subtype
'- valid bit
iiii iiiiiii iiiiiii iiiiiii iiiiiii
^- m-bit id/weight
llll lllllll lllllll lllllll lllllll
^- m-bit length/jump
Also renamed the "mk" tags, since they no longer have special behavior
outside of providing names for entries:
- LFSR_TAG_MK => LFSR_TAG_NAME
- LFSR_TAG_MKBRANCH => LFSR_TAG_BNAME
- LFSR_TAG_MKREG => LFSR_TAG_REG
- LFSR_TAG_MKDIR => LFSR_TAG_DIR
This really just required care around calculating the expected B-tree id
and rbyd id (which are different!).
B-tree append, aka B-tree push with id=weight, is actually the outlier.
We need a B-tree id that can identify the rbyd we're appending to, but
this id itself doesn't exist in the tree yet, which can be a bit tricky.
Christopher Haster