Not sure how this was missed for so long, but we completely forget about
in-flight mroot attrs if we happen to uninline the mtree.
I guess this was missed because only some late-stage fs ops need to
commit mroot attrs (lfsr_fs_grow, lfsr_setattr, upgrades, etc), but
being able to commit to the mroot is definitely an operation we need to
support.
Fixing this in a non-awkward way was a bit tricky. We need some way to
commit both the provided attr-list and our new mtree, but all of the
lower layers only accept a single attr-list. The solution here
is to add a special tail-recursive LFSR_TAG_ATTRS that can be used to
chain together multiple attr-lists. This solves the problem quite
elegantly and may actually be useful in the future?
It takes a bit of code:
code stack
before: 33850 2584
after: 33926 (+0.2%) 2592 (+0.3%)
But this solves our final lfsr_fs_grow-related bug. No more mroot-split
hacks in test_grow, and we can now grow any stuck filesystem.
This acts as a marker to indicate a fuzz test. It should reference a
define, usually SEED, that can be randomized to get interesting test
permutations.
This is currently unused, but could lead to some interesting uses such
as time-based fuzz testing. It's also just useful for inspecting the
tests (make test-list).
littlefs does this internally anyways. The original intention was to
make sure non-powers-of-2 don't break, but we don't really validate what
these end up aligned to. And the intentional mismatch risks confusion
when debugging.
If it's worth testing non-powers-of-2, it should be an explicit test.
The mleafweight naming is... not great...
Renaming mleaf_bits -> mdir_bits and replacing mleafweight with explicit
shifts of 1 << mdir_bits seems to get the job done without introducing a
new and potentially confusing name.
This was a lesson learned from recycle_bits. Sometimes more helpers just
makes code less, not more, readable.
This makes a bit more sense with the new block_recycles name.
block_recycles=0 (previously block_recycles=1) requires 1 erase, but it
doesn't really "recycle" the block. With this change, block_recycles=1
"recycles" the block once (2 erases in total) before relocating, which I
think is a bit more intuitive.
Note, this sort of messes with our power-of-2 rounding, as the
block_recycles is technically rounded down to the nearest power-of-2
after adding 1:
- block_recycles=1022 -> 512 erases
- block_recycles=1023 -> 1024 erases
- block_recycles=1024 -> 1024 erases
- block_recycles=1025 -> 1024 erases
But I'm going to keep the block_recycles description more-or-less as is
for now, as I think this extra detail is more confusing than useful,
powers-of-2 stay powers-of-2, and the <=block_recycles contraint is not
violated.
The original goal here was to restore all of the revision count/
wear-leveling features that were intentionally ignored during
refactoring, but over time a few other ideas to better leverage our
revision count bits crept in, so this is sort of the amalgamation of
that...
Note! None of these changes affect reading. mdir fetch strictly needs
only to look at the revision count as a big 32-bit counter to determine
which block is the most recent.
The interesting thing about the original definition of the revision
count, a simple 32-bit counter, is that it actually only needs 2-bits to
work. Well, three states really: 1. most recent, 2. less recent, 3.
future most recent. This means the remaining bits are sort of up for
grabs to other things.
Previously, we've used the extra revision count bits as a heuristic for
wear-leveling. Here we reintroduce that, a bit more rigorously, while
also carving out space for a nonce to help with commit collisions.
Here's the new revision count breakdown:
vvvvrrrr rrrrrrnn nnnnnnnn nnnnnnnn
'-.''----.----''---------.--------'
'------|---------------|---------- 4-bit relocation revision
'---------------|---------- recycle-bits recycle counter
'---------- pseudorandom nonce
- 4-bit relocation revision
We technically only need 2-bits to tell which block is the most
recent, but I've bumped it up to 4-bits just to be safe and to make
it a bit more readable in hex form.
- recycle-bits recycle counter
A user configurable counter, this counter tracks how many times a
metadata block has been erased. When it overflows we return the block
to the allocator to participate in block-level wear-leveling again.
This implements our copy-on-bounded-write strategy.
- pseudorandom nonce
The remaining bits we fill with a pseudorandom nonce derived from the
filesystem's prng. Note this prng isn't the greatest (it's just the
xor of all mdir cksums), but it gets the job done. It should also be
reproducible, which can be a good thing.
Suggested by ithinuel, the addition of a nonce should help with the
commit collision issue caused by noop erases. It doesn't completely
solve things, since we're only using crc32c cksums not collision
resistant cryptographic hashes, but we still have the existing
valid/perturb bit system to fall back on.
When we allocate a new mdir, we want to zero the recycle counter. This
is where our relocation revision is useful for indicating which block is
the most recent:
initial state: 10101010 10101010 10101010 10101010
'-.'
+1 zero random
v .----'----..---------'--------.
lfsr_rev_init: 10110000 00000011 01110010 11101111
When we increment, we increment recycle counter and xor in a new nonce:
initial state: 10110000 00000011 01110010 11101111
'--------.----''---------.--------'
+1 xor <-- random
v v
lfsr_rev_init: 10110000 00000111 01010100 01000000
And when the recycle counter overflows, we relocate the mdir.
If we aren't wear-leveling, we just increment the relocation revision to
maximize the nonce.
---
Some other notes:
- Renamed block_cycles -> block_recycles.
This is intended to help avoid confusing block_cycles with the actual
physical number of erase cycles supported by the device.
I've noticed this happening a few times, and it's unfortunately
equivalent to disabling wear-leveling completely. This can be improved
with better documentation, but also changing the name doesn't hurt.
- We now relocate both blocks in the mdir at the same time.
Previously we only relocated one block in the mdir per recycle. This
was necessary to keep our threaded linked-list in sync, but the
threaded linked-list is now no more!
Relocating both blocks is simpler, updates the mtree less often,
compatible with metadata redundancy, and avoids aliasing issues that
were a problem when relocating one block.
Note that block_recycles is internally multiplied by 2 so each block
sees the correct number of erase cycles.
- block_recycles is now rounded down to a power-of-2.
This makes the counter logic easier to work with and takes up less RAM
in lfs_t. This is a rough heuristic anyways.
- Moved the lfs->seed updates into lfsr_mountinited + lfsr_mdir_commit.
This avoids readonly operations affecting the seed and should help
reproducibility.
- Changed rev count in dbg scripts to render as hex, similar to cksums.
Now that we using most of the bits in the revision count, the decimal
version is, uh, not helpful...
Code changes:
code stack
before: 33342 2640
after: 33434 (+0.3%) 2640 (+0.0%)
These seem fitting here, even if the test defines aren't "real defines".
The duplicate expressions should still be side-effect free and easy to
optimize out.
This should also avoid future lfs_min32 vs intmax_t issues.
This turned into a sort of system-wide refactor based on learned
knowledge of what we can do with lfsr_attr_t.
The big changes:
- Reverted LFSR_ATTR to mainly take lfsr_data_t again, keeping
lfsr_data_t as the default data representation in the codebase.
Now that we know
LFSR_ATTR_CAT_ still provides concatenation mechanics, and LFSR_ATTR_
provides a way to edit in-flight lfsr_attr_ts.
- Dropped lfsr_cat_t, replaced with explicit const void* + uint16_t,
tried to limit to low-level operations and prefer passing aroud
lfsr_attr_t and lfsr_data_t at a high-level.
Note this cat + cat_count pair is quite similar to the common attrs +
attr_count and buffer + size arguments.
- Adopted lfsr_attr_t more in mid-level functions, lfsr_rbyd_appendattr,
lfsr_rbyd_appendcompactattr, lfsr_file_carve, etc. This is a bit more
ergonomical, allows for use of LFSR_ATTR* macros, and in theory might
even save a bit of stack.
Unfortunately this seems to have resulted in a net hit to code cost,
though I still think it's worth it for the internal ergonomics:
code stack
before: 33652 2624
after: 33780 (+0.4%) 2640 (+0.4%)
Investigating further suggests this may just be the result of compiler
noise and changes to argument placement. lfsr_attr_t does touch a lot of
code...
It's interesting to note the adoption of lfsr_attr_t in
lfsr_rbyd_appendattr* and friends prevents their transformation into
.isra functions, though this doesn't seem to impact code cost too much:
function (5 added, 5 removed) osize nsize dsize
lfsr_cat_size - 48 +48 (+100.0%)
lfsr_file_carve - 1600 +1600 (+100.0%)
lfsr_rbyd_appendattr - 2120 +2120 (+100.0%)
lfsr_rbyd_appendattr_ - 244 +244 (+100.0%)
lfsr_rbyd_appendcompactattr - 68 +68 (+100.0%)
lfsr_rbyd_appendcompactrbyd 144 152 +8 (+5.6%)
lfsr_file_truncate 298 314 +16 (+5.4%)
lfsr_mdir_commit__ 1056 1112 +56 (+5.3%)
lfsr_mdir_compact__ 502 526 +24 (+4.8%)
lfsr_rbyd_appendattrs 132 138 +6 (+4.5%)
lfsr_file_fruncate 386 402 +16 (+4.1%)
lfsr_data_frombtree 84 86 +2 (+2.4%)
lfsr_rbyd_appendcksum 512 520 +8 (+1.6%)
lfsr_file_opencfg 572 580 +8 (+1.4%)
lfsr_rename 608 616 +8 (+1.3%)
lfsr_mkdir 500 504 +4 (+0.8%)
lfsr_bd_prog 278 280 +2 (+0.7%)
lfsr_mdir_commit 2364 2360 -4 (-0.2%)
lfsr_bshrub_commit 716 712 -4 (-0.6%)
lfsr_file_sync 526 514 -12 (-2.3%)
lfsr_file_flush_ 1868 1820 -48 (-2.6%)
lfsr_remove 456 436 -20 (-4.4%)
lfsr_fs_fixgrm 168 160 -8 (-4.8%)
lfsr_cat_size.isra.0 42 - -42 (-100.0%)
lfsr_file_carve.isra.0 1596 - -1596 (-100.0%)
lfsr_rbyd_appendattr.isra.0 2088 - -2088 (-100.0%)
lfsr_rbyd_appendattr_.isra.0 232 - -232 (-100.0%)
lfsr_rbyd_appendcompactattr.isra.0 56 - -56 (-100.0%)
TOTAL 33652 33780 +128 (+0.4%)
So now, instead of one data type trying to do everything, we have two:
1. lfsr_data_t - Readable data, either in-RAM or on-disk
2. lfsr_cat_t - Concatenated data for progging, may be either a simple
in-RAM buffer or an indirect list of lfsr_data_ts
This comes from an observation that most lfsr_attr_t datas were either
simple buffers, NULL, or required the indirect concatenated datas
anyways (concatendated file fragments). By separating lfsr_cat_t and
lfsr_data_t, maybe we can save RAM in lfsr_attr_t by not needing the
three words necessary for the less-common disk references.
Note the interesting tradeoff:
Simple in-RAM buffers/NULL decrease by 1 word (4 bytes):
lfsr_data_t lfsr_cat_t
.---+---+---+---. .---+---+---+---.
|0| size | => |0| size |
+---+---+---+---+ +---+---+---+---+
| ptr | | ptr |
+---+---+---+---+ '---+---+---+---'
| (unused) |
'---+---+---+---'
'-------.-------' '-------.-------'
12 bytes 8 bytes
While on-disk references increase by 2 words (8 bytes):
lfsr_data_t lfsr_cat_t lfsr_data_t
.---+---+---+---. .---+---+---+---. .---+---+---+---.
|1| size | => |1| size | .>|1| size |
+---+---+---+---+ +---+---+---+---+ | +---+---+---+---+
| block | | ptr -------' | block |
+---+---+---+---+ '---+---+---+---' +---+---+---+---+
| off | | off |
'---+---+---+---' '---+---+---+---'
'-------.-------' '-----------------.-----------------'
12 bytes 20 bytes
Unless the on-disk references also need concatenation, in which case
this still saves 1 word (4 bytes).
Note I'm not sure this type split is generalizable to other systems. In
littlefs we can't use recursion, so progging concatenated datas already
required two nested functions, and we happen to never need to read
concatenated data, allowing us to completely omit that functionality. In
other systems, where maybe disk-reference attrs are more common, this
tradeoff may not make sense.
Some other things to note:
- We're also losing the inlined-data representation in this change.
Unfortunately earlier lfsr_data_t measurements showed that this didn't
really contribute much. It saved RAM in name attrs but added quite a
bit of complexity to lfsr_data_t operations.
- By separating simple/cat and RAM/disk, we reduce the abused size bits
from 2-bits down to 1-bit. This doesn't really matter for our current
31/28-bit littlefs impl, but is nice in that it reenables the
theoretical 31/31-bit littlefs impl without in-RAM data-structure
changes.
There are a few temporary hacks that need to be figured out, but this is
already showing code/stack savings. Which is fascinating considering the
new lfsr_cat_* functions and increased temporary allocations:
code stack
before: 33856 2824
after: 33812 (-0.1%) 2800 (-0.8%)
The core problem is that we weren't updating dropped mdirs with weight=0
if the mdir was compacted at the same time. This is hard to notice,
because most operations that can drop don't care about the mdir
afterwards, but in lfsr_fs_fixorphans this caused the fixorphan loop to
think it might still have orphans it could remove.
The implementation is very subtle here:
- In lfsr_mdir_commit_, if an error occurs during lfsr_mdir_compact__,
we need to revert to the original mdir state to allow fallback to mdir
split.
- In lfsr_mdir_commit_, if an error occurs during lfsr_mdir_commit__
(even after a compact), we need to update the mdir in case a drop
reduced the mdir weight to zero.
We also need to update the mdir for things like erased state, but this
doesn't come into play in the compaction route.
Fixed the bug by updating the mdir copy before lfsr_mdir_commit__.
Also added asserts to all insert/delete operations in test_mtree.toml.
We already had drop-during-compaction tests, but these didn't check that
the mdir was updated correctly. The new asserts catch this bug and
should prevent a regression.
Before:
LFSR_ATTR(RM(SUBMASK(REG)), 0, BUF("hi", 2))
Now:
LFSR_ATTR(
LFSR_TAG_RM | LFSR_TAG_SUBMASK | LFSR_TAG_REG, 0,
LFSR_DATA_BUF("hi", 2))
Yes, it's more verbose now.
But there were a couple reasons for dropping the idea:
- The implicit prefixing is a bit magical, and not really all that
common in C code. It would likely confuse new users on first read.
- The implicitly prefixing macros did not play will with macro expansion
rules.
In particular, because the nested not-yet-prefixed macros aren't
really macros, they aren't expanded as a part of argument prescan.
This led to surprising compile-time errors, and prevented recursive
attr-lists (which may be useful for shrubs).
- Implicit prefixes is not very C-like, and in particular it gets in the
way of sed/grep operations on source files.
- RM(SUBMASK(REG)) for combining tags is (IMO) ugly, compared to
LFSR_TAG_RM | LFSR_TAG_SUBMASK | LFSR_TAG_REG, even if the latter
requires more typing.
- Sometimes you need runtime-dependent TAG/DATA values, which implicit
prefixing gets in the way of. The LFSR_TAG_TAG(tag)/
LFSR_DATA_DATA(tag) backdoors worked around this, but they are even
more magical, and added noise to a not-actually-all-that-uncommon use
case.
And it's really not _that_ much extra effort to write out the prefixes
everywhere.
lfs.c:
lines bytes
before: 16894 537171
after: 16907 (+0.1%) 538340 (+0.2%)
tests/*.toml:
lines bytes
before: 53306 1811035
after: 54517 (+2.3%) 1851006 (+2.2%)
qadte came in quite handy again for refactoring the tests without
completely losing my sanity.
So instead of:
uint8_t mptr_buf[LFSR_MPTR_DSIZE];
int err = lfsr_btree_commit(lfs, &mtree_.u.btree, 0, LFSR_ATTRS(
LFSR_ATTR(
MDIR, +lfsr_mleafweight(lfs),
FROMMPTR(lfsr_mdir_mptr(&mdir_), &mptr_buf))));
This can be written as:
int err = lfsr_btree_commit(lfs, &mtree_.u.btree, 0, LFSR_ATTRS(
LFSR_ATTR(
MDIR, +lfsr_mleafweight(lfs),
FROMMPTR(lfsr_mdir_mptr(&mdir_)))));
Explicit stack allocation is still possible with the DATA hole, though a
bit more annoying:
attrs[attr_count++] = LFSR_ATTR(
MDIR, +lfsr_mleafweight(lfs),
DATA(lfsr_data_frommptr(
lfsr_mdir_mptr(&mdir_),
&buf[buf_size])));
buf_size += LFSR_MPTR_DSIZE;
The main motivation for this change is to be consistent with
LFSR_DATA_CAT, which was already implicitly stack allocating. The macros
that take arrays are relatively error-prone otherwise (LFSR_DATA_CAT,
LFSR_ATTRS, etc).
This does come with the benefit that the required buffer size is
implicitly provided by the macro, so no worry of it falling out-of-sync
externally. However this does come with the tradeoff of compound literal
lifetimes, which requires the result to live only as long as the current
expression.
Hopefully the fact that these are MACROs signal that they need special
care to any new developers...
Unfortunately, the use of compound literals also brings a surprising
code/stack cost:
code stack
before: 33912 2872
after: 34016 (+0.3%) 2896 (+0.8%)
Currently I can think of two reasons:
1. It's not possible to declare an uninitialized compound literal.
This probably sounds like a good thing to memory-safety fans, and
initialized is probably a good default for variable declaration, but
the reality is the required initialization does add useless code.
This specific use of compound literals is also low-risk given that we
immediately pass the literal to an lfsr_data_from* function, which
does the initialization.
2. We sometimes share on-stack buffers between branches of ternary
expressions since we know their use is exclusive. These macros sort
of get in the way of that.
What I find a bit curious is GCC doesn't seem capable of optimizating
away these overheads, which I would think would be possible given that
GCC knows all the information of how these buffers end up used.
I've noticed in general compound literals add overhead when the
underlying semantics don't really change. I wonder if this is because
compound literals are relatively new/unused, or some required
side-effects I'm missing. Maybe this will improve in the future?
Anyways, I'm keeping this change for now, since it does improve the
internal attr-list ergonomics/safety. Though these sort of changes are
always open to be revisited in the future.
Interestingly, the future-theoretical transpilation to c89 may save
code/stack because of this, which raises some questions...
I think this is a case where separating the logic out into distinct
functions does more harm than good, by making it harder to understand
how all the different moving parts interact.
This is especially important for lfsr_mdir_commit, since this is where
all atomic operations in the filesystem get tied together. Having atomic
updates complete in different functions was particularly concerning
since it carries some implicit requirements (must not error after!).
The end result is a cumbersome function, but at least internally
relatively straightforward in how the commit propagates through the
mtree/mroot chain and internal state.
---
The other benefit of inlining is better code deduplication, since we
can treat the mroot as a normal mdir until it triggers a split or
relocation.
We can also deduplicate the grm patching, though there may be a better
way to implement this. There are still some awkward bits in the logic.
code stack
before: 33856 2888
after: 33764 (-0.3%) 2832 (-2.0%)
We have bleafs (bleaves?) now, so the mleaf name just makes too much
sense. Even though it's used nowhere else outside of mid decoding, and
may be a bit confusing.
After all this time it feels weird to use a const lfs_t parameter, but
that's really what the mid/mleaf functions should take. These functions
are a bit of a special case as lfsr_mleafweight really wants to just be
a constant.
Code size did not change.
These shims, originally intended to remap the tests to new internal
APIs without a significant rewrite, are a long-outstanding piece of
technical debt. Now that the internal API is more stable, it's time for
that rewrite.
Reasons for not keeping the internal shims:
- They add more complexity to the test suites.
- They come with (out-of-date) constraints that limit what we can test.
- It's more difficult to debug test failures, with 2 layers and all.
I ended up writing a small tree editor out of tree to do most of this
rewrite.
Did it save time? Probably not. But it was quite a bit more fun than
manaully rewriting ~21K lines of code.
With lfsr_mdir_t being a logical cursor pointing to a specific metadata
entry in the on-disk mdir, we don't really need the mid to be provided
on every lookup call (may have jumped the gun a bit in the attr-list
changes).
In the rare case we need to lookup unrelated mids, we call always call
lfsr_rbyd_lookup on the underlying rbyd.
This saves a little bit of code/stack:
code stack
before: 33964 2896
after: 33852 (-0.3%) 2888 (-0.3%)
Well this was quite tedious, but these tests are valuable since our
mtree has a number of hard-to-reach edge cases.
This mainly ports over to the new attr-list format for mdir commits, but
also cleans up a couple of lingering tedious TODO things:
- mtree tests now use the new mdir commit attr-list format.
- Reoriented most tests to use namelookups instead of mid lookups.
Using mid lookups in testing is/was really fragile, since it depends
on exactly how mids get split and moved around.
namelookups are more robust, by design they don't care about the
underlying mtree structure. And really, namelookups are what we care
about in the mtree, mids are just a mechanism for mtree updates to
work.
We don't remove all mid checks though, we just compare against
namelookup-derived mids when it matters (the mtree_opened tests for
example).
- The names we use in testing have also been updated to no longer create
invalid mtrees, i.e. names are ordered correctly and always have a
did.
The previous mess always risked triggering asserts with false
positives.
- By adopting namelookup in the tests, we can actually test the on-disk
state of fuzz testing.
Though note we can't change names once written, without invalidating
our mtree. This limits fuzz testing a little bit, but it's still a
big improvement over the previous fuzz tests.
- Dropped mtree tests that no longer really make sense.
Mainly that the did should never be deleted, so you can never end up
with an empty mtree, dropping the left-most mdir, etc.
There were still a few of things lingering around.
With this, all tests are working again with the attr-list changes. Wooh.
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.
Much like the lfsr_o_* functions, I think we should avoid too many
convenience layers for what really are operations on struct fields.
Otherwise you quickly end up with a lot of boilerplate that just saves a
couple extra characters at invocation. Characters that also help convey
what is being accessed.
- Fixed fsync tests, which needed more lfsr_file_sync calls so multiple
file handles can be opened correctly.
Though this points out there's no way to open a rdonly file on an
uncreated file until sync is called... But I guess you wouldn't be
able to recieve broadcasts until sync anyways? at which point the file
would be created?
- Update mtree tests based on the new remove behavior for regular files.
Before this changed the mid to -1, now it points to the next mid with
the zombie flag set. Upper layers use this to migrate mdirs to a
scratch file if necessary.
- Removed the orphaned mdir test. We don't create orphaned mdirs
anymore.
Technically, orphaned mdirs are currently possible if we lose power in
the middle of the mtree update, but this is a bug and should be fixed
(previous revisions did not have this issue).
While the multi per-type linked-lists were cool and could save RAM in
some structs (at the cost of RAM in the lfs_t struct), this is simpler,
and simpler is good.
The motivation to revert:
1. I noticed most file types have some sort of flags: files,
traversals (future), (not dirs but maybe in the future). These flags
can be merged with the type field to give us typed mdirs at almost
no RAM cost.
2. Using a single linked-list makes it cheaper to add more file types,
which may be useful for managing bookmarks (differently) and scratch
files.
This comes at a runtime cost, since all scans look at all opened
structs, but we really, _really_ don't care about a constant non-IO
runtime cost.
There are code benefits, since we don't need nested iterators to access
all opened mdirs, but also some code cost when we want to filter by
type. As expected stack took a small hit. Humorously, the struct savings
in lfs_t perfectly canceled out the struct hit to lfsr_dir_t:
code stack structs
before: 32992 2968 1080
after: 33004 (+0.0%) 2976 (+0.3%) 1080 (+0.0%)
This trades a runtime check for a different function call. Enforcing
some minor semantics in the function's type/asserts.
This also makes it so there are no special tag bits used during rbyds
lookup, only rbyd commits.
In theory this saves a bit of code, we don't have a runtime check, but
in practice the extra function apparently outweighs the cost of the
runtime check:
code stack
before: 31956 2880
after: 32024 (+0.2%) 2880 (+0.0%)
It turns out permanent root bookmark creates some rather interesting
constraints on our mtree:
1. We can never delete all mids, since at least one mid needs to exist
to represent the root's bookmark.
2. We can never revert to an inlined mdir after uninlining, since our
root bookmark always exists to stop this. This is an unfortunate
downside as it would be nice to be able to reinline mdirs, but not
the end of the world.
This restricts what operations are possible, and transitively, what we
can test.
This commit drops the removal of root bookmarks in test_mtree, which was
a workaround to keep tests from early implementation running. This was
preventing some minor optimizations. This required dropping some tests,
but these tests tested operations that aren't really possible in
practice.
Dropping the removal of root bookmarks allowed for a minor optimization
in lfsr_mdir_drop, and may lead to more in the future (or maybe just
stricter asserts):
code stack
before: 31280 2648
after: 31208 (-0.2%) 2648 (+0.0%)
This ended up being much less of a simplification than I hoped it would.
It's still easier/more efficient to revert to a relocation in most cases
when dropping in an mdir split, and the small gain from simplifying how
drops/commits interact is overshadowed by the code duplication necessary
to separate lfsr_mdir_drop out from lfsr_mdir_commit:
code stack
before: 30952 2528
after: 31280 (+1.1%) 2648 (+4.7%)
Still, this does at least simplify the logical corner cases (we don't
need to abort commits when droppable anymore), and lfsr_mdir_drop is
ultimately necessary for supporting lazy file creation.
Also having a fix-orphans step during mount allows other littlefs
implementations the option to create orphanned mdirs without compat
issues. So this ends up the more flexible approach.
It _might_ be worth having both eager mdir drops and an explicit
lfsr_mdir_drop for lazy file creation in the future, but I doubt this
will end up worth the code duplication...
---
Oh right, I forgot to actually describe this change.
This trades eager mdir drops:
1. Drop mdirs from the mtree immediately as soon as their weight goes
to zero.
For lazy mdir drops:
1. Drop mdirs from the mtree in a second commit.
2. Scan and drop orphaned mdirs on the first write after mount.
This sounds very similar to the previous "deorphan" scan, which risked
an extreme performance cost during mount, but it should be noted this
orphan scan only needs to touch every mdir once. This makes it no worse
than the overhead of actually mounting the filesystem.
We can also keep an eye out for orphaned mdirs when we mount, so no
extra scan is needed unless there was an unlucky powerloss.
Eager mdir dropping sounds simpler, but thanks to deferred commits
introduces some subtle complexity around aborting commits that would
drop an mdir to zero. Remember commits are viewable on-disk as soon as a
commit completes.
In _theory_, lazy mdir drops simplify the logic around committing to
mdirs.
Though the real kicker is that lazy mdir drops are required for lazy file
creation.
The current idea for lazy file creation involves tracking mid-less
opened-but-not-yet-created files. These files can have bshrubs, so they
need space on an mdir somewhere. But they aren't actually created yet,
so they don't have an mid.
This is fine (though it's probably going to be tricky) as long as we
allocate an mid on file sync, but there is always a risk of losing power
with mdirs that contain only RAM-backed files. Fortunately, no-mids
means no orphaned files, but it does mean orphaned mdirs with no synced
contents.
Long story short, lazy mdir drops are currently a necessary evil, and
logical simplification, that unfortunately comes with some cost.
The idea here is to revert moving redund blocks into lfsr_rbyd_t, and
instead just keep a redundant copy of the rbyd blocks in the redund
blocks in lfsr_mdir_t.
Surprisingly, extra overhead in lfsr_mdir_t ended up with worse stack
usage than extra overhead in lfsr_rbyd_t. I guess we end up allocated
more mdirs than rbyds, which makes a bit of sense given how complicated
lfsr_mdir_commit is:
code stack structs
redund union: 30976 2496 1072
redund in rbyd: 30948 (-0.1%) 2528 (+1.3%) 1100 (+2.6%)
redund in mdir: 31000 (+0.1%) 2536 (+1.6%) 1092 (+1.8%)
The mdir option does seem to improve struct overhead, but this hasn't
been a reliable measurement since it doesn't take into account how many
of each struct is allocated.
Given that the mdir option is inferior in both code and stack cost, and
requires more care to keep the rbyd/redund blocks in sync, I think I'm
going to revert this for now but keep the commit in the commit history
since it's an interesting comparison.
This simplifies dependent structs with redundancy, mainly lfsr_mdir_t,
at a significant RAM cost:
code stack structs
before: 30976 2496 1072
after: 30948 (-0.1%) 2528 (+1.3%) 1100 (+2.6%)
Which, to be honest, is not as bad as I thought it would be. Though it
is still pretty bad for no new features.
The motivation for this change:
1. The organization of the previous lfsr_mdir_t struct was a bit hacky
and relied on exact padding so the redund block array and rbyd block
lined up at the right offset.
2. The previous organization prevented theoretical "read-only rbyd
structs" that could omit write-related fields, e.g. eoff and cksum.
This idea is currently unused.
3. The current mdir=level-1, btree/data=level-0 redund design makes this
RAM tradeoff pretty bad, but in theory higher btree redund levels
would need the extra redund blocks in the rbyd struct anyways.
Still, the RAM impact to the current default configuration means this
should probably be reverted...
- Renamed mdir->u.m to mdir->u.mdir.
- Prefer mdir->u.rbyd.* where possible.
- Changed file/dir mdirs to be stored directly, requiring a cast to
lfsr_openedmdir_t to enroll in the opened mdir list.
Reverted to one set of signed lfsr_mid_rid/bid functions, and tried to
make their usage more consistent.
We have two ways to compare mdirs now, lfsr_mdir_cmp (compares block
addresses) and lfsr_mdir_bid (compares mids), and it's not very clear
when to use which one. lfsr_mdir_cmp is a bit more robust in weird mid
cases (mainly inlined mdirs when mroot mid=-1), so currently preferring
that.
Also did some bit twiddling to preserve mid=-1 => bid=-1 and rid=-1,
this save a bit of code:
code stack
before: 31056 2488
after: 30972 (-0.3%) 2496 (+0.3%)
There's only one mtree in a given filesystem. With the recent
lfsr_mdir_commit restructure, it makes more sense for the mtree to be
implicit.
code stack
before: 31096 2480
after: 31016 (-0.3%) 2480 (+0.0%)
Originally, the intention of this rework was to make it possible to
shrub the mtree, i.e. allow an mshrub, i.e. inline the root rbyd of the
mtree to be inlined in the mroot.
This would allow small mtrees, 2, 3, etc mdirs, to save a block that
would be needed for the mtree's root.
But as the mshrub was progressing, minor problems kept unfolding, and
ultimately I've decided to shelve the idea of mshrubs for now. They add
quite a bit of complexity for relatively little gain:
- bshrubs are just complicated to update. They require a call to
lfsr_mdir_commit to update the inlined-root, which is a bit of a
problem when your mshrub needs to be updated inside lfsr_mdir_commit,
and your system disallows recursion...
Recursion _can_ be avoided by separate bshrub commit variants that go
through either lfsr_mdir_commit or lfsr_mdir_commit_, but this
complicates things and requires some code duplication, weakening the
value of reusing the bshrub data-structure.
- It's not always possible to compact the mshrub's backing mroot when
we need to modify the mshrub.
If an mroot becomes full and needs to split, for example, we need to
allocate the new mdirs, update the (new) mshrub, and then commit
everything into the mroot when we compact. But the "update the (new)
mshrub" step can't be done until after we compact, because the mroot
is by definition full.
This _can_ also be worked around, by building an attr list containing
all of the mshrub changes, and committing the mshrub/mroot changes in
the same transaction, but this complicates things and increases the
stack cost for the current hot-path.
- Every shrub needs a configurable shrub size, and the mshrub is no
exception. This adds another config option and complicates shared
shrub eviction code.
- The value for mshrubs is not actually that great.
Unlike file bshrubs, there's only one mshrub in the filesystem, and
I'm not sure there's a situation where a filesystem has >1 mdirs and
the exact number of allocated blocks is critical.
And this complexity is reflected in code cost and robustness, not to
mention developer time. I think for littlefs this is just not worth
doing. At least not now.
We can always introduce mshrubs in a backwards compatible manner if
needed.
---
But this rework did lead to better code organization around mdir commits
and how they update the mtree/mroot, so I'm keeping those changes.
In general lfsr_mdir_commit has been broken up into mtree/mroot specific
functions that _do_ propagate in-device changes. Any commit to the mroot
changes the on-disk state of the filesystem anyways, so the mroot commit
_must_ be the last thing lfsr_mdir_commit does.
This leads to some duplicated updates, but that's not really a problem.
Here's the new call graph inside lfsr_mdir_commit:
lfsr_mdir_commit
.---------' | | | '-----------------.
v | | '-----------------. |
lfsr_mtree_commit | '--------. | |
'---------. | | | |
v v | | |
lfsr_mroot_commit | | |
| '--------. | | |
| v v | |
| lfsr_mdir_commit_ | |
| .--------' '--------. | |
| | .-----------------|-' |
v v v v v
lfsr_mdir_commit__ lfsr_mdir_compact__
This rework didn't really impact code/stack that much. It added a bit of
code, but saved a bit of RAM. The real value is that the narrower-scoped
functions contain more focused logic:
code stack
before: 30780 2504
after: 31096 (+1.0%) 2480 (-1.0%)
This is just a useful type to have to make the code a bit more
readable.
This doesn't affect the code that much, except we are making more
on-stack copies of mptrs since the mdir doesn't technically contain
a mutable mptr. Maybe this should change?
code stack
before: 30768 2496
after: 30776 (+0.0%) 2504 (+0.3%)
This gives the mtree a dedicated type, with direct mptrs (single mdirs)
being stored decoded, instead of encoding into leb128s. This avoids
encoding/decoding in some cases.
This change is currently a net downgrade, but only because we still have
all of the inlined btree code. Eventually this inlined btree code should
be removed:
code stack
before: 31316 2064
after: 31480 (+0.5%) 2072 (+0.4%)
Also tweaked the tests to no longer test dropping the mtree down to
zero size. Thanks to root bookmarks, we never actually do this, and it
simplifies lfsr_mdir_commit to not support this.
My current thinking is that these are conceptually different types, with
BTREE tags representing the entire btree, and BRANCH tags representing
only the inner btree nodes. We already have multiple btree tags anyways:
btrees attached to files, the mtree, and in the future maybe a bmaptree.
Having separate tags also makes it possible to store a btree in a btree,
though I don't think we'll ever use this functionality.
This also removes the redundant weight field from branches. The
redundant weight field is only a minor cost relative to storage, but it
also takes up a bit of RAM when encoding. Though measurements show this
isn't really significant.
New encodings:
btree encoding: branch encoding:
.---+- -+- -+- -+- -. .---+- -+- -+- -+- -.
| weight | | blocks |
+---+- -+- -+- -+- -+ ' '
| blocks | ' '
' ' +---+- -+- -+- -+- -+
' ' | trunk |
+---+- -+- -+- -+- -+ +---+- -+- -+- -+- -'
| trunk | | cksum |
+---+- -+- -+- -+- -' '---+---+---+---'
| cksum |
'---+---+---+---'
Code/RAM changes:
code stack
before: 30836 2088
after: 30944 (+0.4%) 2080 (-0.4%)
Also reordered other on-disk structs with weight/size, so such structs
always have weight/size as the first field. This may enable some
optimizations around decoding the weight/size without needing to know
the specific type in some cases.
---
This change shouldn't have affected functionality, but it revealed a bug
in a dtree test, where a did gets caught in an mdir split and the split
name makes the did unreachable.
Marking this as a TODO for now. The fix is going to be a bit involved
(fundamental changes to the opened-mdir list), and similar work is
already planned to make removed files work.
Since we need an bptr type internally, a block pointer, which is a bit
more complicated than just a single address, calling our mdir pairs
mptrs makes sense.
Ended up changing the name of lfsr_mtree_traversal_t -> lfsr_traversal_t,
since this behaves more like a filesytem-wide traversal than an mtree
traversal (it returns several typed objects, not mdirs like the other
mtree functions for one).
As a part of this changeset, lfsr_btraversal_t (was lfsr_btree_traversal_t)
and lfsr_traversal_t no longer return untyped lfsr_data_ts, but instead
return specialized lfsr_{b,t}info_t structs. We weren't even using
lfsr_data_t for its original purpose in lfsr_traversal_t.
Also changed lfsr_traversal_next -> lfsr_traversal_read, you may notice
at this point the changes are intended to make lfsr_traversal_t look
more like lfsr_dir_t for consistency.
---
Internally lfsr_traversal_t now uses a full state machine with its own
enum due to the complexity of traversing the filesystem incrementally.
Because creating diagrams is fun, here's the current full state machine,
though note it will need to be extended for any
parity-trees/free-trees/etc:
mrootanchor
|
v
mrootchain
.-' |
| v
| mtree ---> openedblock
'-. | ^ | ^
v v | v |
mdirblock openedbtree
| ^
v |
mdirbtree
I'm not sure I'm happy with the current implementation, and eventually
it will need to be able to handle in-place repairs to the blocks it
sees, so this whole thing may need a rewrite.
But in the meantime, this passes the new clobber tests in test_alloc, so
it should be enough to prove the file implementation works. (which is
definitely is not fully tested yet, and some bugs had to be fixed for
the new tests in test_alloc to pass).
---
Speaking of test_alloc.
The inherent cyclic dependency between files/dirs/alloc makes it a bit
hard to know what order to test these bits of functionality in.
Originally I was testing alloc first, because it seems you need to be
confident in your block allocator before you can start testing
higher-level data structures.
But I've gone ahead and reversed this order, testing alloc after
files/dirs. This is because of an interesting observation that if alloc
is broken, you can always increase the test device's size to some absurd
number (-DDISK_SIZE=16777216, for example) to kick the can down the
road.
Testing in this order allows alloc to use more high-level APIs and
focus on corner cases where the allocator's behavior requires subtlety
to be correct (e.g. ENOSPC).
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 main improvement is moving the special inlined-file compaction logic
up into lfsr_mdir_compact__. We only need this logic for files stored in
mdirs, and thanks to its recursive nature, we weren't getting any
benefit from handling this at a lower level anyways.
This is a nice logical restructuring that probably saves a bit of code
cost in the end.
Another significant improvement is moving the staging copy of the
inlined tree's state up into the file struct itself. This solves the
problem of needed N copies of temporary inlined state when you have N
open files.
It also provides a central place to stage changes when compacting
inlined trees, which happens across several different places in the mdir
commit logic. Though some may see this as more a hack than a feature.
Also note-worthy, but minor: these changes required an additional
opened-mdir linked-list to know when the mdir is a file and may contain
an inlined tree.
- mbits -> mleaf_bits
- mlimit -> mleaf_limit
- mweight -> mleaf_weight
- lfsr_mridmask -> lfsr_midrmask
- lfsr_mbidmask -> lfsr_midbmask
This is a bit tricky to name, since we want to clarify it's not the
mtree limit and not the mdir's actual rbyd weight. But this also risks
confusing around the difference between mdirs/mleaves (mdirs are
mtree's leaves).
Taking advantage of the fact that these functions should never error,
changing the return type to lfsr_data_t allows all of the encoding
information to be passed around quite easily.
And, by giving each lfsr_data_from* function an LFSR_DATA_FROM* macro,
these functions can participate in our attr-list generating macros:
LFSR_ATTR(-1, MTREE, 0, FROMBTREE(lfs, mtree, mtree_buf))
Though one thing to watch out for is the borrowed buffer that stores the
actual data. This might welcome use-after-free bugs since it's not super
clear the buffer remains borrowed. Will need to watch out for this.
Adopted lfsr_rid/bid/mid/did_t where appropriate. This includes using
lfsr_rid_t for tag/rbyd weights. Although I am using lfsr_srid_t for
rbyd weights now, since it both captures the use of the sign bit and
reduces the number of casts a bit in the code.
I learned recently Zig has any-bit integers (e.g. uint31_t), and I'm
realizing how nice it would be to have those in this codebase.
Also tried to use lfs_size_t/lfs_off_t more correctly. In Linux/BSD,
only off_t is used for file-size-related operations and is usually much
larger than size_t. These were used interchangably in littlefs and their
original meaning kind of fell by the wayside. Getting their use right
will be important if littlefs ever supports different integer widths.
This only matters for developers, not users, but it still helps a lot to
get debug representations right.
Since the exact mid encoding depends on the block_size in an unintuitive
manner, it's tricky to render in a debug-friendly way that is useful
both with and without tools.
Previously, I avoided shifting the bid representation, since this would
be closer to the value in the device, but this hides the actual
structure of the mtree. Now the bid is shifted, showing the underlying
mtree/mdir structure, at the cost of needing to know the number of mbits
to encode the mid back into an integer.
So for example, on a device with 4KiB blocks, or 8 mbits:
mid=1
mid=258
mid=515
Becomes:
mid=0.1
mid=1.2
mid=2.3
This continues to make the mbits a more fundamental part of littlefs,
but that's probably just how that's going to be.
Christopher Haster