This effectively reverts 1f824a0:
- LFS3_T_COMPACTMETA -> LFS3_T_COMPACT
- gc_compactmeta_thresh -> gc_compact_thresh
And friends.
After using LFS3_T_COMPACTMETA for a bit, I think it just adds noise
without much value. Especially when next to LFS3_T_LOOKAHEAD,
LFS3_GC_PREERASE, LFS3_M_SYNC, etc.
It's interesting that we already have some very distinct verbs for this
sort of thing based on data type (compact => metadata, garbage-collect
=> disk, compress => data).
Our flag space is already really packed, and I'm not sure having these
as separate flags is meaningful or useful for users. They both indicate
to repopulate allocators, and most users probably won't care that there
are two subtly different allocators operating under the hood.
There's an argument that LOOKAHEAD not touching disk is a useful
distinction, but in practice you really only need LOOKAHEAD work when
mounted RDWR.
So, merged the behaviors of LOOKAHEAD + LOOKGBMAP such that
LFS3_*_LOOKAHEAD requests repopulation of all allocators based on
gc_lookahead_thresh and gc_lookgbmap_thresh.
In priority order (some notes below):
1. If max(lookahead, gbmap) < gc_lookahead_thresh => repop lookahead
2. If gbmap < gc_lookgbmap_thresh => repop gbmap
As a plus, this makes it easier to avoid LFS3_IFDEF_GBMAP mess.
---
It's interesting to note LFS3_*_LOOKAHEAD will still repopulate the
lookahead buffer when the gbmap is present, but only if this would gain
more knowledge than was is currently in the gbmap.
I considered disabling lookahead scans completely when we have a gbmap,
but repopulating the lookahead buffer is still useful if the gbmap is at
risk of exhaustion. This is what gc_lookahead_thresh is for anyways, and
users can set gc_lookahead_thresh=0 if they want to disable this
behavior.
Relatedly, lookahead scans are actually prioritized over gbmap scans
(when they would gain knowledge). In theory this minimizes gc latency,
as gbmap scans risk triggering a full lookahead scan when building the
new gbmap.
---
Code changes minimal:
code stack ctx
before: 35152 2136 660
after: 35152 (+0.0%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38076 2136 776
gbmap after: 38080 (+0.0%) 2136 (+0.0%) 776 (+0.0%)
Before, the gbmap allocator worked by feeding the lookahead allocator,
so all allocation requests went through the lookahead buffer:
alloc ---> lookahead ---> gbmap
This worked, and was easy to strap on to the existing system, but
limited what we could cache to what fits in our lookahead buffer. This
doesn't have a big effect on runtime analysis, since fragmentation is
always a concern, but it does mean we're not taking advantage of the
gbmap range representation and accessing disk more than we need to.
It also limits in-use range skipping to a lookahead buffer at a time,
which is problematic as the whole reason for the gbmap is to make the
lookahead buffer mostly irrelevant.
On top of the range issues, this design makes it difficult to add
preerase info, which is coming up on the TODO list.
---
To fix this, I reorganized the two allocators to run in parallel, with
the gbmap being queried first before falling back to the lookahead
buffer:
alloc -+-> gbmap
'-> lookahead
Instead of relying on the lookahead buffer, the gbmap now stores one
range in the gbmap.free field, using the sign to indicate if it's free
vs in-use (not the best name, but oh well).
This adds a word of storage to the gbmap, but as a tradeoff we can track
a full region in RAM and avoid repeated gbmap lookups.
The lookahead buffer can still be populated by gc work, which may be
useful if the gbmap is exhausted, but will also be cleared during gbmap
allocations to avoid out-of-date state.
---
While reworking this, I also tweaked a number of other allocator things:
- Adjusted lookahead.window to point to next block candidate
(lookahead.window and gbmap.window should now always match)
- Renamed lfs3_alloc_zerogbmap -> lfs3_gbmap_zero
- Moved some alloc functions around
Code cost was mostly unaffected. Added an extra word to gbmap's ctx, but
as a tradeoff saved a chunk of stack by avoiding nested allocators:
code stack ctx
before: 35160 2136 660
after: 35152 (-0.0%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38020 2152 772
gbmap after: 38076 (+0.1%) 2136 (-0.7%) 776 (+0.5%)
See previous commit for motivation.
I can't think of how you could easily find this information from the
gbmap during/after mount, short of a O(d log_b d) scan through the
gbmap. Maybe useful, but probably not a great tradeoff for what is only
debug/diagnostic information.
So reverting, but maybe interesting to explore in the future with other
debug APIs.
Code changes:
code stack ctx
before: 35220 2136 660
after: 35160 (-0.2%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38116 2152 776
gbmap after: 38020 (-0.3%) 2152 (+0.0%) 772 (-0.5%)
The idea here was that we could provide best-effort known in-use/free
block info (and eventually pre-erased and bad block info) as a cheaper
alternative to lfs3_fs_usage. It's probably still not what users expect
from lfs3_fs_stat, but may be useful as debug/diagnostic info:
- fsinfo.known_free - Number of known free blocks
- fsinfo.known_inuse - Number of known in-use blocks
- fsinfo.known_preerased* - Number of pre-erased blocks
- fsinfo.known_bad* - Number of bad blocks
- fsinfo.block_count-(all of the above) - Number of unknown blocks
But while known_free/known_inuse is easy enough to find from the
lookahead buffer, it's surprisingly tricky from the gbmap. The best
option I can think of requires scanning the gbmap in O(d log_b d) either
(1) during mount, (2) during mkconsistent, or (3) during lookahead
scans. And that much extra work for debug/diagnostic info seems like a
poor tradeoff.
Note, though, that after scanning once, in theory the info would be
~free to maintain during gbmap rebuilds.
Will revert.
Code changes:
code stack ctx
before: 35160 2136 660
after: 35220 (+0.2%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38020 2152 772
gbmap after: 38116 (+0.3%) 2152 (+0.0%) 776 (+0.5%)
In theory, lfs3_size_t should be used for in-block sizes (though this is
also mixed up with in-device sizes?), lfs3_off_t for file sizes, and
lfs3_block_t for block counts (I don't think lfs3_off_t/lfs3_block_t
will ever differ, but the notation is helpful).
Though I've not done a great job at keeping these types organized...
Changed:
- cfg.block_count: lfs3_size_t -> lfs3_block_t
- cfg.file_limit: lfs3_size_t -> lfs3_off_t
- fsinfo.block_count: lfs3_size_t -> lfs3_block_t
- fsinfo.file_limit: lfs3_size_t -> lfs3_off_t
- lfs3_fs_usage: lfs3_ssize_t -> lfs3_sblock_t
- geometry.block_size: lfs3_size_t -> lfs3_off_t
- geometry.block_count: lfs3_size_t -> lfs3_block_t
- and some internals
No code changes.
- Adopted -2,-2 for unbounded lfs3_mdir_commit___ ranges that include
gstate.
Why not? We treat any negative upper bound as unbounded and it's a bit
easier to read.
- Prefer <=-2 when checking for rid bounds that include gstate.
- Prefer <=-2 when checking for attached rattr weights.
- Cleaned up a couple outdated comments.
No code changes:
code stack ctx
before: 35160 2136 660
after: 35160 (+0.0%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38020 2152 772
gbmap after: 38020 (+0.0%) 2152 (+0.0%) 772 (+0.0%)
May rerevert this in the future, but I'm on the fence.
It's true this only saves a small amount of code, but in theory it also
reduces stack consumption in name-related functions. Currently this
doesn't affect the stack hot-path, which is a bit surprising as this
includes lfs3_set, but it may in the future.
The arguments against this optimization are also a bit weak:
- Non-null-terminated strings - We probably shouldn't optimize for a
theoretical future feature. If anything, we want to optimize in the
opposite direction to best measure the theoretical code cost.
- Precomputing strlen early - While this is generally a good idea, our
rattrs benefit greatly from compact encodings, as rattrs sitting on
the stack are one of the bigger contributors to our stack hot-path.
So for now I'm unreverting to see how long this optimization makes
sense, but could see this being rereverted in the future.
At the very least we probably want to keep the test changes to make
future testing easier.
---
Saves a bit of code:
code stack ctx
before: 35188 2136 660
after: 35160 (-0.1%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38048 2152 772
gbmap after: 38020 (-0.1%) 2152 (+0.0%) 772 (+0.0%)
As much as I don't want to admit it, our 3-word lfs3_data_t struct is
just too large to be treated as pass-by-value with today's compilers.
It's a real shame, because I don't think there's a great technical
reason, just that compiler's pass-by-value optimizations generally stop
after 2 words.
If we could expect 16-bit block sizes (off and size), we could fit in
2 words, but this is already challenged by today's NAND chips
(bs>=128KiB).
---
So, as a compromise, this stops treating lfs3_data_t as pass-by-value,
with the exception of the lfs3_data_from* functions that still return
lfs3_data_t directly.
So instead of:
lfs3_data_t data = lfs3_data_fromecksum(&ecksum, buffer);
data = lfs3_data_slice(data, 8, -1);
return lfs3_data_size(data);
Most operations take lfs3_data_t by pointer:
lfs3_data_t data = lfs3_data_fromecksum(&ecksum, buffer);
lfs3_data_slice(&data, 8, -1);
return lfs3_data_size(&data);
One of the main consequences is there are now several ways to slice data
(internally these all redirect to lfs3_data_slice), and LFS3_DATA_SLICE
will likely see more use since we need temporary allocations to pass the
data slice by address:
- lfs3_data_slice(data, a, b) - Slices the data in place
- lfs3_data_fromslice(data, a, b) - Returns a new data slice
- LFS3_DATA_SLICE(data, a, b) - Creates a new compound-literal slice
---
As a pragmatic compromise, this saves a nice chunk of both code and
stack:
code stack ctx
before: 35316 2176 660
after: 35188 (-0.4%) 2136 (-1.8%) 660 (+0.0%)
code stack ctx
gbmap before: 38172 2192 772
gbmap after: 38048 (-0.3%) 2152 (-1.8%) 772 (+0.0%)
This was originally dropped because it's not strictly necessary,
little-leb128s (28-bits) can always be encoded with the default leb128
encoder (31-bits). But it is useful if only for the assert.
Note this matches lfs3_data_readlleb128, which was never dropped, and is
useful for decreasing decoder DSIZEs.
Maybe it makes sense to drop both of these in the future, especially if
we start running into from-field pressure. But for now, this assert is
useful for ensuring disk compatibility with little 4-byte leb128s.
---
Surprisingly no code cost, at least by default (code alignment?). Though
it did add 4 bytes to the gbmap build (so yes, probably code alignment):
code stack ctx
before: 35316 2176 660
after: 35316 (+0.0%) 2176 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38168 2192 772
gbmap after: 38172 (+0.0%) 2192 (+0.0%) 772 (+0.0%)
See previous commit for why.
The merged commits surprisingly cost more than separate commit
functions. I guess because the compiler is smart enough to deduplicate
the two logic paths here:
code stack ctx
before: 35324 2176 660
after: 35316 (-0.0%) 2176 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38172 2192 772
gbmap after: 38168 (-0.0%) 2192 (+0.0%) 772 (+0.0%)
The good news is this is a win for readability, I think the separate
conditions are easier to understand than a merged commit muddied with a
bunch of lfs3->mtree.r.weight == 0 checks.
The idea here was to merge mtree split commits to try to minimize
redundant logic that only differs in whether or not we need to create
the initial mtree weight.
Surprisingly, this backfired, adding more code than it saved. I guess I
underestimated how effective the compiler is at deduplicating these two
paths of logic:
code stack ctx
before: 35316 2176 660
after: 35324 (+0.0%) 2176 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38168 2192 772
gbmap after: 38172 (+0.0%) 2192 (+0.0%) 772 (+0.0%)
I don't think there was anything inherently wrong with this idea, but:
- The code savings (28 bytes) was surprisingly small.
- Expecting lfs3_path_namelen may be a headache for future
non-null-terminated string support.
- Even if you don't care about non-null-terminated strings, precomputing
strlen as early as possible is a good idea to minimize repeated strlen
scans.
Reverting adds a bit of code:
code stack ctx
before: 35288 2176 660
after: 35316 (+0.1%) 2176 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38140 2192 772
gbmap after: 38168 (+0.1%) 2192 (+0.0%) 772 (+0.0%)
I was poking around at possibly inlining small (<=255) name lens in
lfs3_rattr_t, but realized all LFS3_FROM_NAME rattrs in our system
already use the lfs3_path_namelen pattern (terminates in either
'\0' or '/').
Well, except for our tests, but who cares about those.
Adopting lfs3_path_namelen in LFS3_FROM_NAME saves a bit of code:
code stack ctx
before: 35316 2176 660
after: 35288 (-0.1%) 2176 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38168 2192 772
gbmap after: 38140 (-0.1%) 2192 (+0.0%) 772 (+0.0%)
Here's one interesting use-case for the single-recurse LFS3_tag_RATTRS:
Avoiding a copy of the name creation rattrs in lfs3_file_sync_.
There is a concern with nesting LFS3_tag_RATTRS in that it risks
conflicts across layers, but currently this is ok as long as
high-level LFS3_tag_RATTRS stick to non-negative mids (the mtree split
commit in lfs3_mdir_commit_ only needs to recurse for mroot rattrs).
Saves a bit of code:
code stack ctx
before: 35320 2176 660
after: 35316 (-0.0%) 2176 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38172 2192 772
gbmap after: 38168 (-0.0%) 2192 (+0.0%) 772 (+0.0%)
This originally started as an attempt to drop LFS3_tag_TAIL entirely,
but that didn't really go anywhere. Any attempt to work around the
double rattr-lists during mtree splits results in more mess than this
magic rattr.
But I did notice we only need to support "simple" rattrs during mtree
splits, which means we can just call lfs3_rbyd_appendrattrs to handle
these.
Maybe this will be problematic if we ever want to deduplicate
lfs3_mdir_commit___ and lfs3_rbyd_appendrattrs, but I don't see that
happening because of the different concerns (mdir-specific rattrs):
function code stack ctx
lfs3_mdir_commit___ 1052 744 396
lfs3_rbyd_appendrattrs 142 584 388
The benefit of a single-recurse LFS3_tag_RATTRS:
- Simplifies lfs3_mdir_commit__, no more awkward loop recursion.
- May have other use cases for nesting simple rattrs?
Adds a bit of code:
code stack ctx
before: 35316 2176 660
after: 35320 (+0.0%) 2176 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38148 2192 772
gbmap after: 38172 (+0.1%) 2192 (+0.0%) 772 (+0.0%)
LFS3_tag_RATTRS, now LFS3_tag_TAIL, is a bit funny in that it only
supports tail-recursive rattrs. The whole point of littlefs is
bounded-RAM after all. And if we know LFS3_tag_TAIL will terminate an
rattr-list, why bother with an additional LFS3_tag_NULL?
Like LFS3_RATTR_NULL, LFS3_RATTR_TAIL sets length=0 to indicate the end
of the rattr-lists.
Saves a bit of code:
code stack ctx
before: 35324 2176 660
after: 35316 (-0.0%) 2176 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38156 2192 772
gbmap after: 38148 (-0.0%) 2192 (+0.0%) 772 (+0.0%)
It's funny to see what originally started as a simple list of rbyd attrs
slowly morph into a full isa. But it makes sense. What we really want is
an abstract description of operations that can be played and replayed as
necessary to atomically update the mtree.
Using a fixed lfs3_rattr_t struct to represent this in C is easy, and
avoids strict-aliasing issues, but ultimately limited when it comes to
the wide-range of data we want to attach to attributes.
Unlike a computer's isa, we want to be able to include full 12-24 byte
branch pointers directly in the instruction!
---
So here's a full variable-length isa organized by words (max(uintptr_t,
uint32_t)).
The first 32-bit word extends the 16-bit tag with an extra 16-bits of
control information:
wwll llff ffcc cccc tttt tttt tttt tttt
^'-.-''-.-''--.--' : :
'--|----|-----|----:-----------------:-- compressed weight
:: '----|-----|----:-----------------:-- total len
:: '-----|----:-----------------:-- from encoder
:: '----:-----------------:-- optional count
:: rgmm kkkk -kkk kkkk
11 => w=-1 ^^ ^ '-.' '---.---'
00 => w=0 '|-|---|------|------ rm bit
01 => w=+1 '-|---|------|------ grow bit
10 => w=attached '---|------|------ mask bits
'------|------ tag suptype
'------ tag subtype
The 4-bit length field always encodes the full length of the
instruction, including the instruction itself and optional weight. The
4-bit from + 6-bit count fields operate independently and tell
lfs3_rbyd_appendrattr_ how to actually encode the data related to the
instruction.
To work around strict-aliasing issues, complex structs are expected to
be broken down into words and reconstructed in lfs3_rbyd_appendrattr_.
Most of our structs are organized into words anyways. For example:
// new child
*r++ = LFS3_RATTR(5, LFS3_TAG_BRANCH, -2, LFS3_FROM_BRANCH);
*r++ = LFS3_RATTR_WEIGHT(+child_->weight);
*r++ = LFS3_RATTR_ARG(child_->blocks[0]);
*r++ = LFS3_RATTR_ARG(child_->trunk);
*r++ = LFS3_RATTR_ARG(child_->cksum);
This also changes rattr-lists to be null-terminated, which makes a bit
more sense in a variable-length isa:
*r++ = LFS3_RATTR_NULL; // all zeros, including length
One concern with null-terminated rattr-lists is how easy it is to
forget the null-terminator, but an assert that all non-null rattrs have
non-zero length seemed to catch the many many mistakes during adoption.
Alternatively, separate LFS3_FROM_NULL/LFS3_FROM_NIL from fields could
be used if encoding space gets tight.
I'm also quite happy with the 2-bit weight feild, which allows omitting
the optional weight word for -1,0,+1 weights. These should cover at
least all mdir operations.
Note the exact encoding of the rattr fields is less of a concern than
the tag fields, as it doesn't reside on-disk can be changed on whim.
---
Saves a nice chunk of code and stack:
code stack ctx
before: 35920 2280 660
after: 35324 (-1.7%) 2176 (-4.6%) 660 (+0.0%)
code stack ctx
gbmap before: 38812 2296 772
gbmap after: 38156 (-1.7%) 2192 (-4.5%) 772 (+0.0%)
The stack savings are obvious, but the code savings a bit less so. A
variable length isa _is_ more complicated, but by limiting most encoding
decisions to compile-time (2-bit weights vs 32-bit weights for example),
the savings from fewer word manipulations on the stack wins.
Now that LFS3_TAG_INTERNAL uses the 0x00tt prefix, this bit mask no
longer works.
Fortunately LFS3_TAG_INTERNAL is really only used in LFS3_ASSERTs, so a
less efficient test has no effect on code cost.
No code changes.
Though note most high-level calls (lfs3_file_close, lfs3_dir_close,
etc), still include an assertion at a higher-level.
Why make the internal APIs harder to use than they need to be? As a
plus this drops the need for a separate bool-returning
lfs3_handle_close_.
Saves a bit of code:
code stack ctx
before: 35924 2280 660
after: 35912 (-0.0%) 2280 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38812 2296 772
gbmap after: 38800 (-0.0%) 2296 (+0.0%) 772 (+0.0%)
Unintentionally arriving at the infamous "fsck" name is a bit funny.
But it's probably something we don't want to conflict with if we can
help it, on the off chance we want a sort of lfs3_fsck function in the
future. (This is all hypothetical, but lfs3_fsck may expect an unmounted
filesystem, and have a much larger scope than lfs3_fs_ck. Though typing
this out now I'm realizing how confusing that might be...)
Since lfs3_file_ck and lfs3_fs_ck share a subset of flags, it's not
_entirely_ unreasonable for lfs3_file_ck and lfs3_fs_ck to share the
same namespace.
There's a risk of confusing users around what flags lfs3_file_ck
accepts, but we have asserts, and said flags (LFS3_CK_MKCONSISTENT,
LFS3_CK_LOOKAHEAD, etc) just don't really make sense in lfs3_file_ck:
fs file
y LFS3_CK_MKCONSISTENT 0x00000800 Make the filesystem consistent
y LFS3_CK_LOOKAHEAD 0x00001000 Repopulate lookahead buffer
y LFS3_CK_LOOKGBMAP 0x00002000 Repopulate the gbmap
y LFS3_CK_PREERASE* 0x00004000 Pre-erase unused blocks
y LFS3_CK_COMPACTMETA 0x00008000 Compact metadata logs
y y LFS3_CK_CKMETA 0x00010000 Check metadata checksums
y y LFS3_CK_CKDATA 0x00020000 Check metadata + data checksums
y y LFS3_CK_REPAIRMETA* 0x00040000 Repair data blocks
y y LFS3_CK_REPAIRDATA* 0x00080000 Repair metadata + data blocks
* Planned
Another option would be to document that lfs3_fs_ck accepts both
LFS3_CK_* _and_ LFS3_GC_* flags, but I worry that would be more
confusing. It would also lock us into supporting all LFs3_GC_* flags in
lfs3_fs_ck, which may not always be the case.
Though this is an argument for doing away with the whole
LFS3_M/F/CK/GC/I_* duplication... (tbh another reason for this is to
reduce the number of namespaces by at least one).
No code changes.
TLDR: Replaced lfs3_file_ckmeta/ckdata and lfs3_fs_ckmeta/ckdata with
flag based ck functions:
- lfs3_file_ckmeta -> lfs3_file_ck + LFS3_CK_CKMETA
- lfs3_file_ckdata -> lfs3_file_ck + LFS3_CK_CKDATA
- lfs3_fs_ckmeta -> lfs3_fs_ck + LFS3_FSCK_CKMETA
- lfs3_fs_ckdata -> lfs3_fs_ck + LFS3_FSCK_CKDATA
Note lfs3_fs_ck is equivalent to lfs3_fs_gc, but:
1. Performs the work in one call (equivalent to littlefs2's lfs2_fs_gc)
2. Takes flags at call time (like lfs3_mount) instead of cfg time (like
lfs3_fs_gc)
3. Avoids the constant RAM necessary to track incremental GC state
---
Motivation:
I've been thinking: It's a bit weird that users are able to one-shot
janitorial work in lfs3_mount, but there's no equivalent function after
the filesystem is mounted.
Originally this is what lfs3_fs_gc was for, but after adding support for
incremental GC, it made sense to hide lfs3_fs_gc behind the opt-in
LFS3_GC ifdef due to the extra (ironically non-gc-able) state.
In theory lfs3_trv_t fills a bit of the gap, but, without the internal
i_flag handling and traversal restarts, it's a bit hard to use. And
basically requires duplicating said log, which we need anyways for
lfs3_mount!
So ideally we'd add an explicit one-shot GC function, but now lfs3_fs_gc
is taken.
While thinking about alternative names, I realized we can just call this
lfs3_fs_ck and completely replace lfs3_fs_ckmeta/ckdata.
This has some extra benefits:
- Avoids an explosion of ckmeta/ckdata/repairmeta/repairdata functions
- Discourages redundant traversals that could accomplish more work
- Makes it less confusing that ckdata implies ckmeta
---
I also tweaked lfs3_file_ck to match, but note that lfs3_file_ck is
internally very different from lfs3_fs_ck. For one, lfs3_file_ck only
supports "actual" check flags (LFS3_CK_*) vs all gc flags (LFS3_FSCK_*):
lfs3_file_ck:
LFS3_CK_CKMETA 0x00010000 Check metadata checksums
LFS3_CK_CKDATA 0x00020000 Check metadata + data checksums
LFS3_CK_REPAIRMETA* 0x00040000 Repair metadata blocks
LFS3_CK_REPAIRDATA* 0x00080000 Repair metadata + data blocks
* Planned
lfs3_fs_ck:
LFS3_FSCK_MKCONSISTENT 0x00000800 Make the filesystem consistent
LFS3_FSCK_LOOKAHEAD 0x00001000 Repopulate lookahead buffer
LFS3_FSCK_LOOKGBMAP 0x00002000 Repopulate the gbmap
LFS3_FSCK_PREERASE* 0x00004000 Pre-erase unused blocks
LFS3_FSCK_COMPACTMETA 0x00008000 Compact metadata logs
LFS3_FSCK_CKMETA 0x00010000 Check metadata checksums
LFS3_FSCK_CKDATA 0x00020000 Check metadata + data checksums
LFS3_FSCK_REPAIRMETA* 0x00040000 Repair metadata blocks
LFS3_FSCK_REPAIRDATA* 0x00080000 Repair metadata + data blocks
* Planned
As a plus, this also saves a bit of code:
code stack ctx
before: 35968 2280 660
after: 35924 (-0.1%) 2280 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38828 2296 772
gbmap after: 38812 (-0.0%) 2296 (+0.0%) 772 (+0.0%)
This more closely matches behavior of functions like mkdir and remove,
even though mkgbmap/rmgbmap operate on a special object and not files.
Besides, returning an error is more useful as users are always free to
ignore said error.
Adds what appears to be one literal to mkgbmap (curiously not rmgbmap?
snuck into alignment?):
code stack ctx
before: 35968 2280 660
after: 35968 (+0.0%) 2280 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38824 2296 772
gbmap after: 38828 (+0.0%) 2296 (+0.0%) 772 (+0.0%)
I can't think of a reason this should be uint8_t. Bumping it up to
uint32_t matches the type used for other flags (even though whence is
arguably not flags in a strict sense).
No code changes.
This includes the mask/rm/grow bits:
- LFS3_tag_RM
- LFS3_tag_GROW
- LFS3_tag_MASK0/2/8/12
Our in-device only handle types:
- LFS3_tag_ORPHAN
- LFS3_tag_TRV
- LFS3_tag_UNKNOWN
And in-device only tags with special behavior:
- LFS3_tag_INTERNAL
- LFS3_tag_RATTRS
- LFS3_tag_SHRUBCOMMIT
- LFS3_tag_GRMPUSH
- LFS3_tag_MOVE
- LFS3_tag_ATTRS
Usually I'm not a big fan of case-sensitive naming patterns, but this
has been useful for self-documenting what compat flags are in-device
only. Might as well extend the idea to our tag definitions.
Having on-disk definitions in one place is useful for referencing them
later, even if they aren't relevant for most API users.
.h files in C are already forced to expose a bunch of internal details
anyways, in order to provide struct size/alignment. Might as well
include on-disk information that would have even bigger consequences if
it changed.
Moved:
- Compat flag definitions
- Tag definitions
- DSIZEs and relevant encoding comments - Note some of these were
already required to define lfs3_t
Other than moving things around to make space for planned features, this
also adopts the idea of allowing compat flags to be ored into a single
32-bit integer, at least in the short-term.
Note though that these are still stored in separate wcompat/rcompat
tags, to make compat tests easier, and we may introduce conflicting
flags in the future if we run out of 32-bits. This is just an indulgence
to potentially make tooling/debugging easier until that happens.
Rcompat flags:
RCOMPAT_NONSTANDARD+
0x00000001 ---- ---- ---- ---- ---- ---- ---- ---1
RCOMPAT_WRONLY+ 0x00000004 ---- ---- ---- ---- ---- ---- ---- -1--
RCOMPAT_MMOSS 0x00000010 ---- ---- ---- ---- ---- ---- ---1 ----
RCOMPAT_MSPROUT+ 0x00000020 ---- ---- ---- ---- ---- ---- --1- ----
RCOMPAT_MSHRUB+ 0x00000040 ---- ---- ---- ---- ---- ---- -1-- ----
RCOMPAT_MTREE 0x00000080 ---- ---- ---- ---- ---- ---- 1--- ----
RCOMPAT_BMOSS+ 0x00000100 ---- ---- ---- ---- ---- ---1 ---- ----
RCOMPAT_BSPROUT+ 0x00000200 ---- ---- ---- ---- ---- --1- ---- ----
RCOMPAT_BSHRUB 0x00000400 ---- ---- ---- ---- ---- -1-- ---- ----
RCOMPAT_BTREE 0x00000800 ---- ---- ---- ---- ---- 1--- ---- ----
RCOMPAT_MDIRR1* 0x00001000 ---- ---- ---- ---- ---1 ---- ---- ----
RCOMPAT_MDIRR2* 0x00002000 ---- ---- ---- ---- --1- ---- ---- ----
RCOMPAT_MDIRR3* 0x00003000 ---- ---- ---- ---- --11 ---- ---- ----
RCOMPAT_BTREER1* 0x00004000 ---- ---- ---- ---- -1-- ---- ---- ----
RCOMPAT_BTREER2* 0x00008000 ---- ---- ---- ---- 1--- ---- ---- ----
RCOMPAT_BTREER3* 0x0000c000 ---- ---- ---- ---- 11-- ---- ---- ----
RCOMPAT_GRM 0x00010000 ---- ---- ---- ---1 ---- ---- ---- ----
RCOMPAT_GMV? 0x00020000 ---- ---- ---- --1- ---- ---- ---- ----
RCOMPAT_GDDTREE* 0x00100000 ---- ---- ---1 ---- ---- ---- ---- ----
RCOMPAT_GPTREE* 0x00200000 ---- ---- --1- ---- ---- ---- ---- ----
RCOMPAT_DATAR1* 0x00400000 ---- ---- -1-- ---- ---- ---- ---- ----
RCOMPAT_DATAR2* 0x00800000 ---- ---- 1--- ---- ---- ---- ---- ----
RCOMPAT_DATAR3* 0x00c00000 ---- ---- 11-- ---- ---- ---- ---- ----
rcompat_OVERFLOW+ 0x80000000 1--- ---- ---- ---- ---- ---- ---- ----
* Planned
+ Reserved
? Hypothetical
Wcompat flags:
WCOMPAT_NONSTANDARD+
0x00000001 ---- ---- ---- ---- ---- ---- ---- ---1
WCOMPAT_RDONLY+ 0x00000002 ---- ---- ---- ---- ---- ---- ---- --1-
WCOMPAT_GCKSUM 0x00040000 ---- ---- ---- -1-- ---- ---- ---- ----
WCOMPAT_GBMAP 0x00080000 ---- ---- ---- 1--- ---- ---- ---- ----
WCOMPAT_DIR 0x01000000 ---- ---1 ---- ---- ---- ---- ---- ----
WCOMPAT_SYMLINK? 0x02000000 ---- --1- ---- ---- ---- ---- ---- ----
WCOMPAT_SNAPSHOT? 0x04000000 ---- -1-- ---- ---- ---- ---- ---- ----
wcompat_OVERFLOW+ 0x80000000 1--- ---- ---- ---- ---- ---- ---- ----
+ Reserved
? Hypothetical
Ocompat flags:
OCOMPAT_NONSTANDARD+
0x00000001 ---- ---- ---- ---- ---- ---- ---- ---1
ocompat_OVERFLOW+ 0x80000000 1--- ---- ---- ---- ---- ---- ---- ----
+ Reserved
Other notes:
- M* and B* struct flags were reordered to match META -> DATA order
elsewhere. This no longer matches the tag ordering, but there's an
argument the B* tags apply more generally (all btrees) than the B*
compat flag (only file btrees).
- MDIR/BTREE/DATA redund flags were moved near relevant flags, rather
than sticking them in the higher-order bits as we are planning to do
in the M_*/F_* flags. The compat flags already won't match because of
the mdir/btree split (which is IMO too much detail to include in
M_*/F_* flags, but hard to argue against in the compat flags), and
this keeps the highest bit free for OVERFLOW, which is useful
internally.
- Moving DIR to the current-highest bit makes it easy to add 6 more file
types (7 if you ignore OVERFLOW), before things start getting cramped.
No code changes.
Yeah, after using these for a bit, the RE* names were not great.
Trying LOOK* now, as an alternative that hopefully still implies the
similar behavior without needing an additional prefix for LOOKAHEAD:
- LFS3_*_RELOOKAHEAD -> LFS3_*_LOOKAHEAD
- LFS3_*_REGBMAP -> LFS3_*_LOOKGBMAP
- cfg.regbmap_thresh -> cfg.lookgbmap_thresh
- cfg.gc_relookahead_thresh -> cfg.gc_lookahead_thresh
- cfg.gc_regbmap_thresh -> cfg.gc_lookgbmap_thresh
I mean, why not? These redirect to the same internal lfs3_fs_gc_
function anyways. Might as well keep things consistent.
Added:
LFS3_F_MKCONSISTENT 0x00000800 Make the filesystem consistent
LFS3_F_RELOOKAHEAD 0x00001000 Repopulate lookahead buffer
LFS3_F_MKCONSISTENT is guaranteed to be a noop, but LFS3_F_RELOOKAHEAD
forces a filesystem traversal, which may have some niche use case.
No code changes.
These are unlikely to make much progress, but that doesn't seem like a
great reason to disallow these flags in lfs3_format:
LFS3_F_REGBMAP 0x00002000 Repopulate the gbmap
LFS3_F_COMPACTMETA 0x00008000 Compact metadata logs
These are actually guaranteed to do _no_ work when formatting _without_
the gbmap, but with the gbmap it's less clear. Looking forward to the
planned ckfactory feature, these may be useful for cleaning up any rbyd
commits created as a part of building the initial gbmap.
---
Also tweaked the formatting for LFS3_F_* flags a bit, including making
all ifdefs explicit (mainly ifdef LFS3_RDONLY). Mixed ifdefs are a real
pain to read.
No code changes.
LFS3_REVDBG introduced a lot of overhead for something I'm not sure
anyone will actually use (I have enough tooling that the state of an
rbyd is rarely a mystery, see dbgbmap.py). That, and we're running out
of flags!
So this reduces LFS3_REVDBG to just store one of "himb" in the first
(lowest) byte of the revision count; information that is easily
available:
vvvv---- -------- -------- --------
vvvvrrrr rrrrrr-- -------- --------
vvvvrrrr rrrrrrnn nnnnnnnn nnnnnnnn
vvvvrrrr rrrrrrnn nnnnnnnn dddddddd
'-.''----.----''----.- - - '---.--'
'------|----------|----------|---- 4-bit relocation revision
'----------|----------|---- recycle-bits recycle counter
'----------|---- pseudorandom noise (if revnoise)
'---- h, i, m, or b (if revdbg)
-11-1--- - h = mroot anchor
-11-1--1 - i = mroot
-11-11-1 - m = mdir
-11---1- - b = btree node
Some other notes:
- Enabled LFS3_REVDBG and LFS3_REVNOISE to work together, now that
LFS3_REVDBG doesn't consume all unused rev bits.
Note that LFS3_REVDBG has priority over LFS3_REVNOISE, but _not_
recycle-bits, etc. Otherwise problems would happen for recycle-bits
>2^20 (though do we care?).
- Fixed an issue where using the gcksum as a noise source results in
noise=0 when there is only an mroot. This is due to how we xor out
the current mdir cksum during an mdir commit.
Fixed by using gcksum_p instead of gcksum.
- Added missing LFS3_I_REVDBG/REVNOISE flags in the tests, so now you
can actually run the tests with LFS3_REVDBG/REVNOISE (this probably
just fell out-of-date at some point).
---
Curiously, despite LFS3_REVDBG/REVNOISE being disabled by default, this
did save some code. I'm guessing the non-tail-call mtree/gbmap commit
functions prevented some level of inlining?:
code stack ctx
before: 35964 2280 660
after: 35964 (+0.0%) 2280 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38940 2296 772
gbmap after: 38828 (-0.3%) 2296 (+0.0%) 772 (+0.0%)
Now that we use LFS3_ERR_BUSY for traversals, we no longer have an
excuse for not returning LFS3_ERR_BUSY on root-related errors:
- lfs3_remove(&lfs3, "/") => LFS3_ERR_BUSY
- lfs3_rename(&lfs3, "/", *) => LFS3_ERR_BUSY
- lfs3_rename(&lfs3, *, "/") => LFS3_ERR_BUSY
This better aligns with POSIX. Arguably we should have defined
LFS3_ERR_BUSY for this case anyways, it's not like additional error
codes cost much.
No code changes.
With the relaxation of traversal behavior under mutation, I think it
makes sense to bring back LFS3_T_EXCL. If only to allow traversals to
gaurantee termination under mutation. Now that traversals no longer
guarantee forward progress, it's possible to get stuck looping
indefinitely if the filesystem is constantly being mutated.
Non-excl traversals are probably still useful for GC work and debugging
threads, but LFS3_T_EXCL now allows traversals to terminate immediately
with LFS3_ERR_BUSY at the first sign of unrelated filesystem mutation:
LFS3_T_EXCL 0x00000008 Error if filesystem modified
Internally, we already track unrelated mutation to avoid corrupt state
(LFS3_t_DIRTY), so this is a very low-cost feature:
code stack ctx
before: 35944 2280 660
after: 35964 (+0.1%) 2280 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38916 2296 772
gbmap after: 38940 (+0.1%) 2296 (+0.0%) 772 (+0.0%)
code stack ctx
gc before: 36016 2280 768
gc after: 36036 (+0.1%) 2280 (+0.0%) 768 (+0.0%)
Now that lfs3_mtree_traverse_ uses a sort of state matrix,
lfs3_rbyd_appendrattr_ is the only function still relying on a big
switch-case statement. Replacing it with a series of if-else statements
leaves the codebase switch-case free (ignoring test/bench runners, etc).
Switch-case statements are extremely error prone in C, with the shared
scope, implicit fallthrough, etc. And, with today's compilers, the
result still ends up the same, so switch-case statements offer no
benefit except maybe a more enjoyable syntax for masochists.
Avoiding switch-case statements in code where we care about correctness
is probably a good idea.
No code changes
Mostly for consistency with mtrv.b and gbmap.b, but also (1) this
hopefully reduces confusion around the fact that these can refer to both
bshrubs and btrees, and (2) saves a bit of typing with the messy struct
namespaces forced by C's strict aliasing.
Now that we no longer stage bshrubs in lfs3_trv_ts, bshrubs are limited
to LFS3_TYPE_REG handles. I'm not sure lfs3_o_isbshrub adds anything of
value in this case, so dropping.
I was considering dropping lfs3_bshrub_t completely, since we can always
expect these to be lfs3_file_ts, but decided against it for now as local
lfs3_bshrub_ts may be useful for bshrub commits during block eviction/
repair. Still need to see what that looks like.
Note the slight incongruity of lfs3_bshrub_t vs LFS3_TYPE_REG matches
the incongruity of lfs3_mgc_t and LFS3_type_TRV.
No code changes
This solves the previous gc-needs-block-queue-so-we-can-clobber-block-
queue issue by adding an additional LFS3_t_STALE flag to indicate when
any block queues would be invalid.
So instead of clearing block queues in lfs3_alloc_ckpoint, we just set
LFS3_t_STALE, and any lfs3_trv_ts can clear their block queues in
lfs3_trv_read. This allows lfs3_mgc_ts to be allocated without a block
queue when doing any LFS3_M_*/LFS3_F_*/LFS3_GC_* work.
LFS3_t_STALE is set at the same time as LFS3_t_CKPOINT and LFS3_t_DIRTY,
but we need a separate bit so lfs3_trv_read can clear the flag after
flushing without losing ckpoint/dirty information.
---
Unfortunately, none of the stack-allocated lfs3_mgc_ts are on the stack
hot-path, so we don't immediate savings. But note the 2-words saved in
ctx when compiling in LFS3_GC mode:
code stack ctx
before: 35940 2280 660
after: 35944 (+0.0%) 2280 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38916 2296 772
gbmap after: 38916 (+0.0%) 2296 (+0.0%) 772 (+0.0%)
code stack ctx
gc before: 36012 2280 776
gc after: 36016 (+0.0%) 2280 (+0.0%) 768 (-1.0%)
The main idea here is to drop the flag-encoded tstate state machine, and
replace it with a matrix controlled by special mid + bid values:
-- mid ->
-5 -4 -3 -2 >=-1
bid -2 x x x --> mdir
v >=-1 x gbm gbm x --> bshrub/btree
'----|----|----|----|----> mroot anchor
'----|----|----|----> mroot chain + mtree
'----|----|----> gbmap (in-ram gbmap)
'----|----> gbmap_p (on-disk gbmap)
'----> file bshrubs/btrees
This was motivated by the observation that everything in our filesystem
can be modeled as mdir + bshrub/btree tuples, as long as some states are
noops. And we can cleanly encode these tuples in the unused negative
mid + bid ranges without needing an explicit state machine.
Well, that and the previous tstate state machine approach being an ugly
pile of switch cases and messy logic.
Note though that some mids may need to traverse multiple mdirs/bshrub/
btrees:
- The mroot chain + mtree (mid=-4) needs to traverse all mroots in the
mroot chain, and detect any cycles.
- File mdirs (mid>=-1) need to traverse both the on-disk bshrub/btree
and any opened file handles' bshrubs/btrees before moving onto the
next mid.
This grows O(n^2) because all file handles are in one big unsorted
linked-list, but as usual we don't care.
In addition to the greatly simplified traversal logic, the new state
matrix simplifies traversal clobbering: Setting bid=-2 always forces a
bshrub/btree refetch.
This comes at the cost of traversal _precision_, i.e. we can now revisit
previously visited bshrub/btree nodes. But I think this is well worth it
for more robust traversal clobbering. Traversal clobbering is delicate
and difficult to get right.
Besides, we can already revisit blocks due to CoW references, so what's
the harm in revisiting blocks when under mutation?
---
The simpler traversal logic leads to a nice amount of code savings
across the board:
code stack ctx
before: 36476 2304 660
after: 35940 (-1.5%) 2280 (-1.0%) 660 (+0.0%)
code stack ctx
gbmap before: 39524 2320 772
gbmap after: 38916 (-1.5%) 2296 (-1.0%) 772 (+0.0%)
code stack ctx
gc before: 36548 2304 804
gc after: 36012 (-1.5%) 2280 (-1.0%) 776 (-3.5%)
Note the ctx savings in LFS3_GC mode. Most of the stack/ctx savings
comes from the smaller lfs3_mtrv_t struct, which no longer needs to
stage bshrubs (we no longer care about bshrubs across mdir commit as a
part of the above clobbering simplifications):
before after
lfs3_mtrv_t: 128 100 (-21.9%)
lfs3_mgc_t: 128 100 (-21.9%)
lfs3_trv_t: 136 108 (-20.6%)
Unfortunately, the simpler clobbering means now any gc work needs the
block queue (i.e. lfs3_trv_t), solely so clobbering the block queue
doesn't clobber unallocated memory. Not great but hopefully fixable.
---
Some other notes:
- As a part of simplifying traversal clobbering, everything is triggered
by lfs3_alloc_ckpoint (via lfs3_trv_ckpoint_).
This may clobber traversals more than is strictly necessary, but
that's kinda the idea. Better safe than sorry.
And no more need to explicit lfs3_handle_clobber calls is nice.
- Opened file handle iteration is now tracked by the traversal handle's
position in the handle linked-list, instead of a separate handle
pointer. This means one less thing to disentangle and makes traversals
no longer a special case for things like lfs3_handle_close.
You may think this bumps traversals up to O(n^3) in-ram, but because
we only ever visit each unique handle + mid once, we can keep the
total O(n^2) if we're smart about linked-list updates!
- lfs3_mdir_commit needed to be tweaked to accept mids<=-1, instead of
just mid=-1 for the mroot. Unfortunately I don't know how much this
costs on its own.
- The reorganization of lfs3_mtrv_t means lfs3_mtortoise_t gets its own
struct again!
- No more tstate state machine also frees up a big chunk of the
traversal flag space, which was getting pretty cramped.
This is in preparation for some traversal simplification ideas, which
rely on all auxiliary/non-file btrees being visitable before file
btrees.
In theory the order of file vs auxiliary btrees doesn't really matter,
other than the number of different routes from mtree/mroot -> gbmap/file
btrees being a bit of a pain.
Note this is not true for the mtree, which must come first for
lfs3_mount to work.
---
Adds a bit of code when building with the gbmap:
code stack ctx
before: 36480 2304 660
after: 36476 (-0.0%) 2304 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 39464 2320 772
gbmap after: 39524 (+0.2%) 2320 (+0.0%) 772 (+0.0%)
code stack ctx
gc before: 36552 2304 804
gc after: 36548 (-0.0%) 2304 (+0.0%) 804 (+0.0%)
This is motivated by the observation that the O(n log_b n) btree
iteration really just hasn't been a bottleneck in our benchmarks.
Our write performance is mostly dominated by compaction costs, and while
filesystem _traversals_ are a concern, it's easy to explicitly track
rbyds in lfs3_btrv_t.
Additionally:
- We track mdirs during mtree iteration, which are the true mtree
leaves.
- We already cache file leaves, i.e. bptrs and read-fragments.
On top of this, leaf caching adds complexity, both in terms of
code/stack costs, but also in terms of reliability. It introducing the
need for cache invalidation, which is infamously one of the two hard
problems in computer science!
This is the second(?) time btree leaf traversals have been reverted, so
see previous commit messages for even more arguments against.
---
Eventually, we should probably just delete the btree leaf cache logic to
avoid the maintenance headache (cache invalidation + opt+in/less
testing = ouch). But I want to do a bit more benchmarking comparing the
two modes, so just moving this behind an ifdef for now.
Saves code, and of course RAM:
code stack ctx
before btrv: 37160 2352 688
before: 37088 (-0.2%) 2384 (+1.4%) 688 (+0.0%)
after: 36480 (-1.8%) 2304 (-2.0%) 660 (-4.1%)
But note while this keeps the performance implications of btree leaf
caching, it does not keep the code/stack optimizations that internally
reuse the leaf cache for things (btrv, lookupnext_ rbyd side-channel,
etc).
In _theory_ these could have been kept with enough ifdefs, but it would
have made the codebase quite a bit of a hell to maintain:
code stack ctx
always-bleafcache: 37160 2352 688
no-bleafcache: 36480 (-1.8%) 2304 (-2.0%) 660 (-4.1%)
yes-bleafcache: 37044 (-0.3%) 2384 (+1.4%) 688 (+0.0%)
Gbmap mode has even more savings due to how many gbmap copies we have
flying around:
code stack ctx
gbmap + always-bleafcache: 40132 2368 856
gbmap + no-bleafcache: 39464 (-1.7%) 2320 (-2.0%) 772 (-9.8%)
gbmap + yes-bleafcache: 40052 (-0.2%) 2400 (+1.4%) 856 (+0.0%)
---
In the future, _maybe_ we can revisit this. But I think a better design
would be to cache btree leaves globally, in lfs3_t, similarly to the
theoretical mdir cache. This would allow a user-configurable number of
cached btree nodes, and may make cache invalidation easier.
Note, however, that btree nodes don't need to be fetched (even for
commits now!), so the benefits would be much smaller than for the
theoretical mdir cache.
But hey, it would defend the lack of low-level rbyd tracking during
iteration/rattr queries!
Brings back lfs3_btrv_t, but keeps some of the btree internal changes.
I think the biggest one is dropping the internal branch pointer, now
instead of internally pointing to the root rbyd, we just unconditionally
sync the rbyd state anytime the rbyd matches the root's weight. This is
necessary to avoid out-of-sync state when traversing bshrubs under
mutation.
Also after refactoring I think the current btree traversal logic is
easier to read.
---
This is in preparation for removing the leaf cache, or at least making
it opt-in.
It adds a chunk of stack, but in theory we can reclaim this by allowing
leaf caches to be disabled:
code stack ctx
before: 37160 2352 688
after: 37088 (-0.2%) 2384 (+1.4%) 688 (+0.0%)
I think these are good ideas to bring back when littlefs3 is more
mature, but at the moment the number of different builds is creating too
much friction.
LFS3_KVONLY and LFS3_2BONLY in particular _add_ significant chunks of
code (lfs3_file_readget_, lfs3_file_flushset_, and various extra logic
sprinkled throughout the codebase), and the current state of testing
means I have no idea if any of it still works.
These are also low-risk for introducing any disk related changes.
So, ripping out for now to keep the current experimental development
tractable. May reintroduce in the future (probably after littlefs3 is
stabilized) if there is sufficient user interest. But doing so will
probably also need to come with actual testing in CI.
This just fell out-of-sync a bit during the gbmap work. Note we _do_
support LFS3_RDONLY + LFS3_GBMAP, as fetching the gbmap is necessary for
CKMETA to check all metadata. Fortunately this is relatively cheap:
code stack ctx
rdonly: 10716 896 532
rdonly+gbmap: 10988 (+2.5%) 896 (+0.0%) 680 (+27.8%)
Though this does highlight that a sort of LFS3_NO_TRV mode could remove
quite a bit of code.
This walks back some of the attempt at strict object namespacing in
struct lfs3_cfg:
- cfg.file_cache_size -> cfg.fcache_size
- filecfg.cache_size -> filecfg.fcache_size
- filecfg.cache_buffer -> filecfg.fcache_buffer
- cfg.gbmap_re_thresh -> cfg.regbmap_thresh
Motivation:
- cfg.regbmap_thresh now matches cfg.gc_regbmap_thresh, instead of using
awkwardly different namespacing patterns.
- Giving fcache a more unique name is useful for discussion. Having
pcache, rcache, and then file_cache was a bit awkward.
Hopefully it's also more clear that cfg.fcache_size and
filecfg.fcache_size are related.
- Config in struct lfs3_cfg is named a bit more consistently, well, if
you ignore gc_*_* options.
- Less typing.
Though this gets into pretty subjective naming territory. May revert
this if the new terms are uncomfortable after use.
So:
- cfg.gc_repoplookahead_thresh -> cfg.gc_relookahead_thresh
- cfg.gc_repopgbmap_thresh -> cfg.gc_regbmap_thresh
- cfg.gbmap_repop_thresh -> cfg.gbmap_re_thresh
- LFS3_*_REPOPLOOKAHEAD -> LFS3_*_RELOOKAHEAD
- LFS3_*_REPOPGBMAP -> LFS3_*_REGBMAP
Mainly trying to reduce the mouthful that is REPOPLOOKAHEAD and
REPOPGBMAP.
As a plus this also avoids potential confusion of "repop" as a push/pop
related operation.
This drops LFS3_t_MUTATED in favor of just using LFS3_t_CKPOINTED
everywhere:
1. These meant roughly the same thing, with LFS3_t_MUTATED being a bit
tighter at the cost of needing to be explicitly set.
2. The implicit setting of LFS3_t_CKPOINTED by lfs3_alloc_ckpoint -- a
function that already needs to be called before mutation -- means we
have one less thing to worry about.
Implicit properties like LFS3_t_CKPOINTED are great for building a
reliable system. Manual flags like LFS3_t_MUTATED, not so much.
3. Why use two flags when we can get away with one?
The only downside is we may unnecessarily clobber gc/traversal work when
we don't actually mutate the filesystem. Failed file open calls are a
good example.
However this tradeoff seems well worth it for an overall simpler +
more reliable system.
---
Saves a bit of code:
code stack ctx
before: 37220 2352 688
after: 37160 (-0.2%) 2352 (+0.0%) 688 (+0.0%)
code stack ctx
gbmap before: 40184 2368 856
gbmap after: 40132 (-0.1%) 2368 (+0.0%) 856 (+0.0%)
Note: This affects the blocking lfs3_alloc_repopgbmap as well as
incremental gc/traversal repopulations. Now all repop attempts return
LFS3_ERR_NOSPC when we don't have space for the gbmap, motivation below.
This reverts the previous LFS3_t_NOSPC soft error, in which traversals
were allowed to continue some gc/traversal work when encountering
LFS3_ERR_NOSPC. This results in a simpler implementation and fewer error
cases to worry about.
Observation/motivation:
- The main motivation is noticing that when we're in low-space
conditions, we just start spamming gbmap repops even if they all fail.
That's really not great! We might as well just mark the flash as dead
if we're going to start spamming erases!
At least with an error the user can call rmgbmap to try to make
progress.
- If we're in a low-space condition, something else will probably return
LFS3_ERR_NOSPC anyways. Might as well report this early and simplify
our system.
- It's a simpler model, and littlefs3 is already much more complicated
than littlefs2. Maybe we should lean more towards a simpler system
at the cost of some niche optimizations.
---
This had the side-effect of causing more lfs3_alloc_ckpoints to return
errors during testing, which revealed a bug in our uz/uzd_fuzz tests:
- We weren't flushing after writes to the opened RDWR files, which could
cause delayed errors to occur during the later read checks in the
test.
Fortunately LFS3_O_FLUSH provides a quick and easy fix!
Note we _don't_ adopt this in all uz/uzd_fuzz tests, only those that
error. It's good to test both with and without LFS3_O_FLUSH to test
that read-flushing also works under stress.
Saves a bit of code:
code stack ctx
before: 37260 2352 688
after: 37220 (-0.1%) 2352 (+0.0%) 688 (+0.0%)
code stack ctx
gbmap before: 40220 2368 856
gbmap after: 40184 (-0.1%) 2368 (+0.0%) 856 (+0.0%)
This adds test_gc_nospc with more aggressive testing of gc/traversal
operations in low-space conditions. The original intention was to test
the new soft-ENOSPC traversal behavior, but instead it found a couple
unrelated bugs.
In my defense these involve some rather subtle filesystem interactions
and went unnoticed because we don't usually check data checksums:
1. lfs3_bd_flush had a rare chance where it could corrupt our
prog-aligned pcksum when (1) we bypass the pcache, allowing any
previous contents to stay there until flush/pcksum, and (2) some
other failed prog, in this case failing repopgbmaps due to the
low-space condition, leaves garbage in the pcache. When we flush
we corrupt the pcksum even though the old data belongs to an
unrelated block.
This resulted in CKDATA failing, though the failed check is a false
positive.
As a workaround, lfs3_bd_prog and lfs3_bd_prognext now discard _any_
unrelated pcache, even if bypassing the pcache. This should ensure
consistent behavior in all cases. Note we do something similar for
with the file cache in lfs3_file_write.
This means progs may not complete unless lfs3_bd_flush is called, but
I think we need to call lfs3_bd_flush in all cases anyways to ensure
power-loss safe behavior.
The end result should be a more reliable internal bd prog API.
2. On a successful traversal with LFS3_T_REPOPLOOKAHEAD and
LFS3_T_REPOPGBMAP we adopt both the new gbmap and lookahead buffer.
This is wrong! The lookahead buffer is not aware of the gbmap during
the traversal, and _can't_ be aware as the gbmap changes during
repopulation work. This is the whole reason we have the alloc
ckpoints and the in-flight window.
To fix, adopting the lookahead buffer is now conditional on _not_
adopting a new gbmap.
It makes the code a bit more messy, but this is the correct behavior.
Populating both the gbmap and lookahead buffere requires at least two
passes.
Code changes minimal:
code stack ctx
before: 37248 2352 688
after: 37260 (+0.0%) 2352 (+0.0%) 688 (+0.0%)
code stack ctx
gbmap before: 40204 2368 856
gbmap after: 40220 (+0.0%) 2368 (+0.0%) 856 (+0.0%)
Christopher Haster