When updating any opened mdirs to keep things in sync, we need to know
what rid the mdir is targeting in order to know which on-disk mdir it
should follow in the case of splits. Making this rid an actual member of
the mdir struct simplifies things.
This adds some RAM cost, though the plan is to merge the mid/rid into a
single integer, which requires this change and should actually save RAM
in the long run.
code stack
before: 22342
after: 22204 (-0.6%) 2144 (+1.1%)
It turned out our dir-read-idempotent test never created non-dstart
neighbors. This was a bit of a problem since we relied on dstart entries
to know when our dir read terminates. If we seek to an invalid position
(in theory undefined behavior, but easily possible with concurrent
modifications to the directory), we can end up reading an unrealted,
non-dstart entry, and incorrectly reporting that entry as in our current
dir.
This fix reintroduces the did into the lfsr_dir_t struct and uses the
did to determine end-of-dir. This adds some RAM cost, but is more
resilient to any seeks that overshoot the end of the directory.
Using did is also a stronger guarantee we will never accidentally report
unrelated entries as a part of the current directory.
This is entirely a pragmatic change, lfsr_mdir_commit already does
several hairy things with grm tags, decoding, fixing, reencoding, etc,
so it makes sense to move all the encoding logic into lfsr_mdir_commit.
This leads to a couple optimizations:
- We don't need to decode the grm to apply any last minute fixes.
- By allowing the grm arugment to be mutated (they are just sitting on
the stack anyways, we need a copy in case we back out of change due to
error), we can apply and save any grm fixes in the grm argument
itself.
This means we only need to fix the grm at most once, after any mtree
modifications.
Which in turn saves some code and stack cost:
code stack
before: 22930 2392
after: 22706 (-1.0%) 2344 (-2.0%)
Unfortunately the previous attempt to fix the dir seek system didn't
really work. Using a packed mid/rid integer for the offset is tempting,
but since mid/rid can change with any metadata id change in the
filesystem, dir tell offsets would become invalidated if you modified
files in unrelated directories, which isn't great and likely to catch
users by surprise.
This solution builds on the previous dir offset design, which tracks the
dstart-relative position independently from the current mid/rid in our
directory. To update this correctly when there are unrelated changes to
the filesystem, we need to know if metadata id changes are in the range
between our directories dstart and current mid/rid. This in turn means
we need to track our dstart. So our opened directories need three
separate pointers we need to update on every mdir commit:
dir->pos
|
.-------+-------.
a b c d e f g h i j k l m n o p
^ ^
| |
dir->dstart dir->mdir
This has quite a few moving parts, which I was hoping to avoid.
Fortunately we don't need a second mdir, so the RAM cost is pretty
small.
We can also drop dir->did, since the dstart mid/rid render it redundant,
which is interesting.
This is an attempt to fix issues with dir seeking in a filesystem
undergoing changes. The problem with the previous dstart-relative
position encoding is that if we deleted/created new entries outside of
our current directory, we didn't if they were inside or outside of the
current directory, so we couldn't always update our position correctly.
Instead of using a dstart-relative position, this solution crams both
the mid and rid into a single 31-bit integer. Things get a bit tight
here, so we use the current block_size as a heuristic for how many
possible rids we can ever have in a single mdir. The idea is the larger
the rid encoding needs to be, the smaller the mid encoding needs to be,
and we should, _roughly_, approach the same encoding limitation we would
have to dstart-relative position anyways.
Making some assumptions about the maximum possible number of rids in a
block gives us at most ~block_size/8 rids per mdir.
So for 4096 byte blocks (note the exact encoding is dynamic):
sbbbbbbb bbbbbbbb bbbbbbbr rrrrrrrr
^'-----------+----------''----+---'
'------------|----------------|----- sign bit (used for errors)
'----------------|----- 22-bit metadata bid
'----- 9-bit metadata rid
Note this introduced as new, significant limitation on the number of
total mdirs in the system. Normally I would be against this solution for
that reason, however if we adopt this encoding elsewhere in the system it
may improve some RAM cost and in general simplify things by being able to
store any mid in a single integer. More work needs to be done here...
This approach needs some fleshing out and has its own issues (the
offset returned by tell quickly becomes out of date if the filesystem
is modified, but is that really a problem?), but it improves over the
previous implementation by making tell always correct at that moment.
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.
Only simple tests right now, but the theory is sound.
This mainly required the addition of the fancy in-device move attribute,
which copies all tags associated with an rid from one rbyd to another in
a single transaction.
This is a carryover from the previous littlefs implementation, though it
is easier to implement here since it is effectively a range query on the
rbyd tree, which trees are really good at. This was intentional.
Oh and I suppose this also required implementing lfsr_rename, which has
a few corner cases to watch out for.
It is nice that both lfsr_remove and lfsr_rename can rely on
lfsr_fs_fixgrm to finish all of the removes, which wasn't previously
reasonable due to the overhead of deorphaning.
Ugh. I overlooked a weird corner case in rename's behavior that requires
changes to the grm to support.
POSIX's rename, which lfsr_rename is trying to match, supports renaming
files over existing files, effectively removing the previous file during
the rename.
This is supported, even if the files are directories, but with the
additional requirement that the previous directory is empty (matching
the behavior of lfsr_remove).
This creates a weird situation for littlefs. In order to remove
directories in littlefs, we need to atomically remove both the dstart
entry that reserves the directory's did and the directories entry in its
parent. This is made possible by using the grm to mark one entry as
pending removed while removing the other.
But in order to rename atomically, we need to use the grm to mark the
source of the rename as removed while creating/replacing the destination
of the rename.
So we end up needing two grms simultaneously.
This is extra annoying because the niche case of renaming a directory
over another empty directory is the only case where we need two grms,
but this requirement almost doubles the grm size both in-ram and
reserved in every mdir, from 11 bytes to 21 bytes, and increases the
lfs_t size by 28 bytes.
---
Anyways, this commit extends the grm to support up to two pending removes.
Fortunately the implementation was simple since we already have a type
field that can be extended, and grm operations just needed to be
changed from if statements to for loops.
Recursive removes is proving more challenging to implement than I
expected. The problem with the previous approach is that it moved the
mid into a potentially non-sensical position with the expectation it
would be updated in lfsr_dir_read because the rid overflows the current
weight (since dropping mdirs always set the weight to zero).
But if an unrelated mdir commit followed that happened to touch that
nonsense mid, the mdir would incorrectly be updated to the previous
block, causing problems for the dir's read state.
---
The solution here is to toss all of that out and rely solely on directory
position updates, which are a bit simpler.
So in lfsr_dir_read, if our mid/rid is deleted, we perform a full
rewind+seek to the new position. This can be more costly, but since the
most common case, recursive removal, leaves us with all mid/rids < pos
deleted, it should only add a single mtree lookup per lfsr_dir_read.
Also added prototypes for dir seek/tell/rewind, since we're using
they're logic for this. Though these aren't yet tested. These are built
on the new function lfsr_mtree_seek which captures the common logic of
seek over multiple mdirs in the mtree efficiently, and skips unnecessary
rid lookups where possible.
"Recursion" here just refers to the ability to remove entries in a
directory while iterating over it. This is very useful when you just
want a directory gone, and can be extended to a "true" recursive remove
straightforwardly. This mainly tests that mid/rid updates in opened
mdirs are correct.
To make this work, we need to update opened dirs differently than files,
since opened dirs do not get marked as removed when its rid is removed
and contain an additional position in the dir that needs to be updated.
To keep track of the different types, littlefs now contains 2
linked-lists for opened mdirs. Maybe these should be correctly typed,
but by hiding the specific types behind an array of mdir linked-lists,
we can more efficiently iterate over both lists when necessary.
We should probably compare this approach to the type-tagged approach in
the previous littlefs implementation, but I think the idea of an array
of type-hidden linked-lists just didn't come to me then. There was also
a bit more room in the mdir structs to hide a 1-bit type field. The mdir
structs here are getting pretty squeezed since they are used everywhere.
In theory this is pretty much the same as lfsr_mkdir, but backwards.
The main work was making the interactions between removing mids/rids and
the grm correct. This ends up meaning we just need to update the grm on
any mid/rid update the same way we update the list of opened mdirs.
On the plus side, it turned out to be possible to deduplicate the mdir
uninlining route a bit, by adding range argument to lfsr_mdir_commit_
and changing the write of the newly uninlined mtree/mdir to marking
mtree as dirty and then joining the common path.
This lets us move the pre-commit round of grm updates into a single
location in lfsr_mdir_commit, removing and extra function definition and
the related state marshalling while also simplifying the control-flow.
This also raises the question, can more lfsr_mdir_commit be deduplicated
more? Uninlining is a infrequent operation we don't really need to
optimize for.
---
Testing lfsr_remove also found a bug related to incorrect propagation of
when the mroot becomes "unerased" (when rbyd overflows). This raises the
concern that we're not propagating unerased-states very rigorously, and
unexpected errors may not allow the filesystem to resume.
This has never been in a very good place for littlefs, but would be
worth improving in the future.
Instead of truncating to exactly 28-bits for nice leb128 alignment, we
now truncate to ~the number of metadata entries, which must be >= ~2x
the number dids since each did needs a dir entry and dstart entry.
This has the downside of needing to actually keep track of an estimate
of the number of metadata entries, which is made a bit difficult due to
integer overflow issues (we can have more than 2^32 metadata entries),
but has the upside of allowing a full 2^32 number of dids worst case.
This is really unlikely, but it's nice to not need another configuration
option to control the did limit.
Another option would be to scale the hashes based on the number dids,
which would be a more direct solution. Unfortunately determining the
number of dids during mount requires a O(m*log(m)) scan of each rbyd
to find either dir entries or dstart entries. This solution can easily
end up with an overestimate, but only needs to weight of each rbyd which
can be (and already is) found in O(m).
This implementation is in theory correct, but of course, being untested,
who knows?
Though this does come with remounting added to all of the directory
tests. This effectively tests that all of the directory creation tests
we have so far maintain grm=0 after each unmount-mount cycle. Which is
valuable.
This has, in theory, global-removes (grm) being written out as a part of
of directory creation, but they aren't used in any form and so may not
be being written correctly.
But it did require quite a bit of problem solving to get to this point
(the interactions between mtree splitsand grms is really annoying), so
it's worth a commit.
lfsr_stat is really a directory operation underneath, so it's good to
add to our testing while we are building up the dir tests.
It's interesting to note lfsr_stat and lfsr_dir_read are less
deduplicatable than their previous versions, since lfsr_stat can get
most of it's info from lfsr_mtree_pathlookup. Though there will probably
need to be some code sharing when we get to files with sizes.
This makes it now possible to create directories in the new system.
The new system now uses a single global "mtree" to store all metadata
entries in the filesystem. In this system, a directory is simply a range
of metadata entries. This has a number of benefits, but does come with
its own problems:
1. We need to indicate which directory each file belongs to. To do this
the file's name entry has been changed to a tuple of leb128-encoded
directory-id + actual file name:
01 66 69 6c 65 2e 74 78 74 .file.txt
^ '----------+----------'
'------------|------------ leb128 directory-id
'------------ ascii/utf8 name
If we include the directory-id as part of filename comparison, files
should naturally be next to other files in the same directory.
2. We need a way allocate directory-ids for new directories. This turns
out to be a bit more tricky than I expected.
We can't use any mid/bid/rid inherent to the mtree, because these
change on any file creation/deletion. And since we commit the did
into the tree, that's not acceptable.
Initially I though you could just find the largest did and increment,
but this gives you no way to reclaim deleted dids. And sure, deleted
dids have no storage consumption, but eventually you will overflow
the did integer. Since this can suddenly happen in a filesystem
that's been in a steady-state for years, that's pretty unnacceptable.
One solution is to do a simple linear search over the mtree for an
unused did. But with a runtime of O(n^2 log(n)), this raises
performance concerns.
Sidenote: It's interesting to note that the Linux kernel's allocation
of process-ids, a very similar problem, is surprisingly complex and
relies on a radix-tree of bitmaps (struct idr). This suggests I'm not
missing an obvious solution somewhere.
The solution I settled on here is to instead treat the set of dids as
a sort of hash table:
1. Hash the full directory path into a did.
2. Perform a linear search until we have no collision.
leb128(truncate28(crc32c("dir")))
.--------'
v
9e cd c8 30 66 69 6c 65 2e 74 78 74 ...0file.txt
'----+----' '----------+----------'
'-----------------|------------ leb128 directory-id
'------------ ascii/utf8 name
Worst case, this can still exhibit the worst case O(n^2 log(n))
performance when we are close to full dids. However that seems
unlikely to happen in practice, since we don't truncate our hashes,
unlike normal hash tables. An additional 32-bit word for each file
is a small price to pay for a low-chance of collisions.
In the current implementation, I do truncate the hash to 28-bits.
Since we encode the hash with leb128, and hashes are statistically
random, this gives us better usage of the leb128 encoding. However
it does limit a 32-bit littlefs to 256 Mi directories.
Maybe this should be a configurable limit in the future.
But that highlights another benefit of this scheme. It's easy to
change in the future without disk changes.
3. We need a way to know if a directory-id is allocated, even if the
directory is empty.
For this we just introduce a new tag: LFSR_TAG_DSTART, which
is an empty file entry that indicates the directory at the given did
in the mtree is allocated.
To create/delete these atomically with the reference in our parent
directory, we can use the GRM system for atomic renames.
Note this isn't implemented yet.
This is also the first time we finally get around to testing all of the
dname lookup functions, so this did find a few bugs, mostly around
reporting the root correctly.
Took the opportunity to make some allocator tweaks:
- Renamed lfs.free -> lfs.lookahead, it's previous name did cause some
confusion.
- Renamed lfs.free.off -> lfs.lookahead.start
- Renamed lfs.free.i -> lfs.lookahead.next
- Renamed lfs.free.ack -> lfs.lookahead.acked
- Changed bitmap from using 32-bit words to using 8-bit bytes, dropping
the alignment requirement. One of the reasons for 32-bit alignment was
an attempt at future proofing for some sort of free-list.
This never landed, and if it did, it could have been provided without
breaking backwards compatiblity via an additional config option, at a
minor RAM cost.
We never used ffs/clz instructions for this bitmap, so I don't think
using 32-bit words offers much advantage. It just creates another
potential issue for users if their lookahead buffer is unaligned.
These changes should probably also be upstreamed to the current version.
They don't depend on anything rbyd specific.
Note, at some point lfs_alloc will need to be extended to mark block tags,
etc, as in-use during traversal.
- Renamed mpair -> mptr, may have >2 blocks in the future.
- Renamed branch -> bptr for consistency.
- Renamed other_block -> redund_rbyd.
- Changed comparison functions to use -1, 0, +1, even for unordered
types.
- Added lfs_cmp function for unioning comparisons with signed errors.
Now we read the revision count on-demand, trading off some extra reads
for a smaller lfsr_rbyd_t struct.
I believe this is worth it because:
1. We're created a lot of lfsr_rbyd_t structs as a part of the relatively
complicated mdir/btree commit logic in order to safely fallback on errors.
2. We don't really need the revision count for our Cow btrees, so we
only need to read the revision count on mdir fetch (which we were
already reading too many times), on mdir compact, and on rbyd fetch
as a part of checksum calculation.
This really only adds a O(1) cost when we are compacting, which is rather
small.
Current measurements:
code: 8980 -> 9036 (+0.6%)
stack: 1024 -> 1000 (-2.3%)
Though note this is currently without any mdir/btree commit code being
dragged in.
littlefs uses an invasive linked-list in open mdirs to keep any open
files/dirs (and some special mdirs) in sync during filesystem
operations. The main benefit of this is that the filesystem doesn't need
to know the number of open files at compile time.
The implementation here introduces a new type, lfsr_openedmdir_t, for
mdirs that want to participate in the opened-mdir linked-list. This
saves a couple words of memory in the cases where the mdir does not need
to participate in the opend-mdir linked-list.
Since we are creating quite a few more mdir structs in lfsr_mdir_commit now,
the size of this struct is valuable.
The implementation of lfsr_mdir_commit knew this was coming, so aside
from the new type, adding this feature was straightforward:
1. Update opened-mdirs based on in-flight attrs.
2. Update opened-mdirs rbyd state.
3. Mark any deleted opened-mdirs with the reserved mid -2.
4. Test.
mdirs behave a bit differently than btree nodes here. When an mdir's
weight drops to zero, we eagerly drop the mdir. Unfortunately this
introduce a large number of conditions into lfsr_mdir_commit. Maybe
there's some different way to structure to code to avoid this...
Also expanded mtree tests to cover more corner cases, these are
desperately for any confidence that mdir drops work.
This became surprisingly tricky.
The main issue is knowing when to split mdirs, and how to determine
this without wasting erase cycles.
Unlike splitting btree nodes, we can't salvage failed compacts here. As
soon as the salvage commit is written to disk, the commit becomes immediately
visibile to the filesystem because it still exists in the mtree. This is
a problem if we lose power.
We're likely going to need to implement rbyd estimates. This is
something I hoped to avoid because it brings in quite a bit of
complexity and might lead to an annoying amount of storage waste since
our estimates will need to be conservative to avoid unrecoverable
situations.
---
Also changed the on-disk btree/branch struct to store a copy of the weight.
This was already required for the root of the btree, requiring the
weight to be stored in every btree pointer allows better code
deduplication at the cost of some redundancy on btree branches, where
the weight is already implied by the rbyd structure.
This weight is usually a single byte for most branches anyways.
This may be worth revisiting at some point to see if there's any other
unexpected tradeoffs.
This work already indicates we need more data-related helper
functions. We shouldn't need this many function calls to do "simple"
operations such as fetch the superconfig if it exists.
This finally provides a solution for deferred B-tree inlining without
needing to evaluate attrs.
Deferred inlining is the idea that instead of inlining B-trees as soon
as the number of entries drops to either 1 or 0, we wait until a
compaction occurs to inline a B-tree. This accomplishes a few things:
1. Limits any extra reads for conditions to compaction time.
2. Avoids wasting erased bytes if we drop to 1 or 0 entries only
temporarily.
3. Avoids excessive erase costs if we oscillate between ~1 and ~2
entries.
Unfortunately after moving away from evaluating attrs, deferred inlining
became deceptively tricky.
In the current, non-evaluating-attr implementation, our btree commits
always lag one commit behind. When we compact, we first compact
everything currently in the rbyd, and then append any pending attr.
Never needing to evaluate the attrs removes a big chunk of logic as long
as we can assert that the largest attr set fits after compaction.
But this lagging of commits presents a problem for deferred inlining, if
we detect an inlinable tree during compaction, we can't be sure it's
_actually_ inlinable until we evaluate our attr. Which we really don't
want to do.
The solution here is to move the problem up a level. Instead of trying
to determine when to inline purely from the provided attr, we require
higher-level functions to provide this info in the form of a "cutoff".
Where, if compaction results in fewer entries than this cutoff, the
higher-level function can instead inline.
This effectively allows the higher-level functions to intercept
unnecessary compactions that can be inlined.
So far this solution seems to work quite well, with the added plus of
consolidating the corner cases around inlined/inlining btrees in these
higher-level functions.
---
Note that this has the peculiar side-effect of allowing zero-weight,
non-inlined B-trees. Our previous internal B-tree struct using the sign
of an integer to determine inline-ness, this was changed to use just the
sign-bit for the condition as a sort of ones-complement width field.
I think this sort of encoding may actually bit a tiny bit more
efficient. I was poking around with thumb code and noticed there is no
actual "abs" instruction, with gcc outputing an "it" sequence. But there
is a cheap bit-clear "bic" instruction.
- len => size - these all refer to byte-arrays
- buf => buffer - this doesn't matter but buffer is currently used more
- delta => d - we use delta for weight deltas, gstate deltas, using a
slightly different name (if somehow even less descriptive) for byte
offset-offsets helps avoid name collisions a little bit
The storage changes in btree operations should've probably been a
separate commit but got wrapped up in these changes. Now the high-level
btree operations are responsible to the attr storage for all internal
btree commits, as defined by LFSR_BTREE_SCRATCHATTRS.
This leads to slightly less total RAM usage, since it allows the
low-level btree operations to cannibilize the attrs of the high-level
btree operations as a part of its unrolled-tail-recursive
implementation.
This also includes some other cleanup such as removing old commented out
parts.
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.
- The erased flag in lfsr_rbyd_t uses only a single bit, which is
wasteful for a heavily used struct in littlefs. We can use
rbyd.off=block_size to indicate the same state for free. Note that
when rbyd.off=block_size, we must treat rbyd as unerased anyways.
- Improved state handling in rbyd_append/commit when an error occurs.
I will be trying to make better use of cleanup gotos to make these
functions less unpredictable when an error occurs. Hopefully the state
of littlefs after an error can be well-defined in the future.
- Fixed sign-mismatch warnings in asserts when compiled outside of the
test runner.
There have already been a number of bugs that end up writing -1 out as
leb128s. The current encoder doesn't know the different betwee -1 and
0xffffffff, so asserting before this situation can happen is quite
important for preventing these bad leb128s from ever making it into a
stable version.
Also dropped LFS_ERR_OVERFLOW to use LFS_ERR_CORRUPT for bad leb128
encodings. These end up meaning the same thing to higher layers anyways.
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.
This implements a common B-tree using rbyd's as inner nodes.
Since our rbyds actually map to sorted arrays, this fits together quite
well.
The main caveat/concern is that we can't rely on strict knowledge on the
on-disk size of these things. This first shows up with B-tree insertion,
we can't split in preparation to insert as we descend down the tree.
Normally, this means our B-tree would require recursion in order to keep
track of each parent as we descend down our tree. However, we can
avoid this by not storing our parent, but by looking it up again on each
step of the splitting operation.
This brute-force-ish approach makes our algorithm tail-recursive, so
bounded RAM, but raises our runtime from O(logB(n)) to O(logB(n)^2)
That being said, O(logB(n)^2) is still sublinear, and, thanks to
B-tree's extremely high branching factor, may be insignificant.
If we combine rbyd ids and B-tree weights, we need 32-bit ids since this
will eventually need to cover the full range of a file. This simply
doesn't fit into a single word anymore, unless littlefs uses 64-bit tags.
Generally not a great idea for a filesystem targeting even 8-bit
microcontrollers.
So here is a tag encoding that uses 3 leb128 words. This will likely
have more code cost and slightly more disk usage (we can no longer fit
tags into 2 bytes), though with most tags being alt pointers (O(m log m)
vs O(m)), this may not be that significant.
Note that we try to keep tags limited to 14-bits to avoid an extra leb128 byte,
which would likely affect all alt pointers. To pull this off we do away
with the subtype/suptype distinction, limiting in-tree tag types to
10-bits encoded on a per-suptype basis:
in-tree tags:
ttttttt ttt00rv
^--^^- 10-bit type
'|- removed bit
'- valid bit
iiii iiiiiii iiiiiii iiiiiii iiiiiii
^- n-bit id
lllllll lllllll lllllll lllllll
^- m-bit length
out-of-tree tags:
ttttttt ttt010v
^---^- 10-bit type
'- valid bit
0000000
lllllll lllllll lllllll lllllll
^- m-bit length
alt tags:
kkkkkkk kkk1dcv
^-^^^- 10-bit key
'||- direction bit
'|- color bit
'- valid bit
wwww wwwwwww wwwwwww wwwwwww wwwwwww
^- n-bit weight
jjjjjjj jjjjjjj jjjjjjj jjjjjjj
^- m-bit jump
The real pain is that with separate integers for id and tag, it no
longer makes sense to combine these into one big weight field. This
requires a significant rewrite.
The original idea was weighted B-trees composed out of weighted rbyds,
with the two weight systems being independent. Descent down the B-tree
uses the same technique in the current metadata data-structure of
searching for which branch to take during fetch, basically getting the
search for free (well, on top of the already required O(m) fetch
operation).
But this is fundamentally flawed. While file names provide an absolute
reference for finding matches, weights are relative references. So we
don't have enough information to do weight-based lookup during fetch.
This smells just like the relative-vs-absolute key issues that led to
rbyd vs rbd trees in the first place...
One option is to do rbyd traversals at each B-tree node to build the
necessary information to figure out the weights. But with rbyd
traversals taking O(m log m), this makes B-tree lookups O(log n * m log m),
and B-tree traversals a messy O(n log n * m log m), which is acceptable, but
disapointing for what will likely be the most common operation in the
filesystem.
But the rbyd trees _are_ already weighted. A better solution might be to
go back and rethink the seperation of B-tree weights and rbyd ids.
Unfortunately, with only 16-bits available for rbyd ids, this would
likely require a rewrite of how rbyd tags are encoded...
- Caching is still presenting issues with the new requirements for
rbyd trees, in this case the default bd, with 64 byte progs, revealed
and issue where rcache could become outdated when reading from disk
while ignoring what's in the pcache.
It assumes the pcache will always override the rcache, but this is not
true after pcache is flushed.
This didn't happen before as the rcache and pcache don't
interact while writing in the previous implementation. Because of
these new requirements the caching system probably deserves a
rework...
- The quick tests for sublinear space utilization don't work when
prog_size is > a byte, fortunately we should always have NOR-like
geometry under test, so we can limit these asserts to NOR-like
geometry.
- Lots of problems fitting these tests into 512-byte block_size
geometries, which is a bit concerning. This may be a larger change
from the previous implementation than expected. This may deserve more
scrutiny at small block sizes to see how things fit, since the
sublinear space utilization doesn't really kick in at this scale...
On the other hand it may just be that these tests are too aggressive
for 512-byte block sizes, since they don't yet do compaction, which
should help with padding/crc overhead...
Previously the subtype was encoded above the suptype. This was an issue
if you wanted to, say, traverse all tags in a given suptype.
I'm not sure yet if this sort of functionality is needed, it may be
useful for cleaning up/replacing classes of tags, such as file struct
tags, but not sure yet. At the very least is avoids unintuitive tag
ordering in the tree, which could potential cause problems for
create/deletes.
New encoding:
tags:
iiiiiii iiiiitt ttTTTTT TTT0trv
^----^--------^-^^^- 16-bit id
'--------|-'||- 5-bit suptype (split)
'--||- 8-bit subtype
'|- perturb/remove bit
'- valid bit
lllllll lllllll lllllll lllllll
^- n-bit length
alts:
wwwwwww wwwwwww wwwwwww www1dcv
^^^-^- 28-bit weight
'|-|- color bit
'-|- direction bit
'- valid bit
jjjjjjj jjjjjjj jjjjjjj jjjjjjj
^- n-bit jump
Also a large amount of name changes and other cleanup.
It's quite lucky a spare bit is free in the tag encoding, this means we
don't need a reserved length value as originally planned. We end up using
all of the bits that overlap the alt pointer encoding, which is nice and
unexpected.
- primitive lfs_rbyd_fetch
- primitive lfs_rbyd_commit
- tag reading/progging and encoding machinery
The tag encoding scheme here uses pairs of leb128s, encoding either
a normal tag:
iiii iiiiiii iiiiiTT TTTTTTt ttttt0v
^--------^------^-^- 16-bit id
'------|-|- 8-bit type2
'-|- 6-bit type1
'- valid bit
llll lllllll lllllll lllllll lllllll
^- n-bit length
Or an alt pointer:
wwww wwwwwww wwwwwww wwwwwww wwwcd1v
^^^-^- 28-bit weight
'|-|- color bit
'-|- direction bit
'- valid bit
jjjj jjjjjjj jjjjjjj jjjjjjj jjjjjjj
^- n-bit jump
Note that two bits overlap the alt pointer dir/color encoding, this
is actually not a problem at all since some tags (crcs/fcrcs) don't
participate in the rbyd tree and can use these bits.
There's a number of benefits to using leb128s, which should probably
be written about, most notably is the abstraction of the device's
word-size. The "n-bits" above can be whatever word size works on the
device, trading off code-size for storage capabilities without breaking
compatibility with other devices. This will eventually be negotiated via
the superblock.
Initially I thought the fcrc would be sufficient for all of the
end-of-commit context, since indicating that there is a new commit is a
simple as invalidating the fcrc. But it turns out there are cases that
make this impossible.
The surprising, and actually common, case, is that of an fcrc that
will end up containing a full commit. This is common as soon as the
prog_size is big, as small commits are padded to the prog_size at
minimum.
.------------------. \
| metadata | |
| | |
| | +-.
|------------------| | |
| foward CRC ------------.
|------------------| / | |
| commit CRC -----' |
|------------------| |
| padding | |
| | |
|------------------| \ \ |
| metadata | | | |
| | +-. | |
| | | | +-'
|------------------| / | |
| commit CRC --------' |
|------------------| |
| | /
'------------------'
When the commit + crc is all contained in the fcrc, something silly
happens with the math behind crcs. Everything in the commit gets
canceled out:
crc(m) = m(x) x^|P|-1 mod P(x)
m ++ crc(m) = m(x) x^|P|-1 + (m(x) x^|P|-1 mod P(x))
crc(m ++ crc(m)) = (m(x) x^|P|-1 + (m(x) x^|P|-1 mod P(x))) x^|P|-1 mod P(x)
crc(m ++ crc(m)) = (m(x) x^|P|-1 + m(x) x^|P|-1) x^|P|-1 mod P(x)
crc(m ++ crc(m)) = 0 * x^|P|-1 mod P(x)
This is the reason the crc of a message + naive crc is zero. Even with an
initializer/bit-fiddling, the crc of the whole commit ends up as some
constant.
So no manipulation of the commit can change the fcrc...
But even if this did work, or we changed this scheme to use two
different checksums, it would still require calculating the fcrc of
the whole commit to know if we need to tweak the first bit to invalidate
the unlikely-but-problematic case where we happen to match the fcrc. This
would add a large amount of complexity to the commit code.
It's much simpler and cheaper to keep the 1-bit counter in the tag, even
if it adds another moving part to the system.
This fixes most of the remaining bugs (except one with multiple padding
commits + noop erases in test_badblocks), with some other code tweaks.
The biggest change was dropping reliance on end-of-block commits to know
when to stop parsing commits. We can just continue to parse tags and
rely on the crc for catch bad commits, avoiding a backwards-compatiblity
hiccup. So no new commit tag.
Also renamed nprogcrc -> fcrc and commitcrc -> ccrc and made naming in
the code a bit more consistent.
Previously forward-looking CRCs was just two new CRC types, one for
commits with forward-looking CRCs, one without. These both contained the
CRC needed to complete the current commit (note that the commit CRC
must come last!).
[-- 32 --|-- 32 --|-- 32 --|-- 32 --]
with: [ crc3 tag | nprog size | nprog crc | commit crc ]
without: [ crc2 tag | commit crc ]
This meant there had to be several checks for the two possible structure
sizes, messying up the implementation.
[-- 32 --|-- 32 --|-- 32 --|-- 32 --|-- 32 --]
with: [nprogcrc tag| nprog size | nprog crc | commit tag | commit crc ]
without: [ commit tag | commit crc ]
But we already have a mechanism for storing optional metadata! The
different metadata tags! So why not use a separate tage for the
forward-looking CRC, separate from the commit CRC?
I wasn't sure this would actually help that much, there are still
necessary conditions for wether or not a forward-looking CRC is there,
but in the end it simplified the code quite nicely, and resulted in a ~200 byte
code-cost saving.
Improve the lfs_file_close usage description to make it clearer that the configuration structure must remain valid for its lifetime
In reference to #722
The main change here from the previous test framework design is:
1. Powerloss testing remains in-process, speeding up testing.
2. The state of a test, included all powerlosses, is encoded in the
test id + leb16 encoded powerloss string. This means exhaustive
testing can be run in CI, but then easily reproduced locally with
full debugger support.
For example:
./scripts/test.py test_dirs#reentrant_many_dir#10#1248g1g2 --gdb
Will run the test test_dir, case reentrant_many_dir, permutation #10,
with powerlosses at 1, 2, 4, 8, 16, and 32 cycles. Dropping into gdb
if an assert fails.
The changes to the block-device are a work-in-progress for a
lazily-allocated/copy-on-write block device that I'm hoping will keep
exhaustive testing relatively low-cost.
Christopher Haster