lfsr_file_resync discards the current working state of a file and
reverts it to the contents on disk. It also clears the desynced flag
from files, so provides an alternative to lfsr_file_sync for when you
don't want to write to the filesystem:
disk=A file=A disk=A file=A
| write B | write B
v v
disk=A file=B disk=A file=B
| sync | resync
v v
disk=B file=B disk=A file=A
The main motivation for this is to provide a way to mark desynced
readonly files as in-sync, without putting them into a weird state where
they are "in-sync" but don't match disk.
It's also a bit safer if the file is desynced due to an error, since
errors aren't currently guaranteed to leave file data in a defined
state. Needed to resync to recover from errors avoids accidentally
syncing partial writes.
This exact behavior can also be accomplished by closing+opening the
file, but lfsr_file_resync makes it much easier without _that_ much
extra code. It may even pay for itself if you consider what code it
saves on the user's side of things.
I considered naming this lfsr_file_discard because I think it sounds
cooler, but I figured including sync in the name provides a stronger
hint that it affects the file's desync status.
---
You may think it's not possible for a readonly file to become
out-of-sync from disk, since it's, well, readonly. But it is possible
thanks to desynced files ignoring other sync broadcasts.
Consider what happens if you open a file readonly, and write+sync the
file with another file handle at the same time:
disk=A f1=A f2=A
| desync f2
v
disk=A f1=A f2=A
| write f1=B
v
disk=A f1=B f2=A
| sync f1
v
disk=B f1=B f2=A <-- f2 is out-of-sync without any writes
---
This commit also changes lfsr_file_sync/flush to assert if the file is
readonly. Previously we allowed lfsr_file_sync to be called on readonly
files if it would be a noop, but lfsr_file_resync makes this
unnecessary.
More code means more code, but I think it is well worth it for the
additional flexibility:
code stack
before: 36412 2616
after: 36748 (+0.9%) 2616 (+0.0%)
This is mainly to solve the weird check-hole where passing CKPROGS/
CKREADS as mount flags has no effect on lfsr_format (I mean, it'd be a
bit silly if it did somehow):
LFS_F_RDWR 0 // Format the filesystem as read and write
LFS_F_CKPROGS 0x00000010 // Check progs by reading back progged data
LFS_F_CKREADS 0x00000020 // Check reads via parity bits/checksums
This makes lfsr_format a more cumbersome interface, but I don't know if
this is necessarily a bad thing. There's always risk of data loss when
calling lfsr_format, so maybe it should be a pain to call.
At the very least, format flags may be useful in the future for
enabling/disabling format-time things such as the planned block-map,
parity-tree, etc. Though it's unclear if such significant settings
should be format flags or somehow encoded as fields in our config
struct.
---
The LFS_F_* format flags of course ended up conflicting with our
internal LFS_F_* flags, so I renamed most of the internal flags to match
the closest flag set they participate in:
- LFS_F_TYPE -> LFS_O_TYPE
- LFS_F_UNFLUSH -> LFS_O_UNFLUSH
- LFS_F_UNSYNC -> LFS_O_UNSYNC
- LFS_F_ORPHAN -> LFS_O_ORPHAN
- LFS_F_ZOMBIE -> LFS_O_ZOMBIE
- LFS_F_ORPHANS -> LFS_I_ORPHANS
- LFS_F_UNCOMPACTED -> LFS_I_UNCOMPACTED
- LFS_F_TSTATE -> LFS_T_TSTATE
- LFS_F_BTYPE -> LFS_T_BTYPE
- LFS_F_DIRTY -> LFS_T_DIRTY
- LFS_F_MUTATED -> LFS_T_MUTATED
This may make it a bit less clear which flags are a part of the public
API, vs intended only for internal use, but at the very least our asserts
in format/mount/open/etc should catch most of these mistakes.
---
Code cost ended up being pretty minimal. Actually negative. This is the
second time we're _adding_ a feature that somehow saves code, though the
reality for this one is we're really just pushing constants up into the
user's stack frame. Still, it's a good indication the cost of format
flags is small:
code stack
before: 36452 2680
after: 36448 (-0.0%) 2680 (+0.0%)
These simply imply LFS_O_FLUSH/SYNC on all open writable files.
LFS_M_SYNC is equivalent to MS_SYNCHRONOUS in Linux/etc, while
LFS_M_FLUSH is just provided for consistency.
As pure conveniences, these may seem a bit out of scope for littlefs,
except they are _very_ cheap:
code stack
before: 36356 2664
after: 36356 (+0.0%) 2664 (+0.0%)
Ok, they're not _completely_ free! It just turns out they cost 8 bytes,
and a bit of simplification around flag checking in lfsr_mount saved
8 bytes:
code stack
before: 36356 2664
m_flush/sync: 36364 (+0.0%) 2664 (+0.0%)
mount-no-mask: 36356 (+0.0%) 2664 (+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%)
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