Allocating pre-erased blocks gets quite complicated due to our
restricted flash model, but at least the actual pre-erasing is
relatively straightforward:
- We keep track of known preerased state in lfs3->gbmap.preeraser.
- If LFS3_GC_PREERASE is provided during gc work, we increment the
preeraser's known window by scanning the gbmap.
- Any BMFREE ranges we find, we erase a block at a time, and store the
resulting ecksum in a BMERASED range in the gbmap.
- We keep track of how many blocks we erased, and stop early if this
exceeds cfg.gc_preerase_count. This just lets users tune how many
blocks to preerase in case something (?) prevents preerased blocks
from being used.
Some notes:
- We don't really do anything with ranges in lfs3_alloc_preerase. In
theory we could bulk in erase to minimize the number of commits to the
gbmap, but we expect erase to dominate, so this probably isn't worth
it.
And if erase doesn't dominate, why would you bother pre-erasing
blocks?
- Preerasing isn't really a traversal operation, and is managed by a
sort of secondary state machine in lfs3_fs_gc_.
This also means lfs3_trv_read with LFS3_T_PREERASE does nothing, but I
guess that is ok? It's tempting to try to make lfs3_trv_read also
preerase, but it's unclear what block it should return -- it's
probably the wrong API.
- Introducing ecksums actually went quite a bit smoother than I
expected. Though it helps ecksums are the only optional payload, no
type punning or anything.
Ecksums do muddy the gbmap's design a bit, unfortunately. The main
issue being that we can only merge BMERASED ranges with equal ecksums.
This makes BMERASED ranges less compressable than the others, and may
be one reason to limit cfg.gc_preerase_count.
However:
1. This is where I think it's useful to emphasize that the gbmap's
responsibility is to track _free_ blocks, in-use blocks are
secondary.
When allocating, we're going to stop at the first BMFREE/BMERASED,
but may need to skip over an unbounded number of BMINUSE/BMBAD
blocks. So the compressability of BMFREE/BMERASED ranges should
have less of an impact on block allocation.
2. In practice, most flash uses consistent erase values, so the
resulting ecksums will probably be compressable. The exceptions are
noop-erases (SD/eMMC, RAM, NVRAM, etc), and encryption with block
address permutation?
Though noop-erases are a pretty big exception.
Code changes:
code stack ctx
before: 35116 2136 660
after: 35116 (+0.0%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap+np before: 38040 2136 776
gbmap+np after: 38188 (+0.4%) 2144 (+0.4%) 776 (+0.0%)
code stack ctx
gbmap+yp before: 38040 2136 776
gbmap+yp after: 38608 (+1.5%) 2144 (+0.4%) 796 (+2.6%)
This does two things:
- Deduplicates another fixgrm call, now all fixgrm cleanup (outside of
mkdir/remove) goes through lfs3_mtree_gc.
- Predicates fixgrm on if lookahead work is complete.
Code changes:
code stack ctx
before: 35128 2136 660
after: 35116 (-0.0%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38052 2136 776
gbmap after: 38040 (-0.0%) 2136 (+0.0%) 776 (+0.0%)
Despite programming in C for quite a while, I didn't expect this to
work. But it turns out you _can_ typedef nested structs, you just need
to put the typedef outside the struct.
IMO this makes it a bit easier to understand what state is exclusive to
the lfs3_t struct. We do use these typedefs internally, but only for
pointers into the core lfs3_t.
No code changes.
Also tweaked related flag comments a bit.
---
This adds another internal flag modifier ('a') to dbgflags.py to
indicate a flag is an alias for multiple other flags.
These still show up in -l/--list and name searches:
$ ./scripts/dbgflags.py -l +ck
LFS3_CK_MKCONSISTENT 0x00000100 Make the filesystem consistent
LFS3_CK_LOOKAHEAD 0x00000200 Repopulate lookahead buffer
LFS3_CK_COMPACT 0x00000800 Compact metadata logs
LFS3_CK_CKMETA 0x00001000 Check metadata checksums
LFS3_CK_CKDATA 0x00002000 Check metadata + data checksums
LFS3_CK_CK 0x00003000 Alias for all check work
LFS3_CK_GC 0x00003b00 Alias for all gc work
But are hidden from value searches, as they would be redundant and the
specific low-level flags are probably more useful:
$ ./scripts/dbgflags.py +ck 0x00003000
LFS3_CK_CKMETA 0x00001000 Check metadata checksums
LFS3_CK_CKDATA 0x00002000 Check metadata + data checksums
If lfs3_fs_fixgrm is an implicit requirement for LFS3_T_MKCONSISTENT, we
might as well move it into the core lfs3_mtree_gc logic and save on the
redundant lfs3_fs_fixgrm calls.
Saves a bit of code:
code stack ctx
before: 35164 2136 660
after: 35128 (-0.1%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38088 2136 776
gbmap after: 38052 (-0.1%) 2136 (+0.0%) 776 (+0.0%)
Will revert.
The idea here is that fixgrm isn't really a traversal operation. It's
convenient, but in an effort to simplify things, dropping fixgrm from
lfs3_trv_read makes sense.
But dropping fixgrm seems to cause more problems than it's worth.
---
Note test_trvs is currently failing because attempting to remove an
orphaned stickynote in the grm queue without calling fixgrm breaks
things.
It's probably fixable, but why? If we keep the implied fixgrm it's not
possible to trigger a remove without a clean grm queue. And we want to
keep our grm queue clean anyways to prevent a full fixorphan scan.
Code changes:
code stack ctx
before: 35164 2136 660
after: 35112 (-0.1%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38088 2136 776
gbmap after: 38040 (-0.1%) 2136 (+0.0%) 776 (+0.0%)
This extends the hopefully useful LFS3_GC_ALL flag alias to the other
functions, without trying to figure out what the heck LFS3_M_ALL should
mean semantically.
Current definitions:
// an alias for ck work
*_CK 0x00003000 ---- ---- ---- ---- --11 ---- ---- ----
// an alias for all possible gc work
*_GC 0x00003b00 ---- ---- ---- ---- ++11 11+1 ---- ----
*_MKCONSISTENT 0x00000100 ---- ---- ---- ---- ---- ---1 ---- ----
*_LOOKAHEAD 0x00000200 ---- ---- ---- ---- ---- --1- ---- ----
*_PREERASE+ 0x00000400 ---- ---- ---- ---- ---- -+-- ---- ----
*_COMPACT 0x00000800 ---- ---- ---- ---- ---- 1--- ---- ----
*_CKMETA 0x00001000 ---- ---- ---- ---- ---1 ---- ---- ----
*_CKDATA 0x00002000 ---- ---- ---- ---- --1- ---- ---- ----
*_REPAIRMETA+ 0x00004000 ---- ---- ---- ---- -+-- ---- ---- ----
*_REPAIRDATA+ 0x00008000 ---- ---- ---- ---- +--- ---- ---- ----
+ Planned
One weird artifact of this is that LFS3_GC_CK ~= LFS3_GC_CKDATA, but I'm
not sure that's a bad thing? Note this is very much not true for info
flags, LFS3_I_CK != LFS3_I_CKDATA.
No code changes.
LFS3_GC_ALL will hopefully be useful for users for convenience, but
internally we should probably preter explicit flag sets to make flag
changes explicit and easy to tweak.
---
One intention was to make the progress check less messy, but that didn't
really work. Attempting to deduplicate the LFS3_GC_LOOKAHEAD flag just
hurts literal sharing in the function.
Which is honestly a strong argument against this change...
No code changes.
Unsure if this falls into over-engineering. But note lfs3->flags can
change between gc steps, so the aborting useless traversals is probably
worth keeping if only for that reason.
In the future this is a good function to tweak without worrying about
compatibility issues.
Code changes:
code stack ctx
before: 35124 2136 660
after: 35164 (+0.1%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38048 2136 776
gbmap after: 38088 (+0.1%) 2136 (+0.0%) 776 (+0.0%)
This is a number of tweaks intended to (1) minimize unexpected gc
latency due to last-minute lookahead scans, while (2) keeping the gc
logic simple and easy to reason about:
- Switched to using lfs3->flags directly for the is-work-done predicate.
This ensures lfs3_fs_gc_ never terminates until the requested work is
done, at the risk of, well, never terminating.
But the previous "pending" variables had the same risk (set
gc_compact_thresh=0 for example), it just removed the risk of
non-termination due to conflicting gc requests. In both cases
gc_compact_thresh has the biggest risk of non-termination.
- Prioritize lookahead scans before anything that can allocate
(MKCONSISTENT, COMPACT, etc).
- Dropped aborting useless traversals (ckpointed lookaheads mainly).
It's a good idea, but surprisingly complicated to decide when we
should abort for all traversals. COMPACT, CKMETA, for example, are
still useful to continue even if they can't prove anything about the
system.
Though now that I'm writing this, I'm wondering what the argument
against LOOKAHEAD aborting is. Maybe this should be reverted for
LOOKAHEAD as a special case...
Code changes minimal:
code stack ctx
before: 35152 2136 660
after: 35124 (-0.1%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38076 2136 776
gbmap after: 38048 (-0.1%) 2136 (+0.0%) 776 (+0.0%)
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).
dbgflags.py now uses +flags to indicate flag namespaces, but our gdb
script only forwarded -f/--flags, which made dbgflags a bit of a pain
to use in the debugger!
Fortunately an easy fix.
Now this works:
(gdb) dbgflags +t trv.gc.t.h.flags
LFS3_T_RDWR 0x00000000 Open traversal as read and write
LFS3_T_MKCONSISTENT 0x00000100 Make the filesystem consistent
LFS3_T_LOOKAHEAD 0x00000200 Repopulate lookahead buffer
LFS3_t_TRAVERSAL 0x60000000 Type = traversal
LFS3_t_MDIR 0x00010000 Btype = mdir
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.
The idea is this is similar to ~ for the home directory, hopefully
simplifying littlefs path parsing in scripts:
- /hi -> hi in root dir
- ./hi -> hi in current dir
- ../hi -> hi in parent dir
- ~/hi -> hi in home dir
- %/hi -> hi in littlefs root dir
Note this only works when standalone:
- ~hi != ~/hi
- %hi != %/hi
And files named % can still be referenced with a ./ prefix:
- ./% -> % in current dir
- %/% -> % in littlefs root dir
---
This is probably overkill for dbglfs3.py, as the arg ordering was
already enough to disambiguate disk path vs mroot address vs littlefs
path, but eventually I think the idea will be useful for more powerful
scripts.
A hypothetical:
$ mklfs3 cp disk -b4096 -r image_files %/image_files
This is still a hack to make the seek _enum_ appear somewhat readable in
our dbg _flags_ script. But the previously internal SEEK_MODE was
causing all seek flags to be hidden from -l/--list confusingly.
Just a bit less typing than --o, and lowers risk of conflicts with
actual flags we may care about.
To be honest I was procrastinating because I thought this would be a lot
more work! I was prepared to write a hacky secondary parser, but argparse
already supports this natively with prefix_chars='-+'. Yay!
Test/bench filters have proven to be mostly non-optional, protecting
against bad configuration that doesn't make any sense.
It's still valid to want to override test filters sometimes, but using a
more, uh, forceful verb probably makes sense here.
The shortform would conflict with -f/--fail, so no shortform flag for
this, but some argue --force should never have a shortform flag anyways.
I'm trying to avoid the inevitable conflict with -w/--word, which will
probably become important when exploring non-32-bit filesystem
configurations.
Renaming this to -t/--wait still conflicts with -t/--tree and -t/--tiny,
but as a debug-only flag, I think these are less important.
Oh, and -t/--trace, but test.py/bench.py are already quite different in
their flag naming (see -d/--disk vs -d/--diff).
---
Renamed a few other flags while tweaking things:
- -t/--tiny -> --tiny (dropped shortform)
- -w/--word-bits -> -w/--word/--word-bits
- -t/--tree -> -R/--tree/--rbyd/--tree-rbyd
- -R/--tree-rbyd -> -Y/--rbyd-all/--tree-rbyd-all
- -B/--tree-btree -> -B/--btree/--tree-btree
After tinkering with it a bit, I think the -R/-Y/-B set of flags are a
decent way to organize the tree renderers. At least --tree-rbyd-all does
a better job of describing the difference between --tree-rbyd and
--tree-rbyd-all.
This changes -w/--wait to sleep _after_ -k/--keep-open, instead of
including the time spent waiting on inotifywait in the sleep time.
1. It's easier, no need to keep track of when we started waiting.
2. It's simpler to reason about.
3. It trivially avoids the multiple wakeup noise that plagued
watch.py + vim (vim likes to do a bunch of renaming and stuff when
saving files, including the file 4913 randomly?)
Avoiding this was previously impossible because -~/--sleep was
effectively a noop when combined with -k/--keep-open.
---
Also renamed from -~/--sleep -> -w/--wait, which is a bit more intuitive
and avoids possible shell issues with -~.
To make this work, dropped the -w/--block-cycles shortform flag in
dbgtrace.py. It's not like this flag is ever used anyways.
Though at the moment this is ignoring the possible conflict with
-w/--word-bits...
This was motivated by a discussion with a gh user, in which it was noted
that not having a reserved suptype for internal tags risks potential
issues with long-term future tag compatibility.
I think the risk is low, but, without a reserved suptype, it _is_
possible for a future tag to conflict with an internal tag in an older
driver version, potentially and unintentionally breaking compatibility.
Note this is especially concerning during mdir compactions, where we
copy tags we may not understand otherwise.
In littlefs2 we reserved suptype=0x100, though this was mostly an
accident due to saturating the 3-bit suptype space. With the larger tag
space in littlefs3, the reserved suptype=0x100 was dropped.
---
Long story short, this reserves suptype=0 for internal flags (well, and
null, which is _mostly_ internal only, but does get written to disk as
unreachable tags).
Unfortunately, adding a new suptype _did_ require moving a bunch of
stuff around:
LFS3_TAG_NULL 0x0000 v--- ---- +--- ----
LFS3_TAG_INTERNAL 0x00tt v--- ---- +ttt tttt
LFS3_TAG_CONFIG 0x01tt v--- ---1 +ttt tttt
LFS3_TAG_MAGIC 0x0131 v--- ---1 +-11 --rr
LFS3_TAG_VERSION 0x0134 v--- ---1 +-11 -1--
LFS3_TAG_RCOMPAT 0x0135 v--- ---1 +-11 -1-1
LFS3_TAG_WCOMPAT 0x0136 v--- ---1 +-11 -11-
LFS3_TAG_OCOMPAT 0x0137 v--- ---1 +-11 -111
LFS3_TAG_GEOMETRY 0x0138 v--- ---1 +-11 1---
LFS3_TAG_NAMELIMIT 0x0139 v--- ---1 +-11 1--1
LFS3_TAG_FILELIMIT 0x013a v--- ---1 +-11 1-1-
LFS3_TAG_ATTRLIMIT? 0x013b v--- ---1 +-11 1-11
LFS3_TAG_GDELTA 0x02tt v--- --1- +ttt tttt
LFS3_TAG_GRMDELTA 0x0230 v--- --1- +-11 ----
LFS3_TAG_GBMAPDELTA 0x0234 v--- --1- +-11 -1rr
LFS3_TAG_GDDTREEDELTA* 0x0238 v--- --1- +-11 1-rr
LFS3_TAG_GPTREEDELTA* 0x023c v--- --1- +-11 11rr
LFS3_TAG_NAME 0x03tt v--- --11 +ttt tttt
LFS3_TAG_BNAME 0x0300 v--- --11 +--- ----
LFS3_TAG_REG 0x0301 v--- --11 +--- ---1
LFS3_TAG_DIR 0x0302 v--- --11 +--- --1-
LFS3_TAG_STICKYNOTE 0x0303 v--- --11 +--- --11
LFS3_TAG_BOOKMARK 0x0304 v--- --11 +--- -1--
LFS3_TAG_SYMLINK? 0x0305 v--- --11 +--- -1-1
LFS3_TAG_SNAPSHOT? 0x0306 v--- --11 +--- -11-
LFS3_TAG_MNAME 0x0330 v--- --11 +-11 ----
LFS3_TAG_DDNAME* 0x0350 v--- --11 +1-1 ----
LFS3_TAG_DDTOMB* 0x0351 v--- --11 +1-1 ---1
LFS3_TAG_STRUCT 0x04tt v--- -1-- +ttt tttt
LFS3_TAG_BRANCH 0x040r v--- -1-- +--- --rr
LFS3_TAG_DATA 0x0404 v--- -1-- +--- -1--
LFS3_TAG_BLOCK 0x0408 v--- -1-- +--- 1err
LFS3_TAG_DDKEY* 0x0410 v--- -1-- +--1 ----
LFS3_TAG_DID 0x0420 v--- -1-- +-1- ----
LFS3_TAG_BSHRUB 0x0428 v--- -1-- +-1- 1---
LFS3_TAG_BTREE 0x042c v--- -1-- +-1- 11rr
LFS3_TAG_MROOT 0x0431 v--- -1-- +-11 --rr
LFS3_TAG_MDIR 0x0435 v--- -1-- +-11 -1rr
LFS3_TAG_MSHRUB+ 0x0438 v--- -1-- +-11 1---
LFS3_TAG_MTREE 0x043c v--- -1-- +-11 11rr
LFS3_TAG_BMRANGE 0x044u v--- -1-- +1-- ++uu
LFS3_TAG_BMFREE 0x0440 v--- -1-- +1-- ----
LFS3_TAG_BMINUSE 0x0441 v--- -1-- +1-- ---1
LFS3_TAG_BMERASED 0x0442 v--- -1-- +1-- --1-
LFS3_TAG_BMBAD 0x0443 v--- -1-- +1-- --11
LFS3_TAG_DDRC* 0x0450 v--- -1-- +1-1 ----
LFS3_TAG_DDPCOEFF* 0x0451 v--- -1-- +1-1 ---1
LFs3_TAG_PCOEFFMAP* 0x0460 v--- -1-- +11- ----
LFS3_TAG_ATTR 0x06aa v--- -11a +aaa aaaa
LFS3_TAG_UATTR 0x06aa v--- -11- +aaa aaaa
LFS3_TAG_SATTR 0x07aa v--- -111 +aaa aaaa
LFS3_TAG_SHRUB 0x1kkk v--1 kkkk +kkk kkkk
LFS3_TAG_ALT 0x4kkk v1cd kkkk +kkk kkkk
LFS3_TAG_CKSUM 0x300p v-11 ---- ++++ +pqq
LFS3_TAG_NOTE 0x3100 v-11 ---1 ++++ ++++
LFS3_TAG_ECKSUM 0x3200 v-11 --1- ++++ ++++
LFS3_TAG_GCKSUMDELTA 0x3300 v-11 --11 ++++ ++++
* Planned
+ Reserved
? Hypothetical
Some additional notes:
- I was on the fence on keeping the 0x30 prefix on config tags now that
it is not longer needed to differentiate from null, but ultimately
decided to keep it because: 1. it's fun, 2. it decreases the chance
of false positives, 3. it keeps the redund bits readable in hexdumps,
and 4. it reserves some tags < config, which is useful since order
matters.
Instead, I pushed the 0x30 prefix to _more_ tags, mainly gstate.
As a coincidence, meta related tags (MNAME, MROOT, MRTREE) all shifted
to also have the 0x30 prefix, which is a nice bit of unexpected
consistency.
- I also considered reserving the redund bits across the config tags
similarly to what we've done in struct/gstate tags, but decided
against it as 1. it significantly reduces the config tag space
available, and 2. makes alignment with VERSION + R/W/OCOMPAT a bit
awkward.
Instead I think would should relax the redund bit alignment in other
suptypes, though in practice the intermixing of non-redund and redund
tags makes this a bit difficult.
Maybe we should consider including redund bits as a hint for things
like DATA? DDKEY? BSHRUB? etc?
- I created a bit more space for file btree struct tags, allowing for
both the future planned DDKEY, and BLOCK with optional erased-bit. We
don't currently use this, but it may be useful for the future planned
gddtree, which in-theory can track erased-state in partially written
file blocks.
Currently tracking erased-state in file blocks is difficult due to
the potential of multiple references, and inability to prevent ecksum
conflicts in raw data blocks.
- UATTR/SATTR bumped up to 0x600/0x700 to keep the 1-bit alignment,
leaving the suptype 0x500 unused. Though this may be useful if we ever
run out of struct tags (suptype=0x400), which is likely where most new
tags will go.
---
Code changes were minimal, but with a bunch of noise:
code stack ctx
before: 35912 2280 660
after: 35920 (+0.0%) 2280 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38800 2296 772
gbmap after: 38812 (+0.0%) 2296 (+0.0%) 772 (+0.0%)
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
Christopher Haster