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%)
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.
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%)
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 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%)
To allow relaxing when LFS3_I_REPOPLOOKAHEAD and LFS3_I_REPOPGBMAP will
be set, potentially reducing gc workload after allocating only a couple
blocks.
The relevant cfg comments have quite a bit more info.
Note -1 (not the default, 0, maybe we should explicitly flip this?)
restores the previous functionality of setting these flags on the first
block allocation.
---
Also tweaked gbmap repops during gc/traversals to _not_ try to repop
unless LFS3_I_REPOPGBMAP is set. We probably should have done this from
the beginning since repopulating the gbmap writes to disk and is
potentially destructive.
Adds code, though hopefully we can claw this back with future config
rework:
code stack ctx
before: 37176 2352 684
after: 37208 (+0.1%) 2352 (+0.0%) 688 (+0.6%)
code stack ctx
gbmap before: 40024 2368 848
gbmap after: 40120 (+0.2%) 2368 (+0.0%) 856 (+0.9%)
- LFS3_T_COMPACT -> LFS3_T_COMPACTMETA
- gc_compact_thresh -> gc_compactmeta_thresh
And friends:
LFS3_M_COMPACTMETA 0x00000800 Compact metadata logs
LFS3_GC_COMPACTMETA 0x00000800 Compact metadata logs
LFS3_I_COMPACTMETA 0x00000800 Filesystem may have uncompacted metadata
LFS3_T_COMPACTMETA 0x00000800 Compact metadata logs
---
This does two things:
1. Highlights that LFS3_T_COMPACTMETA only interacts with metadata logs,
and has no effect on data blocks.
2. Better matches the verb+noun names used for other gc/traversal flags
(REPOPGBMAP, CKMETA, etc).
It is a bit more of a mouthful, but I'm not sure that's entirely a bad
thing. These are pretty low-level flags.
And friends:
LFS3_M_REPOPLOOKAHEAD 0x00000200 Repopulate lookahead buffer
LFS3_GC_REPOPLOOKAHEAD 0x00000200 Repopulate lookahead buffer
LFS3_I_REPOPLOOKAHEAD 0x00000200 Lookahead buffer is not full
LFS3_T_REPOPLOOKAHEAD 0x00000200 Repopulate lookahead buffer
To match LFS3_T_REPOPGBMAP, which is more-or-less the same operation.
Though this does turn into quite the mouthful...
There's a strong argument for naming this inline_size as that's more
likely what users expect, but shrub_size is just the more correct name
and avoids confusion around having multiple names for the same thing.
It also highlights that shrubs in littlefs3 are a bit different than
inline files in littlefs2, and that this config also affects large files
with a shrubbed root.
May rerevert this in the future, but probably only if there is
significant user confusion.
And tweaked a few related comments.
I'm still on the fence with this name, I don't think it's great, but it
at least betters describes the "repopulation" operation than
"rebuilding". The important distinction is that we don't throw away
information. Bad/erased block info (future) is still carried over into
the new gbmap snapshot, and persists unless you explicitly call
rmgbmap + mkgbmap.
So, adopting gbmap_repop_thresh for now to see if it's just a habit
thing, but may adopt a different name in the future.
As a plus, gbmap_repop_thresh is two characters shorter.
A big downside of LFS3_T_REBUILDGBMAP is the addition of an lfs3_btree_t
struct to _every_ traversal object.
Unfortunately, I don't see a way around this. We need to track the new
gbmap snapshot _somewhere_, and other options (such as a global gbmap.b_
snapshot) just move the RAM around without actually saving anything.
To at least mitigate this internally, this splits lfs3_trv_t into
distinct lfs3_trv_t, lfs3_mgc_t, and lfs3_mtrv_t structs that capture
only the relevant state for internal traversal layers:
- lfs3_mtree_traverse <- lfs3_mtrv_t
- lfs3_mtree_gc <- lfs3_mgc_t (contains lfs3_mtrv_t)
- lfs3_trv_read <- lfs3_trv_t (contains lfs3_mgc_t)
This minimizes the impact of the gbmap rebuild snapshots, and saves a
big chunk of RAM. As a plus it also saves RAM in the default build by
limiting the 2-block block queue to the high-level lfs3_trv_read API:
code stack ctx
before: 37176 2360 684
after: 37176 (+0.0%) 2352 (-0.3%) 684 (+0.0%)
code stack ctx
gbmap before: 40060 2432 848
gbmap after: 40024 (-0.1%) 2368 (-2.6%) 848 (+0.0%)
The main downside? Our field names are continuing in their
ridiculousness:
lfs3.gc.gc.t.b.h.flags // where else would the global gc flags be?
- lfs3_gbmap_set* -> lfs3_gbmap_mark*
- lfs3_alloc_markfree -> lfs3_alloc_adopt
- lfs3_alloc_mark* -> lfs3_alloc_markinuse*
Mainly for consistency, since the gbmap and lookahead buffer are more or
less the same algorithm, ignoring nuances (lookahead only ors inuse
bits, gbmap rebuilding can result in multiple snapshots, etc).
The rename lfs3_gbmap_set* -> lfs3_gbmap_mark* also makes space for
lfs3_gbmap_set* to be used for range assignments with a payload, which
may be useful for erased ranges (gbmap tracked ecksums?)
This adds LFS3_T_REBUILDGBMAP and friends, and enables incremental gbmap
rebuilds as a part of gc/traversal work:
LFS3_M_REBUILDGBMAP 0x00000400 Rebuild the gbmap
LFS3_GC_REBUILDGBMAP 0x00000400 Rebuild the gbmap
LFS3_I_REBUILDGBMAP 0x00000400 The gbmap is not full
LFS3_T_REBUILDGBMAP 0x00000400 Rebuild the gbmap
On paper, this is more or less identical to repopulating the lookahead
buffer -- traverse the filesystem, mark blocks as in-use, adopt the new
gbmap/lookahead buffer on success -- but a couple nuances make
rebuilding the gbmap a bit trickier:
- Unlike the lookahead buffer, which eagerly zeros in allocation, we
need an explicit zeroing pass before we start marking blocks as
in-use. This means multiple traversals can potentially conflict with
each other, risking the adoption of a clobbered gbmap.
- The gbmap, which stores information on disk, relies on block
allocation and the temporary "in-flight window" defined by allocator
ckpoints to avoid circular block states during gbmap rebuilds. This
makes gbmap rebuilds sensitive to allocator ckpoints, which we
consider more-or-less a noop in other parts of the system.
Though now that I'm writing this, it might have been possible to
instead include gbmap rebuild snapshots in fs traversals... but that
would probably have been much more complicated.
- Rebuilding the gbmap requires writing to disk and is generally much
more expensive/destructive. We want to avoid trying to rebuild the
gbmap when it's not possible to actually make progress.
On top of this, the current trv-clobber system is a delicate,
error-prone mess.
---
To simplify everything related to gbmap rebuilds, I added a new
internal traversal flag: LFS3_t_CKPOINTED:
LFS3_t_CKPOINTED 0x04000000 Filesystem ckpointed during traversal
LFS3_t_CKPOINTED is set, unconditionally, on all open traversals in
lfs3_alloc_ckpoint, and provides a simple, robust mechanism for checking
if _any_ allocator checkpoints have occured since a traversal was
started. Since lfs3_alloc_ckpoint is required before any block
allocation, this provides a strong guarantee that nothing funny happened
to any allocator state during a traversal.
This makes lfs3_alloc_ckpoint a bit less cheap, but the strong
guarantees that allocator state is unmodified during traversal are well
worth it.
This makes both lookahead and gbmap passes simpler, safer, and easier to
reason about.
I'd like to adopt something similar+stronger for LFs3_t_MUTATED, and
reduce this back to two flags, but that can be a future commit.
---
Unfortunately due to the potential for recursion, this ended up reusing
less logic between lfs3_alloc_rebuildgbmap and lfs3_mtree_gc than I had
hoped, but at like the main chunks (lfs3_alloc_remap,
lfs3_gbmap_setbptr, lfs3_alloc_adoptgbmap) could be split out into
common functions.
The result is a decent chunk of code and stack, but the value is high as
incremental gbmap rebuilds are the only option to reduce the latency
spikes introduced by the gbmap allocator (it's not significantly worse
than the lookahead buffer, but both do require traversing the entire
filesystem):
code stack ctx
before: 37164 2352 684
after: 37208 (+0.1%) 2360 (+0.3%) 684 (+0.0%)
code stack ctx
gbmap before: 39708 2376 848
gbmap after: 40100 (+1.0%) 2432 (+2.4%) 848 (+0.0%)
Note the gbmap build is now measured with LFS3_GBMAP=1, instead of
LFS3_YES_GBMAP=1 (maybe-gbmap) as before. This includes the cost of
mkgbmap, lfs3_f_isgbmap, etc.
And:
- Tweaked the behavior of gbmap.window/known to _not_ match disk.
gbmap.known matching disk is what required a separate
lookahead.bmapped in the first place, but we never use both fields.
- _Don't_ revert gbmap on failed mdir commits!
This was broken! If we reverted we risked inheriting outdated
in-flight block information.
This could be fixed by also zeroing lookahead.bmapped, but would force
a gbmap rebuild. And why? The only interaction between mdir commit and
the gbmap is block allocation, which is intentionally allowed to go
out-of-sync to relax issues like this.
Note we still revert in lfs3_fs_grow, the new gbmap we create there is
incompatible with the previous disk size.
As a part of these changes, gbmap.window now behaves roughly the same as
gbmap.known and updates eagerly on block allocation.
This makes lookahead.window and gbmap.window somewhat redundant, but
simplifies the relevant logic (especially due to how lookahead.window
lags behind lookahead.off).
---
A bunch of bugs fell out-of-this, the interactions with lfs3_fs_mkgbmap
and lfs3_fs_grow being especially tricky, but fortunately our testing is
doing a good job.
At least the code changes were minimal, saves a bit of RAM:
code stack ctx
no-gbmap before: 37168 2352 684
no-gbmap after: 37168 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
maybe-gbmap before: 39688 2392 852
maybe-gbmap after: 39720 (+0.1%) 2376 (-0.7%) 848 (-0.5%)
code stack ctx
yes-gbmap before: 39156 2392 852
yes-gbmap after: 39208 (+0.1%) 2376 (-0.7%) 848 (-0.5%)
These two functions allow changing whether or not the gbmap is in use
after format:
// Enable the global on-disk block-map
//
// Returns a negative error code on failure. Does nothing if a gbmap
// already exists.
int lfs3_fs_mkgbmap(lfs3_t *lfs3);
// Disable the global on-disk block-map
//
// Returns a negative error code on failure. Does nothing if no gbmap
// is found.
int lfs3_fs_rmgbmap(lfs3_t *lfs3);
rmgbmap was easy enough, but implementing mkgbmap turned out to be
surprisingly tricky due to how gstate permeates the system:
- Even if we zero gstate when we removing the gbmap, mounting the
image on a driver that doesn't understand the gbmap results in garbage
gstate over time as mdir compacts drop unknown gdeltas.
I think this sort of implicit gdelta cleanup is a good thing, but the
possibility of garbage gstate is a bit annoying.
Example A: the dbg scripts are currently printing a bunch of warnings
for corrupt gstate that can be safely ignored.
To support recovering from garbage gstate in mkgbmap, I changed
lfs3_fs_commitgdelta to _always_ track p state even when disabled. We
already needed to do this in lfs3_fs_flush/consumegdelta anyways,
since we don't know if the gbmap is used until parsing wcompat flags.
- The commit that enables the gbmap is tricky. We need the gbmap enabled
to calculate the new gdelta, but we also need it disabled so we don't
traverse the existing gbmap_p (which may be garbage).
As a workaround I added gbmap.b_p, which is in theory redundant with
gbmap_p, but (1) avoids needing to decode gbmap_p during traversals,
and (2) allows the two to temporarily fall out-of-sync in mkgbmap.
This means we potentially have 5 (!) snaphots flying around when
rebuilding the gbmap, which is starting to get a bit silly. But this
was also motivated by gbmap_p decoding adding roughly the same amount
of RAM to lfs3_mtree_traverse_, so the total RAM usage should in
theory be roughly the same.
There might be a better solution, but this at least gets mkgbmap
working. The gbmap builds are not our most RAM senstive configurations
anyways.
---
Also added a couple more tests in test_gbmap to test these:
- test_gbmap_files
- test_gbmap_rmgbmap
- test_gbmap_mkgbmap
- test_gbmap_rmmkgbmap
- test_gbmap_mkrmgbmap
And an explicit wraparound test to test_alloc. This was loosely implied
by the nospc tests, but it's probably better to have an explicit test.
The only downside is this implementation is limited to files:
- test_alloc_wraparound_files
---
Note we are currently dealing with three different configurations:
no-gbmap (the default), yes-gbmap (LFS3_YES_GBMAP), and maybe-gbmap
(LFS3_GBMAP + LFS3_F_GBMAP at runtime).
It only makes sense to include these in maybe-gbmap mode, so this is the
only mode with a notable code increase. However these functions are
relatively cheap. The stack/ctx changes also affect yes-gbmap, but
should mostly cancel out, see above:
code stack ctx
no-gbmap before: 37168 2352 684
no-gbmap after: 37168 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
maybe-gbmap before: 39292 2456 800
maybe-gbmap after: 39688 (+1.0%) 2392 (-2.6%) 852 (+6.5%)
code stack ctx
yes-gbmap before: 39116 2456 800
yes-gbmap after: 39156 (+0.1%) 2392 (-2.6%) 852 (+6.5%)
In lfs3_fs_grow, we need to update any gbmaps to match the new disk
size. The actual patch to the gbmap is easy, but it does get a bit
delicate since we need to feed the gbmap with an allocator in the new
disk size.
Fortunately, the opportunistism of the gbmap allocator avoids any
catch-22 issues, as long as we make sure to not trigger any gbmap
rebuilds.
Adds a bit of code, but not much:
code stack ctx
before: 37168 2352 684
after: 37168 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
gbmap before: 39000 2456 800
gbmap after: 39116 (+0.3%) 2456 (+0.0%) 800 (+0.0%)
Having gbmap/bmap used in different places for the same thing was
confusing. Preferring gbmap as it is consistent with other gstate (grm
queue, gcksums), even if it is a bit noisy.
It's interesting to note what didn't change:
- The BM* range tags: LFS3_TAG_BMFREE, etc. These already differs from
the GBMAP* prefix enough, and adopting GBM* would risk confusion for
actual gstate.
- The gbmap revdbg string: "bb~r". We don't have enough characters for
anything else!
- dbgbmap.py/dbgbmapsvg.py. These aren't actually related to the gbmap,
so the name difference is a good thing.
TLDR: This drops the idea of different bmap strategies/modes, and sorts
out most of the compile-time/runtime conditional bmap interactions.
---
Motivation: Benchmarking (at least up to the 32-bit word limit) has
shown the bmap will unlikely be a significant bottleneck, even on large
disks. The largest disks tend to be NAND, and NAND's ridiculous block
size limits pressure on block allocation.
There are still concerns for areas I haven't measured yet:
- SD/eMMC/FTL - Small blocks, so more pressure on block allocation. In
theory the logical block size can be artificially increased, but this
comes with a granularity tradeoff.
- I've only measured throughput, latency is a whole other story.
However, users have reported lfs3_fs_gc is useful for mitigating this,
so maybe latency is less of a concern now?
But while there may still be room for improvement via alternative bmap
strategies, the risk a concerning amount of complexity. Yes,
configuration gets more complicated, but the real issue is any bmap
strategies that try to track _deallocations_ (the original idea being
treediffing) risk falling leaking blocks if all cases aren't covered.
The current "bmap cache" strategy strikes a really nice balance where it
reduces _amortized_ block allocation -> ~O(log n) without RAM, while
retaining the safe, bug-resistant, single-source-of-truth properties
that come with lookahead-based allocation.
---
So, long story short, dropping other strategies, and now the presence of
the bmap is a boolean flag.
This is also the first format-specific flag:
- Define LFS3_BMAP to enable the bmap logic, but note by default the
bmap will still not be used.
- Define LFS3_YES_BMAP to force the bmap to be used.
- With LFS3_BMAP, passing LFS3_F_GBMAP to lfs3_format will include the
on-disk block-map.
- No flag is needed during mount, the presence of the bmap is determined
by the on-disk wcompat flags (LFS3_WCOMPAT_GBMAP). This also prevents
rw mounting if the bmap is not supported, but rdonly mounting is
allowed.
- Users can check if the bmap is in use via lfs3_fs_stat, which reports
LFS3_I_GBMAP in the flags field.
There's still some missing pieces, but these will be a bit more
involved:
- lfs3_fs_grow needs to be made bmap aware!
- We probably want something like lfs3_fs_mkgbmap and lfs3_fs_rmgbmap to
allow converting between bmap backed/not-backed filesystem images.
Code changes minimal:
code stack ctx
before: 37172 2352 684
after: 37172 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38844 2456 800
bmap after: 38852 (+0.0%) 2456 (+0.0%) 800 (+0.0%)
New bmap range tags:
LFS3_TAG_BMRANGE 0x033u v--- --11 --11 uuuu
LFS3_TAG_BMFREE 0x0330 v--- --11 --11 ----
LFS3_TAG_BMINUSE 0x0331 v--- --11 --11 ---1
LFS3_TAG_BMERASED 0x0332 v--- --11 --11 --1-
LFS3_TAG_BMBAD 0x0333 v--- --11 --11 --11
Note 0x334-0x33f are still reserved for future bmap tags, but the new
encoding fits in the surprisingly common 2-bit subfield that may
deduplicate some decoding code.
Fitting in 2-bits is the main reason for this, now that in-flight ranges
look like they won't be worth exploring further. Worst case we can
always add more bm tags in the future. And it may even make sense to use
an entire bit for in-flight tags, since in theory the concept can apply
to more than just in-use blocks.
---
Another benefit of this encoding: In-use vs free is a bit check, and I
like the implication that an in-use + erased block can only be a bad
block.
No code changes:
code stack ctx
before: 37172 2352 684
after: 37172 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38844 2456 800
bmap after: 38844 (+0.0%) 2456 (+0.0%) 800 (+0.0%)
Now that lfs3_alloc_ckpoint is more complicated, and can error, it makes
sense for lfs3_alloc_ckpoint to be implied by lfs3_mdir_commit.
Most lfs3_mdir_commit calls represent an atomic transaction from one
state -> another, so this saves a bit of code:
code stack ctx
before: 36912 2368 684
after: 36836 (-0.2%) 2368 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38512 2400 812
bmap after: 38436 (-0.2%) 2400 (+0.0%) 812 (+0.0%)
The notable exception being bshrub-related commits in
lfs3_bshrub_commitroot_. Bshrub commits are trying to resolve an
in-flight btree, so the relevant blocks are very much _not_ at rest.
---
I've been hesitant to adopt this mostly just because it makes the
lfs3_mdir_commit* names even more of a mess:
- lfs3_mdir_commit__ -> lfs3_mdir_commit___
- lfs3_mdir_commit_ -> lfs3_mdir_commit__
- lfs3_mdir_commit -> lfs3_mdir_commit_
- added lfs3_mdir_commit
- lfs3_mdir_compact -> lfs3_mdir_compact_
- add lfs3_mdir_compact
- lfs3_mdir_alloc__ -> lfs3_mdir_alloc___
- lfs3_mdir_estimate__ -> lfs3_mdir_estimate___
- lfs3_mdir_swap__ -> lfs3_mdir_swap___
The main change is error propagation in lfs3_alloc_ckpoint. Since
lfs3_alloc_ckpoint writes to disk during bmap rebuilds, it can now fail
in all sorts of ways. Fortunately lfs3_alloc_ckpoint should only ever be
called by write operations, where these errors are be expected.
With bmap rebuild errors now reported correctly, this unblocks most of
the remaining test TODOs:
- Passing test_badblocks
- Passing test_ck
- Passing test_trvs
With this, LFS3_YES_BMAP is now passing all but two tests, which are
still ifndef-disabled as a temporary measure:
- test_btree - We make some low-level assumptions about the lookahead
allocator when testing btrees. It's probably not worth trying to get
this passing with the bmap allocator.
- test_grow - This one does need fixing! We currently don't update
on-disk bmaps correctly when growing the filesystem.
Code changes minimal:
code stack ctx
before: 36912 2368 684
after: 36912 (+0.0%) 2368 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38456 2400 812
bmap after: 38512 (+0.1%) 2400 (+0.0%) 812 (+0.0%)
We need to adjust mids to ignore orphans during dir traversal, but we
shouldn't also adjust the dir position. In theory it shouldn't matter if
we use adjusted/non-adjusted dir positions, but it becomes a problem if
intermediate writes cause those orphans to be cleaned up. Now all your
dir positions are wrong.
Not entirely sure why this only started to fail with the bmap. I'm
guessing it's just due to the additional gstate causing the mdirs to
split differently.
Code changes minimal:
code stack ctx
before: 36920 2368 684
after: 36912 (-0.0%) 2368 (+0.0%) 684 (+0.0%)
Tangential, but toss this on the pile of problems with dir positions.
I'm increasingly convinced we should just remove the concept if we can
get away with it.
There's really no reason to immediately commit the bmap to disk, at
least no until the first mdir commit, when we need to at least discard
the previous bmap state.
We already do all the gstate handling in lfs3_mdir_commit anyways, and
piggybacking on mdir commit lets us get rid of the annoying extra mdir
param in lfs3_alloc_ckpoint.
This does mean a slightly higher risk of needing to re-rebuild the bmap
after a powerloss, but in theory only if the user does something weird
like writing to a file and never calling sync. Most on-disk operations
terminate in an mdir commit as that's how any state change becomes
atomically visibile in littlefs.
Saves a nice bit of stack:
code stack ctx
before: 36920 2368 684
after: 36920 (+0.0%) 2368 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38552 2472 812
bmap after: 38464 (-0.2%) 2400 (-2.9%) 812 (+0.0%)
At least at a proof-of-concept level, there's still a lot of cleanup
needed.
To make things work, lfs3_alloc_ckpoint now takes an mdir, which
provides the target for gbmap gstate updates.
When the bmap is close to empty (configurable via bmap_scan_thresh), we
opportunistically rebuild it during lfs3_alloc_ckpoints. The nice thing
about lfs3_alloc_ckpoint is we know the state of all in-flight blocks,
so rebuilding the bmap just requires traversing the filesystem + in-RAM
state.
We might still fall back to the lookahead buffer, but in theory a well
tuned bmap_scan_thresh can prevent this from becoming a bottleneck (at
the cost of more frequent bmap rebuilds).
---
This is also probably a good time to resume measuring code/ram costs,
though it's worth repeating the above note about the bmap work still
needing cleanup:
code stack ctx
before: 36840 2368 684
after: 36920 (+0.2%) 2368 (+0.0%) 684 (+0.0%)
Haha, no, the bmap isn't basically free, it's just an opt-in features.
With -DLFS3_YES_BMAP=1:
code stack ctx
no bmap: 36920 2368 684
yes bmap: 38552 (+4.4%) 2472 (+4.4%) 812 (+18.7%)
This doesn't fully replace the lookahead buffer, but at least augments
it with known bmap state when available.
To be honest, this is a minimal effort hack to try to get something
benchmarkable without dealing with all the catch-22 issues that a
self-support bmap allocator would encounter (allocating blocks for the
bmap requires a bmap, oh no).
Though now that I'm writing this, maybe this is a reasonable long-term
solution? Having the lookahead buffer to fall back on solves a lot of
problems, and, realistically, it's unlikely to be a performance
bottleneck unless the user has extreme write requests (>available
storage?).
---
Also tweaked field naming to be consistent between the bmap and
lookahead buffer.
This avoids issues with the different traversal paths with an mtree vs
inline-mtree. Previously this was broken when the mtree was inlined.
This order also makes more sense if we want to check mdirs before we
consider the gstate to be trustworthy enough for gbmap traversal.
The neat thing about the on-disk bmap is that it's a range tree. We can
leverage order-statistic properties to compactly represent ranges of
similar blocks.
However, this does make updating the bmap slightly more complicated...
This comes from an observation that we never actually use the leaf cache
during traversals, and there is surprisingly little risk of a lookup
creating a conflict in the future.
Btree traversal fall into two categories:
1. Full traversals, where we traverse a full btree all at once. These
are unlikely to have lookup conflicts because everything is
usually self-contained in one chunk of logic.
2. Incremental traversals. These _are_ at risk, but in our current
design limited to lfs3_trv_t, which already creates a fully
bshrub/btree copy for tracking purposes.
This copy unintentionally, but conveniently, protects against lookup
conflicts.
So, why not reuse the btree leaf cache to hold the rbyd state during
traversals? In theory this makes lfs3_btree_traverse the same cost and
lfs3_btree_lookupnext, drops the need for lfs3_btrv_t, and simplifies
the internal API.
The only extra bit of state we need is the current target bid, which is
now expected as a caller-incremented argument similar to
lfs3_btree_lookupnext iteration.
There was a bit of futzing around with bid=-1 being necessary to
initialize traversal (to avoid conflicts with bid=-1 => 0 caused by
empty btrees). But the end result is a btree traversal that only needs
one extra word of state.
---
Unfortunately, in practice, the savings were not as great as expected:
code stack ctx
before: 36792 2400 684
after: 36876 (+0.2%) 2384 (-0.7%) 684 (+0.0%)
This does claw back some stack, but less than a full rbyd due to the
union with the mtortoise in lfs3_trv_t. The mtortoise now dominates. It
might be possible to union the mtortoise and the bshrub/btree state
better (both are not needed at the same time), but strict aliasing rules
in C make this tricky.
The new lfs3_btree_traverse is also a bit more complicated in terms of
code cost. In theory this would be offset by the simpler traversal setup
logic, but we only actually call lfs3_btree_traverse twice:
1. In lfs3_mtree_traverse
2. In lfs3_file_ck
Still, some stack savings + a simpler internal API makes this worthwhile
for now. lfs3_trv_t is also due for a revisit, and hopefully it's
possible to better union things with btree leaf caches somehow.
This is an indulgence to simplify the upcoming auxiliary btree work.
Brings back the previously-reverted per-btree leaf caches, where each
lfs3_btree_t keeps track of two rbyds: The root and the most recently
accessed leaf.
At the surface level, this optimizes repeated access to the same btree
leaf. A common pattern for a number of littlefs's operations that has
proven tricky to manually optimize:
- Btree iteration
- Pokes for our crystalization heuristic
- Checksum collision resolution for dids and (FUTURE) ddkeys
- Related rattrs attached to a single bid
But the real motivation is to drop lfs3_btree_*lookupleaf and simplify
the internal APIs. If repeated lfs3_btree_lookup*s are already
efficient, there's no reason for extra leaf-level APIs, and in theory
any logic that interacts with btrees will be simpler.
---
This comes at a cost (humorously about the same amount as the
tag-returning refactor, if you ignore the extra 28 bytes of ctx).
Unsurprisingly, increasing the size of lfs3_btree_t has the biggest
impact on stack and ctx:
code stack ctx
before: 36084 2336 656
after: 36784 (+1.9%) 2400 (+2.7%) 684 (+4.3%)
Also note from the previous commit messages: Btree leaf caching has
resulted in surprisingly little performance improvement for our current
benchmarks + implementation. It turns out if you're dominated by write
cost, optimizing btree lookups -- which already skip rbyd fetches, has
barely noticeable impact.
---
A note on reverting!
Eventually (after the auxiliary btree work) it will probably make sense
to revert this -- or at least provide a non-leaf-caching build for
code/RAM sensitive users.
I don't think this should be reverted as-is. Instead, I think we should
allow the option to just disable the leaf cache, while keeping the
simpler internal API. This would give us the best of all three worlds:
- A small code/RAM option
- Optimal btree iteration/nearby-lookup performance
- Simpler internal APIs
The only reason this isn't already implemented is because I want to
avoid fragmenting the codebase further while we're still in development
mode.
Note this includes both the lfs3_config -> lfs3_cfg structs as well as
the LFS3_CONFIG -> LFS3_CFG include define:
- LFS3_CONFIG -> LFS3_CFG
- struct lfs3_config -> struct lfs3_cfg
- struct lfs3_file_config -> struct lfs3_file_cfg
- struct lfs3_*bd_config -> struct lfs3_*bd_cfg
- cfg -> cfg
We were already using cfg as the variable name everywhere. The fact that
these names were different was an inconsistency that should be fixed
since we're committing to an API break.
LFS3_CFG is already out-of-date from upstream, and there's plans for a
config rework, but I figured I'd go ahead and change it as well to lower
the chances it gets overlooked.
---
Note this does _not_ affect LFS3_TAG_CONFIG. Having the on-disk vs
driver-level config take slightly different names is not a bad thing.
- test_traversal -> test_trvs
- lfs3_traversal_t -> lfs3_trv_t
- lfs3_btraversal_t -> lfs3_btrv_t
- t -> trv
- bt -> btrv
- lfs3_traversal_* -> lfs3_trv_*
- lfs3_btraversal_* -> lfs3_btrv_*
The traversal type is becoming one of the more fundamental types in
littlefs, and if DIR and REG both get shortened names, it makes sense
for TRV to have one as well.
This also removes the temptation to use t for traversals, which is
probably an even worse name.
---
Note that lfs3_btree_traverse, lfs3_mtree_traverse, etc, remain
unaffected. This may change in the future, but it's interesting to note
that verbs seem to need much less typing than nouns.
- lfs3_omdir_t -> lfs3_handle_t
- lfs3.omdirs -> lfs3.handles
- o -> h
- lfs3_omdir_* -> lfs3_handle_*
- lfs3_omdir_ismidopen -> lfs3_mid_isopen
From conversations with users, the term "handle" or "file handle" seems
to be the most common/easily understood term for the lfs3_file_t struct
itself. It makes sense to adopt this in our codebase.
I usually dislike inventing new names for things when prefixes can imply
a relationship (size -> ssize, cache -> rcache, shrub -> bshrub, etc),
but lfs3_omdirs_t was probably a bit much.
Last but not least, this adopts tag-returns in lfs3_mtree_pathlookup,
and indirectly in all of lfs3_mtree_pathlookup's callers (which is
almost every top-level filesystem function -- anything that needs to
look up a path).
At this level, the muxed tag/err type really shows its versatility. Take
the LFS3_ERR_NOENT and LFS3_TAG_ORPHAN tags/errs for example.
Conceptually, these take very different code paths, but after calling
lfs3_mtree_pathlookup, it's easy to switch on both as though they
represent the same file-not-found condition.
We have to be a bit more careful now to not confuse err and tag
variables in these functions, and `goto failed` is now a bit of a
landmine, but the end result is another nice chunk of code savings:
code stack ctx
before: 36216 2336 656
after: 36084 (-0.4%) 2336 (+0.0%) 656 (+0.0%)
---
I believe this finishes the tag-returning refactor, which means we can
take a step back and look at how effective tag/err muxing is as a code
size optimization:
code stack ctx
before tag-returns: 36828 2368 656
after tag-returns: 36084 (-2.0%) 2336 (-1.4%) 656 (+0.0%)
A free 744 bytes is not bad! Especially considering there's no real
downside to this.
The 32 bytes of stack savings is nice too, and suggests we had ~8
unnecessary tag out-pointers sitting on the stack hot-path.
- lfs3_mdir_namelookup
- lfs3_mtree_namelookup
These are interesting, because, unlike lfs3_rbyd_namelookup, we don't
care about how query mids compare with the found mid.
Adopting tag-returns does mean we no longer return the relevant tag
when the query mid is missing, but the fact that the tests are passing
means this is a non-issue.
Shaves off a bit more code:
code stack ctx
before: 36260 2336 656
after: 36216 (-0.1%) 2336 (+0.0%) 656 (+0.0%)
Maybe these should have been updated in lock-step with
lfs3_mtree_pathlookup, but lfs3_mtree_pathlookup is going to impact a
lot more code...
- lfs3_mtree_traverse_
- lfs3_mtree_traverse
- lfs3_mtree_gc
I like this one if only for the reduced API noise. All of these layers
need to inspect the tag to know what to do, moving the tag to the return
position means less mucking around with points in our core traversal
logic.
Shaves off a bit more code:
code stack ctx
before: 36348 2336 656
after: 36260 (-0.2%) 2336 (+0.0%) 656 (+0.0%)
- lfs3_mdir_lookupnext
- lfs3_mdir_lookup
Like btree lookups, mdir lookups are also tag-inspection heavy, so we
see some nice savings:
code stack ctx
before: 36520 2352 656
after: 36348 (-0.5%) 2336 (-0.7%) 656 (+0.0%)
lfs3_mdir_lookup also highlights how tag-returns help reduce API noise
around the tag mask bits. lfs3_mdir_lookup's tag out-pointer doesn't
really make sense with the default non-masked tags, and moving it to the
return position hides it aways a bit.
- lfs3_btree_lookupleaf
- lfs3_btree_lookupnext
- lfs3_btree_lookup
- lfs3_btree_traverse
- NOT lfs3_btree_namelookup
Looks like we're starting to claw back stack usage a bit. This makes
sense as the btree logic involves the most layers -- with out-pointers
it needs more temporary copies to inspect tags along the way:
code stack ctx
before: 36576 2376 656
after: 36520 (-0.2%) 2352 (-1.0%) 656 (+0.0%)
This is the start of a big refactor to try to move tag out-pointers into
the return position of functions, muxing with error codes via the
sign-bit when necessary.
So instead of:
lfs3_tag_t tag_;
lfs3_data_t data_;
int err = lfs3_rbyd_lookup(&lfs3, &rbyd, rid, tag,
&tag_, &data_);
if (err) {
return err;
}
We now do:
lfs3_data_t data_;
lfs3_stag_t tag_ = lfs3_rbyd_lookup(&lfs3, &rbyd, rid, tag,
&data_);
if (tag_ < 0) {
return tag_;
}
In theory, removing an out-pointer saves both code and stack, though it
will be interesting to actually see how much of an affect this has after
the dust has settled.
littlefs v2 used this technique heavily for its 32-bit tags, but we
never did a comparison with/without tags in the return position.
This is a big rewrite in the test code, so hopefully this ends up worth
it :)
Lots of regex.
Note this implicitly limits error codes to 16-bits, but supported error
codes are already a bit limited because we're using int everywhere
(instead of int32_t). If we need 32-bit error codes we can always add
another type to represent the mux in the future (lfs3_etag_t?).
---
So far the code savings look promising:
code stack ctx
before: 36828 2368 656
after: 36576 (-0.7%) 2376 (+0.3%) 656 (+0.0%)
Stack usage is a big disappointing, but hopefully that is just a
temporary cost due to the internal scaffolding between different API
types while the refactor is ongoing.
This replaces the `bool align` parameter that goes through all the prog
layers with an optional prog-aligned cksum stored in the lfs3_t struct.
Normally ignored, this prog-aligned cksum can be requested by setting
cksum=&lfs3->pcksum in any prog call.
Does this work? Yes. Is it a great solution? Ehhhh...
I've been tinkering with other solutions that avoid the `bool align`
parameter, but with no luck.
- `bool align`, or previously two cksum arguments, work, but create a
bit of a messy API. I'd like to find an alternative solution.
- Changing the cksum pointer to a richer lfs3_cksum_t struct with flags
also works, but would be an even messier API.
- Adding an lfs3_t side-channel, lfs3->pcache could include a pointer to
an optional prog-aligned cksum. But this would be the same/more cost
as just storing the pcksum in lfs3_t. And then we'd need to worry
about disentangling the cksum pointer on errors, etc.
- We could set a flag in lfs3->flags for alignment. This avoids the
extra 4 bytes of ctx, but still suffers from the risk of entangled
state on errors, etc.
- We could unconditionally calculate lfs3->pcksum. But then we'd be
calculating a lot of cksums we don't use (every metadata commit), and
still using the extra 4 bytes of ctx.
Lacking a good solution, using cksum=&lfs3->pcksum to indicate a
prog-aligned cksum is at least an ok solution.
I will happily change this if an alternative comes up in the future.
Another way of viewing this is that `&lfs3->pcksum` acts as a special
magic pointer value to tell the prog layers to calculate lfs3->pcksum.
A different non-NULL constant value could have worked just as well, but
those are a bit trickier to create in C.
---
Actually, there is a "better" cursed solution:
- Rely on pointer alignment to sneak a flag into the cksum pointer's
lower bits.
But, while clever, this is is outside of C's machine model and would
limit portability.
---
This trades 4 bytes of ctx for 58 bytes of code and simpler (debatable)
internal prog APIs:
code stack ctx
before: 36860 2384 652
after: 36832 (-0.1%) 2384 (+0.0%) 656 (+0.6%)
In theory this also saves stack in all the prog APIs, but none of prog
APIs end up on the stack hot-path. In our codebase the read APIs
dominate the stack thanks to block allocator traversals.
LFS3_CKDATACKSUMREADS is just too much.
The downside is it may not be clear how LFS3_CKDATACKSUMREADS interacts
with the future planned LFS3_CKREADS (LFS3_CKREADS implies
LFS3_CKDATACKSUMS + LFS3_CKMETAREDUND), but on the flip side you may
actually be able to type LFS3_CKDATACKSUMS on the first try.
Christopher Haster