This was a funny issue for external benchmarking, where we've focused
mostly on throughput benchmarking so far.
The current throughput approach is to run a benchmark for a given
simtime, and record the number of bytes written after. This is great for
allowing benchmarks to fail gracefully, but doesn't really work with the
current bench runner, which expected a known n in BENCH_START.
We can work around this by calling BENCH_START/STOP a second time
(making a mess of later scripts), but it would be nice if this was fixed
in the bench runner.
---
Humorously, BENCH_START just stores n to be printed out when BENCH_STOP
is called, so this was an easy fix.
This is based on some work in external benchmarks. What's worked well
there is emulating a global simtime based on per-byte estimates.
This moves the emulated simtime into emubd/kiwibd, and extends the idea
with both per-byte and per-op timing estimates for hopefully more
realistic results.
---
The problem is how NAND flash reads work.
Per-byte timing estimates are surprisingly accurate for NOR flash. There
is some overhead for sending the address, but it's mostly dominated by
bus cost (~20ns/B [1]).
NAND flash, on the otherhand, technically does support byte-level reads,
but first needs to read into 2KiB buffer. Surprisingly, these are pretty
close in cost (~19ns/B bus [2] vs ~12ns/B buffer [2]).
This close-ness makes modeling NAND flash difficult. If we set
read_size=1, we risk hiding the cost of small reads, which littlefs3 is
full of (rbyd lookups). If we set read_size=2048, we unfairly penalize
littlefs3 for the same reason.
---
The solution here is to expose both per-byte and per-op timing
estimates. This lets you model NAND reads using two data points:
^
| realtime --> ...............o
| : .....'''' :
| ...............:'''' ^ :
| :....''''' | :
| ..........:::::: simtime :
| .....:'''' :
|o....:::::.....: :
|: :
|: :
+:-----------------------------------------------------------:>
min read max read
Where:
bus_timing = 19ns
buffer_timing = 25us
buffer_size = 2KiB
erase_size = 128KiB
min_read = buffer_timing
max_read = (erase_size/buffer_size)*buffer_timing - buffer_timing
read_timing = min_read
readed_timing = ((max_read - min_read)/erase_size) + bus_timing
simtime = reads*read_timing + readed*readed_timing
(per-op) (per-byte)
This should correctly penalize small reads without complicating
emubd/kiwibd too much.
That's the idea anyways! It will take some use to understand if this is
a reasonable approach.
As a plus, this is a superset of the per-byte model, so both can be used
for realistic vs idealistic simulations (and to test the bus+buffer
model itself).
1: https://www.winbond.com/resource-files/W25Q256JV%20SPI%20RevQ%2002072025%20Plus.pdf
2: https://www.winbond.com/resource-files/W25N01GV%20Rev%20R%20070323.pdf
The big TEST_IMPLICIT_DEFINES and TEST_CFG macros have been a big
pain-in-the-ass to maintain. Mostly due to C preprocessor annoyances
(bleh escaped newlines) and no-ifdef workarounds, which make a real mess
of things.
This does two things:
1. Moves all the defines out of test_runner.h and into test_defines.h
(same for benches).
2. Inverts the include logic such that test_defines.h gets included many
times with various "query macros" defined.
Currently just two, but can easily add more:
1. TEST_DEFINE(name, value) - name and default value for a define
2. TEST_CFG(name, value) - name and value for a cfg field
This seems to work surprisingly well. It solves all of the above C
preprocessor issues, and provides a flexible method for defining test
defines.
Note an important part of making this work is that test_defines.h
expands to an empty string by default.
This was resulting in memory leak warnings from Valgrind, which were
getting in the way of debugging an unrelated uninitialized memory issue.
We normally wouldn't care about this sort of bounded memory leaks, but
in this case Valgrind can't tell if the memory leak is from the runner
or filesystem, errors, and prevents other tests from running. Just to be
more annoying, this only triggered when overriding defines, which is
something you do exactly when you are trying to debug something.
Fortunately, with a bit of typecasting we still have access to the
allocated value arrays (type-stripped due to opaque test_define_t), and
can clean up the relevant memory.
The original motivation for making LFS3_PREERASE opt-out, is that it
makes sense for LFS3_GBMAP to bring in all gbmap-related features
(PREERASE, BADBLOCKS (future)). However, after a bit of use, I think
this just complicates our ifdef logic too much.
So instead, LFS3_PREERASE is now opt-in, with the intention of making
all ifdefs relative only to the default build. I think this will make it
easier to reason about ifdefs, at least internally.
Eventually, I want to look into alternative default builds (LFS3_BIGGER,
LFS3_BIGGERR, ..., LFS3_BIGGEST), which would provide an alternative way
to enable all gbmap-related features. Though these builds have a
high-risk of bikeshedding (LFS3_GC?), so we'll see.
---
That being said, the main ergonomic improvement was probably adding
a #error, so we don't have to check ifdef GBMAP everywhere.
Maybe this should be extended to LFS3_RDONLY? Or maybe not, LFS3_RDONLY
is a bit of a special case.
No code changes:
code stack ctx
before: 35144 2136 660
after: 35144 (+0.0%) 2136 (+0.0%) 660 (+0.0%)
code stack ctx
gbmap before: 38380 2144 776
gbmap after: 38380 (+0.0%) 2144 (+0.0%) 776 (+0.0%)
code stack ctx
preerase before: 38920 2168 796
preerase after: 38920 (+0.0%) 2168 (+0.0%) 796 (+0.0%)
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 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.
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).
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.
Yeah, after using these for a bit, the RE* names were not great.
Trying LOOK* now, as an alternative that hopefully still implies the
similar behavior without needing an additional prefix for LOOKAHEAD:
- LFS3_*_RELOOKAHEAD -> LFS3_*_LOOKAHEAD
- LFS3_*_REGBMAP -> LFS3_*_LOOKGBMAP
- cfg.regbmap_thresh -> cfg.lookgbmap_thresh
- cfg.gc_relookahead_thresh -> cfg.gc_lookahead_thresh
- cfg.gc_regbmap_thresh -> cfg.gc_lookgbmap_thresh
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.
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%)
This is an alias for all possible gc work, which is a bit more
complicated than you might think due to compile-time features (example:
LFS3_GC_REPOPGBMAP).
The intention is to make loops like the following easy to write:
struct lfs3_fsinfo fsinfo;
lfs3_fs_stat(&lfs3, &fsinfo) => 0;
lfs3_trv_t trv;
lfs3_trv_open(&lfs3, &trv, fsinfo.flags & LFS3_GC_ALL) => 0;
...
It's possible to do this by explicitly setting all gc flags, but that
requires quite a bit of knowledge from the user.
Another option is allowing -1 for gc/traversal flags, but that loses
assert protection against unknown/misplaced flags.
---
This raises more questions about the prefix naming: it feels a bit weird
to take LFS3_I_* flags, mask with LFS3_GC_* flags, and pass them as
LFS3_T_* flags, but it gets the job done.
Limiting LFS3_GC_ALL to the LFS3_GC_* namespace avoids issues with
opt-out/mode flags such as LFS3_T_RDONLY, LFS3_T_MTREEONLY, etc. For
this reason it probably doesn't make sense to add something similar to
the other namespaces.
- 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.
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.
I think a good rule of thumb is if you refer to some variable/config/
field with a different name in comments/writing/etc more often than not,
you should just rename the variable/config/field to match.
So yeah, gbmap_rebuild_thresh controls when the gbmap is rebuilt.
Also touched up the doc comment a bit.
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.
For some reason emubd had both a path argument to lfs3_emubd_create, and
a disk_path config option, with only the disk_path actually being used.
But the real curiosity is why did GCC only starting warning about it
when copied to kiwibd? path is clearly unused in lfs3_emubd_createcfg,
but no warning...
---
Anyways, not sure which one is a better API, but we definitely don't
need two APIs, so eeny meeny miny moe...
Went ahead and chose the lfs3_emubd_create path param for some
consistency with filebd.
Long story short: %zd != %jd!
This was a simple oversight when writing the bench printing code, and
easy to miss on x86_64 and other modern PCs, but the mistake becomes
very apparent when trying to bench under qemu in thumb mode!
We're not currently using these (at the moment it's unclear if the
original intention behind the treediff algorithms is worth pursuing),
and they are showing up in our heap benchmarks.
The good news is that means our heap benchmarks are working.
Also saves a bit of code/ctx in bmap mode:
code stack ctx
before: 37024 2352 684
after: 37024 (+0.0%) 2352 (+0.0%) 684 (+0.0%)
code stack ctx
bmap before: 38752 2456 812
bmap after: 38704 (-0.1%) 2456 (+0.0%) 800 (-1.5%)
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%)
Note --list-suite-paths was already skipping case-less suites! I think
only -Y/--summary was an outlier.
This is consistent with test.py's matching of suite ids when no cases
are found (test_runner itself doesn't really care, it just reports no
matching cases). Though we do still compile case-less suites and include
them in the test_suites array, which may be confusing in the future.
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.
So we now keep blocks around until they can be replaced with a single
fragment. This is simpler, cheaper, and reduces the number of commits
needed to graft (though note arbitrary range removals still keep this
unbounded).
---
So, this is a delicate tradeoff.
On one hand, not fully fragmenting blocks risks keeping around bptrs
containing very little data, depending on fragment_size.
On the other hand:
- It's expensive, and disk utilization during random _deletes_ is not
the biggest of concerns.
Note our crystallization algorithm should still clean up partial
blocks _eventually_, so this doesn't really impact random writes.
The main concerns are lfs3_file_truncate/fruncate, and in the future
collapserange/punchhole.
- Fragmenting bptrs introduces more commits, which have their own
prog/erase cost, and it's unclear how this impacts logging operations.
There's no point in fragmenting blocks at the head of a log if we're
going to fruncate them eventually.
I figure lets err on minimizing complexity/code size for now, and if
this turns out to be a mistake, we can always revert or introduce
fragmenting >1 fragment blocks as an optional feature in the future.
---
Saves a big chunk of code, stack, and even some ctx (no more
fragment_thresh):
code stack ctx
before: 37504 2448 656
after: 37024 (-1.3%) 2416 (-1.3%) 652 (-0.6%)
This one was a bit more involved.
Removes utils that are no longer useful, and made sure some of the
name/API changes over time are adopted consistently:
- lfs_npw2 -> lfs_nlog2
- lfs_tole32_ -> lfs_tole32
- lfs_fromle32_ -> lfs_fromle32
Also did another pass for lfs_ prefixes on mem/str functions. The habit
to use the naked variants of these is hard to break!
This prevents runaway O(n^2) behavior on devices with extremely large
block sizes (NAND, bs=~128KiB - ~1MiB).
The whole point of shrubs is to avoid this O(n^2) runaway when inline
files become necessarily large. Setting FRAGMENT_SIZE to a factor of the
BLOCK_SIZE humorously defeats this.
The 512 byte cutoff is somewhat arbitrary, it's the natural BLOCK_SIZE/8
FRAGMENT_SIZE on most NOR flash (bs=4096), but it's probably worth
tuning based on actual device performance.
So now crystal_thresh only controls when fragments are compacted into
blocks, while fragment_thresh controls when blocks are broken into
fragments. Setting fragment_thresh=-1 will follow crystal_thresh and
keeps the previous behavior.
These were already two separate pieces of logic, so it makes sense to
provide two separate knobs for tuning.
Setting fragment_thresh lower than crystal_thresh has some potential to
reduce hysteresis in cases where random writes push blocks close to
crystal_thresh. It will be interesting to explore this more when
benchmarking.
---
The additional config option adds a bit of code/ctx, but hopefully that
will go away in the future config rework:
code stack ctx
before: 35584 2480 636
after: 35600 (+0.0%) 2480 (+0.0%) 640 (+0.6%)
And the related config options:
- cfg->file_buffer_size -> cfg->file_cache_size
- file->cfg->buffer_size -> file->cfg->cache_size
- file->cfg->buffer -> file->cfg->cache_buffer
The original motivation to rename this to file->buffer was to better
align with what other filesystems call this, but I think this is a case
where internal consistency is more important than external consistency.
file->cache better matches lfs->pcache and lfs->rcache, and makes it
easier to read code involving both file->cache and other user-provided
buffers.
Keeping the upstream name also helps with continuity.
While I think shrub_size is probably the more correct name at a
technical level, inline_size is probably more what users expect and
doesn't require a deeper understanding of filesystem details.
The only risk is that users may think inline_size has no effect on large
files, when in fact it still controls how much of the btree root can be
inlined.
There's also the point that sticking with inline_size maintains
compatibility with both the upstream version and any future version that
has other file representations.
May revisit this, but renaming to lfs->cfg->inline_size for now.
Now that we no longer have bmoss files, inline_size and shrub_size are
effectively the same thing.
We weren't using this, so no code change, but it does save a word of
ctx:
code stack ctx
before: 36280 2576 640
after: 36280 (+0.0%) 2576 (+0.0%) 636 (-0.6%)
Incremental gc, being stateful and not gc-able (ironic), was always
going to need to be conditionally compilable.
This moves incremental gc behind the LFS_GC define, so that we can focus
on the "default" costs. This cuts lfs_t in nearly half!
lfs_t with LFS_GC: 308
lfs_t without LFS_C: 168 (-45.5%)
This does save less code than one might expect though. We still need
most of the internal traversal/gc logic for things like block allocation
and orphan cleanup, so most of the savings is limited to the RAM storing
the incremental state:
code stack ctx
before: 37916 2608 768
after with LFS_CFG: 37944 (+0.1%) 2608 (+0.0%) 768 (+0.0%)
after without LFS_CFG: 37796 (-0.3%) 2608 (+0.0%) 620 (-19.3%)
On the flip side, this does mean most of the incremental gc
functionality is still availables in the lfsr_traversal_t APIs.
Applications with more advanced gc use-cases may actually benefit from
_not_ enabling the incremental gc APIs, and instead use the
lfsr_traversal_t APIs directly.
Before:
int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags);
After:
int lfsr_gc(lfs_t *lfs);
int lfsr_gc_setflags(lfs_t *lfs, uint32_t flags);
int lfsr_gc_setsteps(lfs_t *lfs, lfs_soff_t steps);
---
The interesting thing about the lfsr_gc API is that the caller will
often be very different from whoever configures the system. One example
being an OS calling lfsr_gc in a background loop, while leaving
configuration up to the user.
The idea here, is instead of forcing the OS to come up with its own
stateful system to pass flags to lfsr_gc, we just embed this state in
littlefs directly. The whole point of lfsr_gc is that it's a stateful
system anyways.
Unfortunately this state does require a bit more logic to maintain,
which adds code/ctx cost:
code stack ctx
before: 37812 2608 752
after: 37916 (+0.3%) 2608 (+0.0%) 768 (+2.1%)
This was the one piece needed to be able to replace amor.py with csv.py.
The missing feature in csv.py is the ability to keep track of a
running-sum, but this is a bit of a hack in amor.py considering we
otherwise view csv entries as unordered.
We could add a running-sum to csv.py, or instead, just include a running
sum as a part of our bench output. We have all the information there
anyways, and if it simplifies the mess that is our csv scripts, that's a
win.
---
This also replaces the bench "meas", "iter", and "size" fields with the
slightly simpler "m" (measurement? metric?) and "n" fields. It's up to
the specific benchmark exactly how to interpret "n", but one field is
sufficient for existing scripts.
This moves all ckread-related logic behind the new opt-in compile-time
LFS_CKREADS flag. So in order to use ckreads you need to 1. define
LFS_CKREADS at compile time, and 2. pass LFS_M_CKREADS during
lfsr_mount.
This was always the plan since, even if ckreads worked perfectly, it
adds a significant amount of baggage (stack mostly) to track the
ck context of all reads.
---
This is the first non-trivial opt-in define in littlefs, so more test
framework features!
test.py and build.py now support the optional ifdef attribute, which
makes it easy to indicate a test suite/case should not be compiled when
a feature is missing.
Also interesting to note is the addition of LFS_IFDEF_CKREADS, which
solves several issues (and general ugliness) related to #ifdefs in
expression. For example:
// does not compile :( (can't embed ifdefs in macros)
LFS_ASSERT(flags == (
LFS_M_CKPROGS
#ifdef LFS_CKREADS
| LFS_M_CKREADS
#endif
))
// does compile :)
LFS_ASSERT(flags == (
LFS_M_CKPROGS
| LFS_IFDEF_CKREADS(LFS_M_CKREADS, 0)));
---
This brings us way back down to our pre-ckread levels of code/stack:
code stack
before-ckreads: 36352 2672
ckreads: 38060 (+4.7%) 3056 (+14.4%)
after-ckreads: 36428 (+0.2%) 2680 (+0.3%)
Unfortunately, we do end up with a bit more code cost than where we
started. Mainly due to code moving around to support the ckread
infrastructure:
code stack
lfsr_bd_readtag: +52 (+23.2%) +8 (+10.0%)
lfsr_rbyd_fetch: +36 (+5.0%) +8 (+6.2%, cold)
lfs_toleb128: -12 (-25.0%) -4 (-20.0%, cold)
total: +76 (+0.2%) +8 (+0.3%)
But oh well. Note that some of these changes are good even without
ckreads, such as only parsing the last ecksum tag.
So instead of configuring gc_steps at mount time (or eventually compile
time), lfsr_fs_gc now takes a steps parameter that controls how much gc
work to attempt:
int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags);
This API was needed internally to better deduplicate on-mount gc, and I
figured it might also be useful for users to be able to easily change
gc_steps per lfsr_fs_gc call.
I realize this could also be accomplished with the theoretical
lfsr_fs_gccfg, but it's a bit easier to not need a struct every call.
Most likely, depending on project/system, users will always call
lfsr_fs_gc with either 1 (minimal work) or -1 (maximal work), or, worst
case, can define a system-wide GC_STEPS somewhere.
---
Deduplicating on-mount gc work better saved some code, though it's worth
noting this could have been done internally and not exposed to users:
code stack
before: 36476 2680 (+0.0%)
after: 36316 (-0.4%) 2680 (+0.0%)
This has been a long-time coming, mount flags are just too useful for
configuring a filesystem at runtime.
Currently this is limited to LFS_M_RDONLY and LFS_M_CKPROGS, but there
are a few more planned in the future:
LFS_M_RDWR = 0x0000, // Mount the filesystem as read and write
LFS_M_RDONLY = 0x0001, // Mount the filesystem as readonly
LFS_M_STRICT* = 0x0002, // Error if on-disk config does not match
LFS_M_FORCE* = 0x0004, // Ignore compat flags, mount readonly
LFS_M_FORCEWITHRECKLESSABANDON*
= 0x0008, // Ignore compat flags, mount read write
LFS_M_CKPROGS = 0x0010, // Check progs by reading back progged data
LFS_M_CKREADS* = 0x0020, // Check reads via checksums
* Hypothetical
As a convenience, we also return mount flags in the struct lfs_fsinfo's
flags field as their relevant LFS_I_* variants. Though only to match
statvfs, and only because it's cheap, littlefs's API is low-level and we
should expect users to know what flags they passed to lfsr_mount.
As for the new mount flags:
- LFS_M_RDONLY - For consistency with existing APIs, this just asserts
on write operations, which makes it a bit useless... But the info flag
LFS_I_RDONLY may be useful for falling back to a readonly mode if
we encounter on-disk compat issues.
At least if implement the theoretical LFS_UNTRUSTED_USER mode
LFS_M_RDONLY could become a runtime error.
- LFS_M_RDWR - This really just exists to compliment LFS_M_RDONLY and to
match LFS_O_RDONLY/LFS_O_RDWR. It's just an alias for 0, and I don't
think there will ever be a reason to make it non-0 (but I can always
be wrong!).
- LFS_M_CKPROGS - This replaces the check_progs config option and avoids
using a full byte to store a bool.
We should probably also have a compile-time option to compile this out
(LFS_NO_CKPROGS?), but that's a future thing to do.
This ended up adding a surprising bit of code, considering we're just
moving flags around, and noise in lfs_alloc added a bit of stack again:
code stack
before: 35880 2672
after: 35932 (+0.1%) 2680 (+0.3%)
Thinking about use case a bit, most lfsr_fs_gc will be to perform
background work, and can benefit from being incremental.
We already support incremental gc and all the mess associated with
traversal invalidation via the traversal API, so we might as well expose
this through lfsr_fs_gc.
The main downside is that we need to store an lfsr_traversal_t object
somewhere, which is not exactly a cheap struct. I was originally
considering limiting incremental gc to the traversal API for this
reason, but I think the value add of an incremental lfsr_fs_gc is too
compelling... Though we really should add a compile-time option
(LFS_NO_GC? LFS_NO_INCRGC?) to allow users to opt-out of this RAM cost
if they're never going to call this function.
Oh, and lfs_t also becomes self-referential, which might become a
problem for higher-level language users...
---
The incremental behavior of lfsr_fs_gc can be controlled by the new
gc_steps config option. This allows more than one step to be performed
at a time, which may allow for more progress when intermixed with
write-heavy filesystem operations. Setting gc_steps=-1 performs a full
traversal every call, which guarantees always making some amount of
progress.
This adds a bit of code, since we now need to check for/resume existing
traversals. But the real cost is the added RAM to lfs_t, which is
unfortunately wasted if you never call lfsr_fs_gc:
code stack lfs_t
before: 35708 2672 164
after: 35756 (+0.1%) 2672 (+0.0%) 296 (+80.5%)
lfs_fs_gc is still not reimplemented, but this is accessible through the
traversal API with LFS_T_COMPACT.
This is also the first traversal operation that can mutate the
filesystem, which brings its own set of problems:
- We need to set LFS_F_DIRTY in lfsr_mtree_gc now, which really
highlights how much of a mess having two flag fields is...
We do _not_ clobber in this case, since we assume lfsr_mtree_gc knows
what it's doing.
- We can now commit to an mroot in the mroot chain outside of the normal
mroot chain update logic.
This is a bit scary, but should just work.
The only issue so far is that we need to allow mdirs to follow the
mroot during mroot splits if mid=-1, even if they aren't lfs_t's mroot
mdir.
This should now be decently tested with the new
test_traversal_compact_* tests.
- It's easy for mtraversal's mdir and mtinfo's mdir to fall out of sync
when mutating... Why do we have two of these?
The actual compaction itself is pretty straightforward: just mark as
unerased, eoff=-1, and call lfsr_mdir_commit with an empty commit. This
is now wrapped up in lfsr_mdir_compact.
Code changes:
code stack
before: 34528 2640
after: 34652 (+0.4%) 2640 (+0.0%)
Though the real hard part will be implementing gc_compact_thresh over
btree nodes...
These emulate powerloss behavior where only some of the bits being
progged are actually progged if there is a powerloss. This behavior was
the original motivation for our ecksums/fcrcs, so it's good to have this
tested.
As a simplification, these only test the extremes:
- LFS_EMUBD_POWERLOSS_SOMEBITS => one bit progged
- LFS_EMUBD_POWERLOSS_MOSTBITS => all-but-one bit progged
Also they flips bits instead of preserving exact partial prog behavior,
but this is allowed (progs can have any intermediate value), has the
same effect as partial progs, and should encourage failed progs.
This required a number of tweaks in emubd: moved powerloss before prog,
moved mutate after powerloss, etc, but these shouldn't affect other
powerloss behaviors. Handling powerloss after prog was only to avoid
power_cycles=1 being useless, it's not strictly required.
Good news is testing so far suggests our ecksum design is sound.
More information upstream (f2a6f45, fc2aa33, 7873d81), but this adds
LFS_EMUBD_POWERLOS_OOO for testing out-of-order block devices that
require sync to be called for things to serialize. It's a simple
implementation, just reverts the first write since last sync on
powerloss, but gets the job done.
Cherry-picking these changes required reverting emubd's scratch buffer,
but carrying around an extra ~block_size of memory isn't a big deal
here.
This configuration option enables the previous behavior of reading back
every prog to check that the data was written correctly.
Unfortunately, this brings a bit of baggage, thanks to our cache
interactions being more complicated now:
- We really want to reuse the rcache for prog validation, despite the
cache performance implications. Unfortunately, we simply can't, thanks
to the new bd utility functions tying up the rcache. lfsr_bd_cpy, for
example, does not expect rcache to be invalidated between a read and
prog, and if it is, things break (I may or may not have found this by
experience).
These bd utilities are valuable, so we really need some other way to
validate our progs.
- Since we can't rely on the rcache, this leaves checksumming as the
only option for validating progs. Checksumming isn't perfect, as there
is a decent chance of false negatives, but to be honest it's probably
good enough for anything that's not malicious.
- This also adds the new constraint that we need to be able to read back
any prog into the pcache, which implies read_size <= prog_size. This
constraint didn't exist when we could clobber our rcache, but this is
not worth throwing away the new bd utilities. Not to mention
clobbering our rcache could hurt cache performance.
Why not make read_size <= prog_size conditional on check_progs?
The main reason is convenience. One very compelling use case for
check_progs is to help debug unknown filesystem/integration failures,
buf if you can't enable check_progs without changing the filesystem
configuration, you can't really rely on check_progs for debugging.
This helps future proof what we expect from block devices, in case
future error detection/correction mechanisms can benefit from our
prog_size always being readable.
Code changes were not that significant, however there was a surprising
stack cost. This seems to be because lfsr_bd_read__ can now be called
from multiple places, causing it to no longer be inlined in
lfsr_bd_read_, costing a bit of stack for the additional function call:
before: 33566 2624
after: 33682 (+0.3%) 2640 (+0.6%)
These really shouldn't be used all that often. Test filters are usually
used to protect against invalid test configurations, so if you bypass
test filters, expect things to fail!
But some filters just prevent test cases from taking too long. In these
cases being able to manually bypass the filter is useful for debugging/
benchmarking/etc...
This acts as a marker to indicate a fuzz test. It should reference a
define, usually SEED, that can be randomized to get interesting test
permutations.
This is currently unused, but could lead to some interesting uses such
as time-based fuzz testing. It's also just useful for inspecting the
tests (make test-list).
Christopher Haster