- BENCH_SIMTIME() => lfs3_kiwibd_simtime()
- BENCH_SIMRESET() => lfs3_kiwibd_simreset()
- BENCH_SIMPAUSE() => lfs3_kiwibd_simpause()
- BENCH_SIMRESUME() => lfs3_kiwibd_simresume()
- BENCH_RESET() => lfs3_kiwibd_simreset() + BENCH_STACK/HEAP_RESET()
- BENCH_PAUSE() => lfs3_kiwibd_simpause() + BENCH_STACK/HEAP_PAUSE()
- BENCH_RESUME() => lfs3_kiwibd_simresume() + BENCH_STACK/HEAP_RESUME()
This does two things:
1. Adds pause/resume counters to bd counters to make it easier to
exclude operations from the current bench (potentially useful for
seq+disk usage).
2. Exposes bd simtime operations as BENCH_* macros, to make it a bit
easier to interact with simtime without tying all the benches to
kiwibd. (Not that we'll ever probably not use kiwibd, but still).
Also adopted 32-bit counters for stack/heap pause state instead of a
32-bit stack. Not that either are at a risk of overflowing, but better
safe than sorry.
The point of having separate test/bench runners is to minimize
complexity when different concerns overlap, and the stack/heap
measurements haven't proven necessary for testing yet.
Keeping them around just adds a maintenance burden, and risks messy
interactions with test features if you ever try to turn them on
(heap + powerloss = memory leaks yay).
So removing for now.
If they are useful in the future (cheaper Valgrind-esque checks?),
copying from bench_runner.c -> test_runner.c is super easy.
---
Note these are still available and enabled by default in the bench
runner.
The litmus benches are really only intended for introspection/debugging/
cool plots/etc. They're interesting to poke around with and cover a wide
range of littlefs's data-structures, but are not very rigorous.
To make this more clear for new users, added a new litmus flag for
benches:
litmus = true
This doesn't change anything about how the bench is run, but serves as a
marker to hint that the bench is intended for non-rigorous benchmarking.
---
In the makefile, litmus tests are disabled by default at runtime
(--no-litmus). This is to limit `make bench` to benches that are useful
for performance comparisons.
With --no-litmus at runtime, the litmus benches are at least compiled
into the bench_runner, which should hopefully encourage keeping them up
to date with code changes. Eventually we should also run them in CI, but
only to check for runtime errors.
Unlike our tests, we're not really worried about compile time at the
moment due to how few/small our benches are.
Logging is one of those things that's very useful to keep around, but
has a high-risk of stack/heap costs that shouldn't count towards any
benchmarks (you can always disable logging).
So, lets exclude them from stack/heap measurements.
This could've been done by defining all of littlefs's LFS3_DEBUG/INFO/
WARN/ERROR macros, but intercepting printf directly is a bit less
tedious. As a plus, we eliminate logging costs from any other filesystem
we benchmark, without need to fiddle with everyone's logging APIs.
---
Hmm. Actually, now that I've done a test run, these changes seem to have
no effect.
Which makes sense in hindsight:
1. For efficiencies sake, printf likely tries to allocate infrequently.
Maybe only during the first call?
And we print the bench id before entering the bench.
2. The way our stack measurements work, we only count them if we enter a
bd op or call BENCH_STACK_PAUSE().
So any printfs encountered previously would have been ignored by our
stack measurements.
Still, better safe than sorry.
- Renamed BENCH_STACK/HEAP -> BENCH_STACK/HEAP_WATERMARK
- Renamed BENCH_YES_STACK/HEAP -> BENCH_STACK/HEAP
- Tweaked stack/heap watermarks to hopefully be easier to access when
debugging. Now also exposed as global variables
(bench_stack/heap_watermark).
I considered changing BENCH_STACK/HEAP_WATERMARK to be the variable
itself, to be consistent with TEST_PLS, but decided against it:
1. BENCH_STACK_CURRENT() is a bit magic in that it relies on
__attribute__((noinline)) to force a new stack frame. This wouldn't
really be possible with a variable.
2. TEST_PLS is at least constant from the _current run_'s perspective.
This isn't true for the stack/heap watermarks.
- Reworked internals a bit to hopefully be simpler
--no-internal has already proven useful for skipping internal tests for
refactoring, so it makes sense to add --no-reentrant/fuzz flags as well.
--no-fuzz seems particularly useful for when you want to skip the less
targeted fuzz tests:
- with fuzz tests: 634616/634616 passed, in 1239.90s
- with --no-fuzz: 85434/85434 passed, in 423.41s
I also added runtime variants to test/bench_runner and test/bench.py.
These may be useful to skip tests without needing to recompile the
runner.
---
Also tweaked -s/--step to filter permutations in any --list-* flags, for
consistency.
The main changes:
- Added TEST_DEFINES and BENCH_DEFINES to allow overriding the default
test/bench define header:
-DTEST_DEFINES=my_test_defines.h
Note these are VERY different from LFS_DEFINES upstream. They aren't a
typical header file, and are included multiple times with various
query macros.
It's hacky, but works surprisingly well.
Or maybe I'll just do anything to avoid having to write multiline
macros. Ugh, backslashes.
- Moved more logic into bench/test_defines.h, including everything
needed to integrate other filesystems out-of-tree.
This mostly meant moving all of the cfg initialization logic into its
own query macro (replacing the BENCH/TEST_CFG field macros).
But this also includes a bunch of small tweaks encountered while trying
to get external benchmarks running again.
The external benchmarks include several other filesystems (littlefs2,
SPIFFS, Yaffs2), and I'm hoping this injectable/queryable header thing
will do a good job at avoiding a maintenance headache. (At least a
better job than forking bench_runner.c, which was the previous
solution.)
These have been battle-tested in external benchmarks, and have proven
useful for finding a runtime estimate on stack+heap usage.
Of course, to be realistic they need to be cross-compiled and run under
QEMU (which does work!), but even on x86_64 they provide a nice insight
into RAM usage. In practice the only real difference is pointer width
anyways.
---
Enabled by default for the bench runner, these are available if
TEST/BENCH_YES_HEAP and/or TEST/BENCH_YES_STACK are defined.
(This default is provided by the Makefile. At least heap measurements
rely on linker flags, so it probably doesn't make sense to default
enable in the bench runner itself.)
Stack vs heap rely on slightly different mechanisms:
- Stack: Uses GCC's __builtin_frame_address(0) to measure the current
stack usage on entry to every bd operation.
- Heap: Relies on GCC's -Wl,--wrap flags to intercept every malloc/free
call, to track the current heap usage.
These are available via bench/test macros:
- BENCH_STACK() - Maximum stack usage of the current run
- BENCH_STACK_CURRENT() - Current stack usage
- BENCH_HEAP() - Maximum heap usage of the current run
- BENCH_HEAP_CURRENT() - Current heap usage
Note BENCH_STACK_CURRENT() can be useful for separating out the bench's
ctx from total stack usage, similarly to our static analysis.
---
One surprising outcome is that these heap hooks trivially implement a
memory leak detector. Maybe that could be useful in the test_runner as a
cheaper alternative to Valgrind?
kiwibd has been used extensively in external benchmarks, it makes sense
to make it the default bd for the bench runner:
- test_runner - defaults to emubd - more testing features
- bench_runner - defaults to kiwibd - lighter-weight disks
The benefit of kiwibd is the disk is just one big blob of RAM, so
basically no overhead. This is important when benchmarking on multi-GiB
disks.
emubd is much heavy, but as a tradeoff can do quite a bit more:
bad-block simulation, wear simulation, snapshotting, etc.
---
In theory the bd used by each runner can be controlled at compile-time
by defining -DBENCH_EMUBD, etc, but I have a feeling no one will ever
use this.
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.
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.
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.
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.
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 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.
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.
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).
The main star of the show is the adoption of __builtin_trap() for
aborting on assert failure. I discovered this GCC/Clang extension
recently and it integrates much, _much_ better with GDB.
With stdlib's abort(), GDB drops you off in several layers of internal
stdlib functions, which is a pain to navigate out of to get to where the
assert actually happened. With __builtin_trap(), GDB stops immediately,
making debugging quick and easy.
This is great! The pain of debugging needs to come from understanding
the error, not just getting to it.
---
Also tweaked a few things with the internal print functions to make
reading the generated source easier, though I realize this is a rare
thing to do.
Motivation:
- Debuggability. Accessing the current test/bench defines from inside
gdb was basically impossible for some dumb macro-debug-info reason I
can't figure out.
In theory, GCC provides a .debug_macro section when compiled with -g3.
I can see this section with objdump --dwarf=macro, but somehow gdb
can't seem to find any definitions? I'm guess the #line source
remapping is causing things to break somehow...
Though even if macro-debugging gets fixed, which would be valuable,
accessing defines in the current test/bench runner can trigger quite
a bit of hidden machinery. This risks side-effects, which is never
great when debugging.
All of this is quite annoying because the test/bench defines is
usually the most important piece of information when debugging!
This replaces the previous hidden define machinery with simple global
variables, which gdb can access no problem.
- Also when debugging we no longer awkwardly step into the test_define
function all the time!
- In theory, global variables, being a simple memory access, should be
quite a bit faster than the hidden define machinery. This does matter
because running tests _is_ a dev bottleneck.
In practice though, any performance benefit is below the noise floor,
which isn't too surprising (~630s +-~20s).
- Using global variables for defines simplifies the test/bench runner
quite a bit.
Though some of the previous complexity was due to a whole internal
define caching system, which was supposed to lazily evaluate test
defines to avoid evaluating defines we don't use. This all proved to
be useless because the first thing we do when running each test is
evaluate all defines to generate the test id (lol).
So now, instead of lazily evaluating and caching defines, we just
generate global variables during compilation and evaluate all defines
for each test permutation immediately before running.
This relies heavily on __attribute__((weak)) symbols, and lets the
linker really shine.
As a funny perk this also effectively interns all test/bench defines by
the address of the resulting global variable. So we don't even need to
do string comparisons when mapping suite-level defines to the
runner-level defines.
---
Perhaps the more interesting thing to note, is the change in strategy in
how we actually evaluate the test defines.
This ends up being a surprisingly tricky problem, due to the potential
of mutual recursion between our defines.
Previously, because our define machinery was lazy, we could just
evaluate each define on demand. If a define required another define, it
would lazily trigger another evaluation, implicitly recursing through
C's stack. If cyclic, this would eventually lead to a stack overflow,
but that's ok because it's a user error to let this happen.
The "correct" way, at least in terms of being computationally optimal,
would be to topologically sort the defines and evaluate the resulting
tree from the leaves up.
But I ain't got time for that, so the solution here is equal parts
hacky, simple, and effective.
Basically, we just evaluate the defines repeatedly until they stop
changing:
- Initially, mutually recursive defines may read the uninitialized
values of their dependencies, and end up with some arbitrarily wrong
result. But as the defines are repeatedly evaluated, assuming no
cycles, the correct results should eventually bubble up the tree until
all defines converge to the correct value.
- This is O(n*e) vs O(n+e), but our define graph is usually quite
shallow.
- To prevent non-halting, we error after an arbitrary 1000 iterations.
If you hit this, it's likely because there is a cycle in the define
graph.
This is runtime configurable via the new --define-depth flag.
- To keep things consistent and reproducible, we zero initialize all
defines before the first evaluation.
I don't think this is strictly necessary, but it's important for the
test runner to have the exact same results on every run. No one wants
a "works on my machine" situation when the tests are involved.
Experimentation shows we only need an evaluation depth of 2 to
successfully evaluate the current set of defines:
$ ./runners/test_runner --list-defines --define-depth=2
And any performance impact is negligible (~630s +-~20s).
So:
switch (cond) {
case 0:;
// first case
break;
case 1:;
// second case
break;
default:;
// default case
break;
}
This basically adopts our current label style for the case statements in
switch statements. It initially looks like quite a monstrosity, but I
think it does a good job at highlighting that case statements in C are
no safer than labels and gotos.
I would not use this style in a language with better scoping in switch
statements.
I'd prefer not to use switch statements, their scoping rules in C are
just too error-prone, and the compiler usually optimizes things out
anyways, but there are some places where switch statements are clearly
the correct organization -- state machines such as lfsr_traversal_read
for example.
If you're curious about the ':;' ending, this is used in our current
style for labels to avoid "declaration is not a statement" warnings.
Which I think is just a bit of leftover from C historically not having
mixed statements/declarations.
So:
x = (cond) ? yes : no;
Where there are always parentheses around the condition, even if not
required for disambiguity. Additional parentheses are always allowed,
but the parenthesized condition helps signal that a ternary operator is
coming earlier in the expression.
This style has grown on me as I think it helps code readability. It
reminds me of the required parentheses for if/while statements.
Might as well adopt codebase-wide.
The previous encoding was a bit problematic with our linear and log
heuristics, which can grow thousands of powerlosses deep. You know you
have a problem when you're copying a test id that spans a dozen lines.
It also meant we were spending O(n^2) time just encoding powerloss ids:
before: 942.68s
after: 921.94s (-2.2%)
This new encoding takes advantage of the unused characters in our leb16
encoding, with an 'x' prefix indicating linear-heuristic powerlosses
and a 'y' prefix indicating log-heuristic powerlosses ('w' is used for
negative leb16s).
Before:
- explicit: 42q2q2
- linear: 123456789abcdefg1h1i1j1k1l1m1n1o1p1q1r1s1t1u1v1
- log: 1248g1g2g4g8gg1gg2gg4gg8
After:
- explicit: 42q2q2
- linear: xg2
- log: yc
This turned out to not be all that useful.
Tests already take quite a bit to run, which is a good thing! We have a
lot of tests! 942.68s or ~15 minutes of tests at the time of writing to
be exact. But simply multiplying the number of tests by some number of
geometries is heavy handed and not a great use of testing time.
Instead, tests where different geometries are relevant can parameterize
READ_SIZE/PROG_SIZE/BLOCK_SIZE at the suite level where needed. The
geometry system was just another define parameterization layer anyways.
Testing different geometries can still be done in CI by overriding the
relevant defines anyways, and it _might_ be interesting there.
Like many prngs, xorshift breaks down when the internal state is 0. The
common fix is to explicitly check for this and replace with a 1 when
this happens (usually when seeding, in this API we have to check every
update, this is less efficient but I don't think we really care).
As a slight tweak, this now checks for 0 but replaces it with -1. This
makes seed=0 different from seed=1, which is nice when using
seed=range(0,n) in tests/benches.
So now instead of needing:
./scripts/test.py ./runners/test_runner test_dtree
You can just do:
./scripts/test.py test_dtree
Or with an explicit path:
./scripts/test.py -R./runners/test_runner test_dtree
This makes it easier to run the script manually. And, while there may be
some hiccups with the implicit relative path, I think in general this will
make the test/bench scripts easier to use.
There was already an implicit runner path, though only if the test suite
was completely omitted. I'm not sure that would ever have actually
been useful...
---
Also increased the permutation field size in --list-*, since I noticed it
was overflowing.
This turned out to have limited use for the tests themselves. I was
hoping to avoid the mount->format->mount fallback when powerloss
testing, but we still need it in case format was interrupted.
Still, TEST_PLS is very useful for debugging.
Previouly it was difficult to set a breakpoint at a specific location,
and after a specific powerloss event. Now all you need is this in gdb:
b <line> if test_pls == <pls>
- coalesce_size - The amount of data allowed to coalesce into single
data entries.
- crystallize_size - How much data is allowed to be written to btree
inner nodes before needing to be compacted into a block.
Also deduplicated the test config is something I've been wanting to do
for a while. It doesn't make sense to need to modify several different
instantiations of lfs_config every time a config option is added or
removed...
Currently limited to inlined files and only simpler truncate-writes.
But still this lets us test file creation/deletion.
This is also enough logic to make it clear that, even though we have
some powerful high-level primitives, mapping file operations onto these
is still going to be non-trivial.
The previous system of relying on test name prefixes for ordering was
simple, but organizing tests by dependencies and topologically sorting
during compilation is 1. more flexible and 2. simplifies test names,
which get typed a lot.
Note these are not "hard" dependencies, each test suite should work fine
in isolation. These "after" dependencies just hint an ordering when all
tests are ran.
As such, it's worth noting the tests should NOT error of a dependency is
missing. This unfortunately makes it a bit hard to catch typos, but
allows faster compilation of a subset of tests.
---
To make this work the way tests are linked has changed from using custom
linker section (fun linker magic!) to a weakly linked array appended to
every source file (also fun linker magic!).
At least with this method test.py has strict control over the test
ordering, and doesn't depend on 1. the order in which the linker merges
sections, and 2. the order tests are passed to test.py. I didn't realize
the previous system was so fragile.
To help with this, added TEST_PL, which is set to true when powerloss
testing. This way tests can check for stronger conditions (no EEXIST)
when not powerloss testing.
With TEST_PL, there's really no reason every test in t5_dirs shouldn't
be reentrant, and this gives us a huge improvement of test coverage very
cheaply.
---
The increased test coverage caught a bug, which is that gstate wasn't
being consumed properly when mtree uninlining. Humorously, this went
unnoticed because the most common form of mtree uninlining, mdir splitting,
ended up incorrectly consuming the gstate twice, which canceled itself
out since the consume operation is basically just xor.
Also added support for printing dstarts to dbglfs.py, to help debugging.
This marks internal tests/benches (case.in="lfs.c") with an otherwise-unused
flag that is printed during --summary/--list-*. This just helps identify which
tests/benches are internal.
TEST_PERMUTATION/BENCH_PERMUTATION make it possible to map an integer to
a specific permutation efficiently. This is helpful since our testing
framework really only parameterizes single integers.
The exact implementation took a bit of trial and error. It's based on
https://stackoverflow.com/a/7919887 and
https://stackoverflow.com/a/24257996, but modified to run in O(n) with
no extra memory. In the discussion it seemed like this may not actually
be possible for lexicographic ordering of permutations, but fortunately
we don't care about the specific ordering, only the reproducibility.
Here's how it works:
1. First populate an array with all numbers 0-n.
2. Iterate through each index, selecting only from the remaining
numbers based on our current permutation.
.- i%rem --.
v .----+----.
[p0 p1 |-> r0 r1 r2 r3]
Normally to maintain lexicographic ordering you should have to do a O(n)
shift at this step as you remove each number. But instead we can just swap
the removed number and number under the index. This effectively
shrinks the remaining part of the array, but permutes the numbers
a bit. Fortunately, since each successive permutation swaps
at the same location, the resulting permutations will be both
exhaustive and reproducible, if unintuitive.
Now permutation/fuzz tests can reproduce specific failures by defining
either -DPERMUTATION=x or -DSEED=x.
I wondered if walking in Python 2's footsteps was going to run into the
same issues and sure enough, memory backed iterators became unweildy.
The motivation for this change is that large ranges in tests, such as
iterators over seeds or permutations, became prohibitively expensive to
compile. This meant more iteration moving into tests with more steps to
reproduce failures. This sort of defeats the purpuse of the test
framework.
The solution here is to move test permutation generation out of test.py
and into the test runner itself. The allows defines to generate their
values programmatically.
This does conflict with the test frameworks support of sets of explicit
permutations, but this is fixed by also moving these "permutation sets"
down into the test runner.
I guess it turns out the closer your representation matches your
implementation the better everythign works.
Additionally the define caching layer got a bit of tweaking. We can't
precalculate the defines because of mutual recursion, but we can
precalculate which define/permutation each define id maps to. This is
necessary as otherwise figuring out each define's define-specific
permutation would be prohibitively expensive.
This turned out to be a bit tricky, and the scheme in bench_rbyd is
broken.
The core issue is that we don't have a distinction between physical and
logical block sizes, so we can't use a block device configured for one
geometry with a littlefs instance operating on a different geometry. For
this and other reasons we should probably have two configuration
variables in the future, but at the moment that is out of scope.
The problem with the approach in bench_rbyd, which changes the
lfs_config at runtime, is that this breaks emubd which also depends on
lfs_config due to a leaky abstraction. This causes unnoticed memory
corruption.
---
To get something working, the tests now change the underlying BLOCK_SIZE
test define before the tests are run. This starts the test with a block
device configured with a large block_size. To keep this from breaking
things the geometry definitions in the test and bench runners no longer
use default dependent definitions, instead defining everything
explicitly.
With block_size being so large, this makes some of the emubd operations
less performant, notably the --disk option for exposing block device
state during testing.
It would also be nice to use the copy-on-write backend of emubd for some
of the permutation testing, but since it operates on a block-by-block
basis, it doesn't really work when the block device is just one big
block.
- Added support for negative numbers in the leb16 encoding with an
optional 'w' prefix.
- Changed prettyasserts.py rule to .a.c => .c, allowing other .a.c files
in the future.
- Updated .gitignore with missing generated files (tags, .csv).
- Removed suite-namespacing of test symbols, these are no longer needed.
- Changed test define overrides to have higher priority than explicit
defines encoded in test ids. So:
./runners/bench_runner bench_dir_open:0f1g12gg2b8c8dgg4e0 -DREAD_SIZE=16
Behaves as expected.
Otherwise it's not easy to experiment with known failing test cases.
- Fixed issue where the -b flag ignored explicit test/bench ids.
When you add a function to every benchmark suite, you know if should
probably be provided by the benchmark runner itself. That being said,
randomness in tests/benchmarks is a bit tricky because it needs to be
strictly controlled and reproducible.
No global state is used, allowing tests/benches to maintain multiple
randomness stream which can be useful for checking results during a run.
There's an argument for having global prng state in that the prng could
be preserved across power-loss, but I have yet to see a use for this,
and it would add a significant requirement to any future test/bench runner.
The linear powerloss heuristic provides very good powerloss coverage
without a significant runtime hit, so there's really no reason to run
the tests without -Plinear.
Previous behavior can be accomplished with an explicit -Pnone.
Two flags introduced: -fcallgraph-info=su for stack analysis, and
-ftrack-macro-expansions=0 for cleaner prettyassert.py warnings, are
unfortunately not supported in Clang.
The override vars in the Makefile meant it wasn't actually possible to
remove these flags for Clang testing, so this commit changes those vars
to normal, non-overriding vars. This means `make CFLAGS=-Werror` and
`CFLAGS=-Werror make` behave _very_ differently, but this is just an
unfortunate quirk of make that needs to be worked around.
- Moved to Ubuntu 22.04
This notably means we no longer have to bend over backwards to
install GCC 10!
- Changed shell in gha to include the verbose/undefined flags, making
debugging gha a bit less painful
- Adopted the new test.py/test_runners framework, which means no more
heavy recompilation for different configurations. This reduces the test job
runtime from >1 hour to ~15 minutes, while increasing the number of
geometries we are testing.
- Added exhaustive powerloss testing, because of time constraints this
is at most 1pls for general tests, 2pls for a subset of useful tests.
- Limited coverage measurements to `make test`
Originally I tried to maximize coverage numbers by including coverage
from every possible source, including the more elaborate CI jobs which
provide an extra level of fuzzing.
But this missed the purpose of coverage measurements, which is to find
areas where test cases can be improved. We don't want to improve coverage
by just shoving more fuzz tests into CI, we want to improve coverage by
adding specific, intentioned test cases, that, if they fail, highlight
the reason for the failure.
With this perspective, maximizing coverage measurement in CI is
counter-productive. This changes makes it so the reported coverage is
always less than actual CI coverage, but acts as a more useful metric.
This also simplifies coverage collection, so that's an extra plus.
- Added benchmarks to CI
Note this doesn't suffer from inconsistent CPU performance because our
benchmarks are based on purely simulated read/prog/erase measurements.
- Updated the generated markdown table to include line+branch coverage
info and benchmark results.
This happens in rare situations where there is a failed mdir relocation,
interrupted by a power-loss, containing the destination of a directory
rename operation, where the directory being renamed preceded the
relocating mdir in the mdir tail-list. This requires at some point for a
previous directory rename to create a cycle.
If this happens, it's possible for the half-orphan to contain the only
reference to the renamed directory. Since half-orphans contain outdated
state when viewed through the mdir tail-list, the renamed directory
appears to be a full-orphan until we fix the relocating half-orphan.
This causes littlefs to incorrectly remove the renamed directory from
the mdir tail-list, causes catastrophic problems down the line.
The source of the problem is that the two different types of orphans
really operate on two different levels of abstraction: half-orphans fix
failed mdir commits, while full-orphans fix directory removes/renames.
Conflating the two leads to situations where we attempt to fix assumed
problems about the directory tree before we have fixed problems with the
mdir state.
The fix here is to separate out the deorphan search into two passes: one
to fix half-orphans and correct any mdir-commits, restoring the mdirs
and gstate to a known good state, then two to fix failed
removes/renames.
---
This was found with the -Plinear heuristic powerloss testing, which now
runs on more geometries. The failing case was:
test_relocations_reentrant_renames:112gg261dk1e3f3:123456789abcdefg1h1i1j1k1
l1m1n1o1p1q1r1s1t1u1v1g2h2i2j2k2l2m2n2o2p2q2r2s2t2
Also fixed/tweaked some parts of the test framework as a part of finding
this bug:
- Fixed off-by-one in exhaustive powerloss state encoding.
- Added --gdb-powerloss-before and --gdb-powerloss-after to help debug
state changes through a failing powerloss, maybe this should be
expanded to any arbitrary powerloss number in the future.
- Added lfs_emubd_crc and lfs_emubd_bdcrc to get block/bd crcs for quick
state comparisons while debugging.
- Fixed bd read/prog/erase counts not being copied during exhaustive
powerloss testing.
- Fixed small typo in lfs_emubd trace.
- Fixed prettyasserts.py parsing when '->' is in expr
- Made prettyasserts.py failures not crash (yay dynamic typing)
- Fixed the initial state of the emubd disk file to match the internal
state in RAM
- Fixed true/false getting changed to True/False in test.py/bench.py
defines
- Fixed accidental substring matching in plot.py's --by comparison
- Fixed a missed LFS_BLOCk_CYCLES in test_superblocks.toml that was
missed
- Changed test.py/bench.py -v to only show commands being run
Including the test output is still possible with test.py -v -O-, making
the implicit inclusion redundant and noisy.
- Added license comments to bench_runner/test_runner
Based loosely on Linux's perf tool, perfbd.py uses trace output with
backtraces to aggregate and show the block device usage of all functions
in a program, propagating block devices operation cost up the backtrace
for each operation.
This combined with --trace-period and --trace-freq for
sampling/filtering trace events allow the bench-runner to very
efficiently record the general cost of block device operations with very
little overhead.
Adopted this as the default side-effect of make bench, replacing
cycle-based performance measurements which are less important for
littlefs.
Christopher Haster