We don't actually need these, all we need are utils defined for the
largest integer size we operate on, currently uint32_t.
Counterintuitively this should make it easier to adopt different integer
widths in the future.
Or maybe this will bite us when lfs_off_t >> lfs_size_t? Oh well, if
that's the case we can fix it then.
No code changes:
code stack
before: 33886 2560
after: 33886 (+0.0%) 2560 (+0.0%)
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).
A much requested feature, this allows much finer control of how RAM is
allocated for the system.
It was difficult to introduce this in previous versions of littlefs due
to how we steal caches during certain file operations, but now we don't
do that and treat the caches much more transparently.
Managing separate cache sizes does add a bit of code, but this is well
worth the potential for RAM savings due to increased flexibility:
code stack
before: 33656 2632
after: 33714 (+0.2%) 2640 (+0.3%)
Also interesting to note this reduces alignment requirements for the
rcache/pcache, since they don't need to share alignment, and completely
removes any alignment requirement from the file buffers.
The main idea here is that diverse tests are better than many similar
tests.
Sure, if we throw fuzz tests at the system all day we'll eventually find
more bugs, but if a developer is in the loop that time is going to be
better spent writing specific tests targeting the fragile parts of the
system.
And don't worry, we can still throw fuzz tests at the system all day by
specifying explicit seeds with -DSEED=blah.
Changes:
- Limited dir-related powerloss fuzz testing to N <= 16.
These tests were the biggest culprit of excessive test runtime,
requiring O(n^2) redundant operations to recover from powerlosses
(they just replay the full sequence on powerloss).
- As a tradeoff, bumped most fuzz tests to a minimum of 20 seeds.
The big exception being the test_fwrite tests, which are heavily
parameterized and already take the most time to run. Each parameter
combination also multiplies the effective number of seeds, so
increasing the number of base seeds will probably have diminishing
returns.
- Limited test_fwrite_reversed to SIZE <= 4*1024*CHUNK.
Writing a file backwards is just about the worst way you could write a
file, since all buffering/coalescing expect writes to eventually make
forward progress. On the flip side, because it's uncommon, writing a
file backwards is also a great way to find bugs. But at some point a
compromise needs to be made.
Impacted test runtimes:
case otime ntime dtime
test_btree_push_fuzz 0.3 0.5 +0.2 (+60.2%)
test_btree_push_sparse_fuzz 0.4 3.3 +2.9 (+720.4%)
test_btree_update_fuzz 0.4 0.9 +0.6 (+141.6%)
test_btree_update_sparse_fuzz 0.5 4.5 +4.1 (+857.4%)
test_btree_pop_fuzz 0.6 2.3 +1.7 (+314.7%)
test_btree_pop_sparse_fuzz 1.2 5.7 +4.4 (+356.2%)
test_btree_split_fuzz 0.5 1.4 +0.8 (+150.2%)
test_btree_split_sparse_fuzz 0.4 5.6 +5.1 (+1163.2%)
test_btree_find_fuzz 0.5 0.7 +0.2 (+50.7%)
test_btree_find_sparse_fuzz 1.0 3.0 +2.0 (+189.8%)
test_btree_traversal_fuzz 0.6 2.3 +1.6 (+260.4%)
test_dirs_mkdir_many 3.3 2.1 -1.3 (-37.8%)
test_dirs_mkdir_many_backwards 3.5 2.1 -1.4 (-39.9%)
test_dirs_mkdir_fuzz 115.3 106.4 -8.9 (-7.7%)
test_dirs_rm_many 283.9 76.8 -207.0 (-72.9%)
test_dirs_rm_many_backwards 216.1 80.6 -135.5 (-62.7%)
test_dirs_rm_fuzz 647.0 68.5 -578.5 (-89.4%)
test_dirs_mv_many 14.2 15.4 +1.1 (+7.9%)
test_dirs_mv_many_backwards 16.5 14.5 -2.1 (-12.5%)
test_dirs_mv_fuzz 1932.5 156.7 -1775.8 (-91.9%)
test_dirs_general_fuzz 561.9 74.5 -487.4 (-86.7%)
test_dread_recursive_rm 336.6 46.2 -290.4 (-86.3%)
test_dread_recursive_mv 55.5 44.6 -11.0 (-19.8%)
test_fsync_rrrr_fuzz 0.4 0.3 -0.1 (-18.4%)
test_fsync_wrrr_fuzz 8.0 12.4 +4.5 (+56.0%)
test_fsync_wwww_fuzz 13.2 33.4 +20.2 (+152.6%)
test_fsync_wwrr_fuzz 5.4 50.9 +45.5 (+841.6%)
test_fsync_rwrw_fuzz 2.4 8.4 +6.0 (+253.9%)
test_fsync_rwrw_sparse_fuzz 3.2 7.5 +4.2 (+129.9%)
test_fsync_rwtfrwtf_sparse_fuzz 6.1 8.5 +2.4 (+39.3%)
test_fsync_drrr_fuzz 11.8 9.2 -2.6 (-21.8%)
test_fsync_wddd_fuzz 9.3 11.9 +2.6 (+28.0%)
test_fsync_rwdrwd_fuzz 1.6 33.1 +31.5 (+1963.4%)
test_fsync_rwdrwd_sparse_fuzz 0.3 1.8 +1.4 (+418.8%)
test_fsync_rwtfdrwtfd_sparse_fuzz 0.3 1.1 +0.8 (+260.2%)
test_fwrite_reversed 728.5 345.2 -383.3 (-52.6%)
TOTAL 7587.5 3792.3 -3795.2 (-50.0%)
- Fixed fsync tests, which needed more lfsr_file_sync calls so multiple
file handles can be opened correctly.
Though this points out there's no way to open a rdonly file on an
uncreated file until sync is called... But I guess you wouldn't be
able to recieve broadcasts until sync anyways? at which point the file
would be created?
- Update mtree tests based on the new remove behavior for regular files.
Before this changed the mid to -1, now it points to the next mid with
the zombie flag set. Upper layers use this to migrate mdirs to a
scratch file if necessary.
- Removed the orphaned mdir test. We don't create orphaned mdirs
anymore.
Technically, orphaned mdirs are currently possible if we lose power in
the middle of the mtree update, but this is a bug and should be fixed
(previous revisions did not have this issue).
This is a compromise on consistency and not breaking expected
invariants.
The problem: rdonly files can become unsynced:
1. file is opened rdonly + desync
2. the same file is opened and written to
3. we try to sync our original file handle
What we want:
1. sync should ensure disk + files are in-sync
2. rdonly implies sync should not write to disk
Without desync, and in other systems, this is not a problem, because
rdonly files can never become unsynced.
But with desync, a state (albiet a roundabout one) can be reached where
we can't satisfy both of these invariants.
I wanted to just assert on syncing a rdonly file, but this is supported
on POSIX and other systems, and it makes sense that you would want to
unconditionally call sync in certain circumstances (ensuring close can't
write to disk for example).
So adopts the approach of allowing flush and sync on rdonly files when
possible, and when not possible, sync simply returns LFS_ERR_INVAL and
makes it the user's problem.
For the above example, this has the side effect of making the rdonly file
desync again, so close can complete without touching disk.
As a plus, a desynced rdonly file can now be used to test if a file has
been written to. Though I'm not sure when this would be useful... Or
if it's a good idea to suggest this use of the API...
Reading more into POSIX, it seems that most of the write functions do
have special behavior built into what would implicitly be a noop.
It's difficult to find, since it usually doesn't matter, but consider
the m_time field. The following operations do _not_ update m_time:
- write when size=0
- truncate when size does not change
- fruncate when size does not change
I think it's safe to extend these to sync broadcasts in littlefs, and
only guarantee sync broadcasts when the file state has changed (even
though that may mean other file handles may remain out-of-date!).
In this interpretation, the "write operations" described in POSIX more
mean the implicit write operations effected by write/truncate/fruncate.
That being said, it's not clear what the best approach is, desync files
make this all a bit more muddled... This may also be reverted.
This is an extension of the noop-sync after unrelated write-sync after
desync corner case:
op a state b state
in-sync in-sync
desync(b) in-sync desync
write(a) unsync desync
sync(a) in-sync' desync
sync(b) in-sync in-sync
But instead of explicitly calling lfsr_file_sync, what if you implicitly
triggered sync through something like a write on a file with the
LFS_O_SYNC flag, but not a normal write, a noop write, write(0)?
If the definition of LFS_O_SYNC is taken literally as "lfsr_file_write
and friends implicitly call lfsr_file_sync after every call", then this
should behave just as if lfsr_file_sync had been called, and
unconditionally broadcast the sync. Since this is the simplest
interpretation, I think this is what we should implement.
Added tests, and adopted this behavior. Fortunately this just involves
some small gotos (https://xkcd.com/292):
code stack
before: 33020 2976
after: 33026 (+0.0%) 2976 (+0.0%)
There are a number of nuanced cases to watch out for when mixing sync,
desync, and "noop syncs" (sync when no write operation has occured):
1. Noop-sync after unrelated write:
op a state b state
in-sync in-sync
write(a) unsync in-sync
sync(b) in-sync in-sync
In this case, a should be clobbered by b when b syncs. But this
gets tricky since b is still up to date with the disk, so b's
unsynced flag is not set.
The solution here is to just unconditionally broadcast all sync
operations irregardless of on-disk state. This is all in-device
anyways, so it shouldn't really add any overhead.
2. Noop-sync after unrelated write-sync after desync:
op a state b state
in-sync in-sync
desync(b) in-sync desync
write(a) unsync desync
sync(a) in-sync' desync
sync(b) in-sync in-sync
In this case, a should again be clobbered by b, even though a is
in-sync with the disk. This is not tricky because of a's state, but
because b doesn't know it is no longer in-sync with the disk.
The solution here is to set the unsynced flag on all desynced files
when an unrelated file is synced. This way, b knows it needs to
update disk if sync is called. We already scan all opened files to
update in-sync files, so this has very little cost.
3. Readonly-sync after unrelated write-sync after desync?
This is basically the same as 2., but involves a readonly file:
op a state b state (rdonly)
in-sync in-sync
desync(b) in-sync desync
write(a) unsync desync
sync(a) in-sync' desync
sync(b) ??? in-sync
In this case, I have no idea what should happen.
I would guess the least surprising result would be for b to write
its contents to a/disk? Bringing everything in-sync?
But this implies that b, a readonly file, should write to disk.
This isn't the only place a read operation would result in a write.
RDWR files, for example, can flush buffers during a file read. But at
least there, the file is open RDWR, not strictly RDONLY.
It seems like writing during sync on a readonly file breaks some sort
of invariant users expect.
But the alternative: Dropping the current state of b in favor of a's
state, is inconsistent with sync on WRONLY/RDWR files, and seems like
it breaks some sort of invariant about sync modifying the current
file's state...
Given this situation, I think the best course of action is to just
disallow sync on readonly files. It is now an assert.
There is some precedent for this, upstream we already omit sync when
compiled in LFS_READONLY mode. Though this does deviate from POSIX
behavior...
Worst case, by asserting, this leaves us free to introduce different
readonly-sync behavior in the future without breaking backwards
compatibility.
---
Maybe there should be some sort of lfsr_file_resync function to
discard current changes? Though this can be done with a close+open
cycle, so I think the value would be low.
Added tests over these cases and fixed where they broke, except for 3.,
lfsr_file_sync and lfsr_file_flush get asserts now to prevent their use
on readonly files.
Also added a couple more specific tests to cover cases I was concerned
about.
This should avoid confusion between "multiple handles" and "multiple
files" (name undecided) test suites.
It also fits well because this suite really is just testing nuanced
sync/desync behavior.
Christopher Haster