- mdir_bits -> mbits
- lfsr_mid_bid -> lfsr_mbid
- lfsr_mid_rid -> lfsr_mrid
These now match the naming in the dbg scripts.
I feel like this is more terse in a way that is also more readable, but
maybe that's just me.
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.
This does a couple things:
- Makes attr-lists a bit more self-documenting.
- Adds a bit more type-safety. The LFSR_RATTR_* macros should be able to
reject types that don't match the expected encoding.
- Makes it easier to adjust dsize estimates at one location.
Specifically, this makes it harder to forget bptr's LFSR_BPTR_DSIZE.
---
Surprisingly this did have a small impact on code size. I'm not entirely
sure why, but considering how much of the codebase this touches I'm just
going to chalk this up to compiler noise:
code stack ctx
before: 35488 2440 636
after: 35536 (+0.1%) 2440 (+0.0%) 636 (+0.0%)
lfsr_file_carve seems the hardest hit:
function (0 added, 0 removed) osize nsize dsize
lfsr_file_open 16 20 +4 (+25.0%)
lfsr_file_carve 1316 1356 +40 (+3.0%)
lfsr_remove 408 412 +4 (+1.0%)
TOTAL 35488 35536 +48 (+0.1%)
Mainly just for self-documentation reasons.
This may also make it easier to add LFSR_RATTR_BUF-specific asserts/
tweaks/etc, and helps future refactoring.
But functionally LFSR_RATTR_BUF is equivalent to LFSR_RATTR for now.
No code changes.
With the new internal LFSR_RATTR API, there's really no reason to keep
these around.
At one point these were useful for both the implicit lvalues and
automatic buffer size, but GCC's problems with compound-literals and
code size made them almost always backfire.
Now, they're mostly obsolete thanks to the new LFSR_RATTR_* macros.
We do still have a couple LFSR_DATA_* macros (LFSR_DATA_BUF,
LFSR_DATA_SLICE, etc), but these are a bit more fundamental to the
lfsr_data_t type.
This finishes the eager -> lazy attr encoding rework.
Which makes it a good time to look at the total savings from adopting
lazy attr encoding, though there's still a bit of tinkering to do (eager
branches, cksum tags, etc):
code stack ctx
before lazy-attrs: 36280 2576 636
after lazy-attrs: 35592 (-1.9%) 2472 (-4.0%) 636 (+0.0%)
A ~free 688 byte savings in code and 104 bytes in stack is not bad.
This fully adopts LFSR_RATTR__ and friends:
- LFSR_RATTR -> LFSR_RATTR__ or LFSR_RATTR_DATA__
- LFSR_RATTR_BUF -> LFSR_RATTR__
- LFSR_RATTR_CAT -> LFSR_RATTR_CAT__
- LFSR_RATTR_NOOP -> LFSR_RATTR_NOOP__
- LFSR_RATTR_NAME -> LFSR_RATTR_NAME__
Note the new LFSR_RATTR__ macro also lets us a drop the special rattr
macros, at the cost of a bit less type safety:
- LFSR_RATTR_RATTRS -> LFSR_RATTR__
- LFSR_RATTR_MOVE -> LFSR_RATTR__
- LFSR_RATTR_GRM -> LFSR_RATTR__ (we weren't using this?)
- LFSR_RATTR_SHRUBCOMMIT -> LFSR_RATTR__
Curiously, this ended up adding ~88 bytes to lfsr_file_carve:
function (0 added, 0 removed) osize nsize dsize
lfsr_file_carve 1228 1316 +88 (+7.2%)
lfsr_mdir_commit 2144 2152 +8 (+0.4%)
lfsr_mdir_commit__ 1192 1188 -4 (-0.3%)
lfsr_file_truncate 184 182 -2 (-1.1%)
lfsr_mount 98 96 -2 (-2.0%)
TOTAL 35508 35596 +88 (+0.2%)
I'm really not sure why, all I can think of is maybe the change from a
forced-inline function to a macro added a bunch of compiler noise?
Still, 80 bytes is not worth two competing LFSR_RATTR APIs. Though
it may be worth looking into this in the future.
Total code changes:
code stack ctx
before: 35508 2472 636
after: 35596 (+0.2%) 2472 (+0.0%) 636 (+0.0%)
This one is interesting in that we don't just encode to a buffer, but
need to express the concatenation of did + name somehow. Fortunately we
can still leverage the cat circuitry by setting data_count=-2:
- LFSR_TAG_NAME -+-> cat(fromleb128(did), name)
- LFSR_TAG_REG -+
- LFSR_TAG_DIR -+
- LFSR_TAG_STICKYNOTE -'
This does break our shrub estimate for name attrs (currently names have
no technical limit), which would be an issue, but we just happen to never
commit names to shrubs.
In theory you _could_ accurately estimate name attrs if you limited
names to <=(2^15)-5, but I figured this wouldn't be worth the extra code
cost in LFSR_RATTR_NAME__... Especially since it would just go unused...
Saves a nice bit of code, though no stack since we currently don't
allocate any names on the hot-path (lfsr_file_truncate):
code stack ctx
before: 35580 2440 636
after: 35472 (-0.3%) 2440 (+0.0%) 636 (+0.0%)
Note this does _not_ include LFSR_TAG_BOOKMARK, which only contains the
did and can avoid a stack allocation if encoded as a single leb128 attr.
Though breaking up the LFSR_TAG_NAME types does risk a more complicated
switch-case-table...
- LFSR_TAG_GEOMETRY ---> lfsr_data_fromgeometry
Not much to say about this one, LFSR_TAG_GEOMETRY is a bit of an
outlier.
I did consider deduplicating with the mptr encoder, but decided that
would be too hacky, and create problems for future metadata redundancy
things.
Still saves code though, which is nice:
code stack ctx
before: 35632 2440 636
after: 35580 (-0.1%) 2440 (+0.0%) 636 (+0.0%)
- LFSR_TAG_RCOMPAT -+-> lfsr_data_fromle32
- LFSR_TAG_WCOMPAT -+
- LFSR_TAG_OCOMPAT -+
- LFSR_TAG_GCKSUMDELTA -'
- LFSR_TAG_NAMELIMIT -+-> lfsr_data_fromleb128
- LFSR_TAG_FILELIMIT -+
- LFSR_TAG_BOOKMARK -+
- LFSR_TAG_DID -'
This is nice mainly from an internal API standpoint. Single le32/leb128
attrs should be pretty lightweight, and it's nice for the API to reflect
that.
With a bit of tinkering with the internal lfsr_rattr_t type, we can even
pass these directly in the lfsr_rattr_t struct itself, so no need to
keep single le32/leb128 attrs on the stack:
buffer rattr: cat attr: le32/leb128 attr:
.---+---+---+---. .. .---+---+---+---. .. .---+---+---+---.
| tag |0|size | | tag |1|count| | tag |0|dsize|
+---+---+---+---+ +---+---+---+---+ +---+---+---+---+
| weight | | weight | | weight |
+---+---+---+---+ .. +---+---+---+---+ .. +---+---+---+---+
| ptr -------. | ptr -------. | le32/leb128 |
'---+---+---+---' | '---+---+---+---' | '---+---+---+---'
.---+---+---+---. | .---+---+---+---. |
| data |<' |mm| size |<'
: : : +---+---+---+---+
| data |
+ +
| |
+---+---+---+---+
|mm| size |
: : :
While tinkering I also ended up renaming a couple things:
- rattr.cat -> rattr.u.datas, rattr.u.buffer, rattr.u.etc
- rattr.count -> rattr.data_count
- added lfsr_rattr_dtag for ignoring on-disk/explicit-data tags
- lfsr_rattr_size -> lfsr_rattr_dsize
Surprisingly very little code savings though. I guess we don't use
single le32/leb128 attrs enough to overcome the added complexity to
lfsr_rbyd_appendrattr_'s switch-case-table?
code stack ctx
before: 35636 2440 636
after: 35632 (-0.0%) 2440 (+0.0%) 636 (+0.0%)
That or there's something else weird going on with this union and
compiler assumptions. Attempting to adopt .u.etc in LFSR_RATTR__ alone
adds ~100 bytes of code, even though both .u.etc and .u.cat are the same
type (const void *)...
Not entirely sure what's going on...
The idea here is to move as much attr encoding logic as possible into
lfsr_rbyd_appendrattr_, so we don't encode most attrs until the last
minute, right before we write the tag+data to disk.
This has some pretty big theoretical benefits:
- Deduplicates encoding logic, so most attrs will only have a single
lfsr_data_from* call in the entire system.
This saves code size used for function calls, stack allocations, etc.
- In theory, _significantly_ better stack usage.
The main downside with eager encoding is that we need a buffer to
hold the encoding, and this buffer needs to stay allocated while all
of the commit machinery does its work.
This ends up stacking when any low-level attr buffers in
lfsr_btree_commit/lfsr_mdir_commit/etc, even though we don't _really_
need all of these attrs encoded at the same time.
Heck, we don't even need all of the attrs in the same _commit_ to be
encoded at the same time.
Lazily encoding avoids all of this.
- It's actually a nicer internal API, and means less risk we lose/
misallocate one of the encoding buffers.
The main downside is this makes attr encodings less gc-able. However, so
far it seems like you need most tags the moment you try to write to the
filesystem, and unwanted code costs can be worked around by allowing
more code to be conditionally compiled-out (at a testing cost).
This also means we don't know the actual on-disk attr size until we're
writing attrs out to disk. Fortunately, we've ended up relying on attr
size less than I thought we would. We still need it for shrub estimates,
but we can use the worst-case encoding size (LFSR_BPTR_DSIZE) there.
---
To start, this adopts lazy attr encoding for most of the obvious/
less-involved attrs:
- LFSR_TAG_BSHRUB ---> lfsr_data_fromshrub
- LFSR_TAG_BTREE -+-> lfsr_data_frombtree
- LFSR_TAG_MTREE -'
- LFSR_TAG_MROOT -+-> lfsr_data_frommptr
- LFSR_TAG_MDIR -'
- LFSR_TAG_ECKSUM ---> lfsr_data_fromecksum
Of interesting note is LFSR_TAG_BSHRUB. These changes actually make
shrub trunk encoding less of a special case, which _must_ be lazily
encoded due to last minute shrub changes caused by mdir compactions,
relocations, etc. This lets us drop the unique LFSR_TAG_SHRUBTRUNK
handling.
Though it does risk bugs if a future refactor ever reverts to eager
encoding... I've tried to highlight this with comments around
LFSR_TAG_BSHRUB's encoding.
These changes also required moving a significant number of the
LFSR_*_DSIZE macros around so they are declared before
lfsr_rbyd_appendrattr_. This is unfortunate as it moves them farther
away from from the related lfsr_data_from* implementations, but as far
as I'm aware there's no way around this.
We also need to _not_ lazily encode when an attr is in the concatenated-
data form (count < 0), or else this breaks mdir compaction. This has the
interesting side-effect of still allowing eager encoding with
LFSR_DATA_BUF, which, while less efficient, is very useful for our
tests.
---
So far, code/stack changes look promising:
code stack ctx
before: 36280 2576 636
after: 35848 (-1.2%) 2504 (-2.8%) 636 (+0.0%)
This is the correct name for our rbyd attr type, even if it requires a
bit more typing.
lfsr_attr_t would be a better name, but that conflicts with our
user-facing attrs.
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%)
Bptrs really are a file concept, despite the name (bptr =>
block-pointer). Other bshrubs/btrees do not have bptrs.
Returning decoded bptrs from lfsr_bshrub_lookupnext and friends was a
bit of a hack to make bsprouts (single bptrs) work, but now that we
don't support bsprouts, we don't need this hack anymore.
To avoid code duplication, this does reroute mtree traversal through
lfsr_file_traverse_. Which is a bit weird, but not the worst thing this
codebase has ever done.
Code changes:
code stack ctx
before: 36492 2608 640
after: 36460 (-0.1%) 2608 (+0.0%) 640 (+0.0%)
This makes a bit less sense than adopting lfsr_bshrub_t in lfsr_bshrub_*
functions, but it gives LFSR_TAG_SHRUBCOMMIT/SHRUBTRUNK direct access to
the staging shrub without needing the shrub + 1 hack.
The whole shrub vs bshrub distinction is already a bit broken anyways,
with us relying on the opened-mdir list to correctly stage all shrubs in
the filesystem.
---
Curiously, this again ends up with net negative impact on code cost:
code stack ctx
before: 36484 2608 640
after: 36492 (+0.0%) 2608 (+0.0%) 640 (+0.0%)
This moves all of the shrub tracking logic from lfsr_obshrub_t into
lfsr_bshrub_t, completely drops the lfsr_obshrub_t type, and changes all
lfsr_bshrub_* functions to take lfsr_bshrub_t instead of the mdir+shrub
pair.
This makes the lfsr_bshrub_* functions <-> lfsr_bshrub_t relationship
more consistent with other APIs, such as lfsr_btree_t:
- lfsr_bshrub_lookupnext(lfs, &file->o.o.mdir, &file->o.bshrub, ...)
+ lfsr_bshrub_lookupnext(lfs, &file->b, ...)
I think the reason why this design wasn't obvious before is because, at
least conceptually, having the lfsr_mdir_t live inside the lfsr_bshrub_t
is a bit weird. It's only thanks to lfsr_file_t invasively using the
internal lfsr_mdir_t that we can avoid duplicate lfsr_mdir_t objects.
This also reorganizes the structs in lfs.h a bit, and renames the
related file.o -> file.b fields (much needed because lfs->gc.t.o.o.mdir.
rbyd.blocks was starting to get _real_ confusing).
---
Unfortunately, reducing the number of arguments to lfsr_bshrub_*
functions did not save nearly as much code as I thought it would. It
even ended up with a net _increase_ of code, apparently due to needing
to recalculate the bshrub->shrub offset more often:
code stack ctx
before: 36476 2608 640
after: 36484 (+0.0%) 2608 (+0.0%) 640 (+0.0%)
Strange, but this rework is still worthwhile if only for the code
readability.
The lfsr_shrub_t/lfsr_btree_t union was _technically_ not undefined
behavior, because the relevant fields were all a part of the "common
initial sequence", but collapsing these to the same type certainly does
simplify things.
The only weirdness is that we now store shrub.estimate in shrub.eoff.
We could add a union here, but the extra noise is just not worth the
slighty better name. The shrub.estimate is a sort of "simulated
shrub.eoff" anyways.
---
This makes it so all of these types alias to the same core lfsr_rbyd_t
type, which I suppose actually reflects the on-disk format quite well:
lfsr_shrub_t => lfsr_rbyd_t
lfsr_bshrub_t
lfsr_btree_t
Code cost more-or-less unaffected:
code stack ctx
before: 36432 2608 640
after: 36436 (+0.0%) 2608 (+0.0%) 640 (+0.0%)
This is a pretty suspicious looking test failure, considering the recent
changes to the mroot/mtree and related splitting logic, but it just
turned out to be a bug in the test logic.
Sort of. This loop is trying to create an mroot that will both compact
and split, but it doesn't check if the mdir was split prematurely, so it
just keeps adding files until we hit a true LFS_ERR_NOSPC condition:
if ((file1.o.o.mdir.rbyd.eoff & 0x7fffffff) > GC_COMPACT_THRESH
&& estimate > BLOCK_SIZE/2) {
break;
}
The solution is to make the filename size a bit smaller so we don't
split too early.
I also added some asserts to catch premature splits in case this happens
again. These tests are a bit delicate.
We had to be a bit clever with our lfsr_mtree_t representation to
support msprouts. Now that we don't support msprouts, we can simplify
this and drop the lfsr_mtree_t type completely! which is nice for both
code cost and readability.
Saves a bit more code:
code stack ctx
before: 38344 2624 640
after: 38284 (-0.2%) 2624 (+0.0%) 640 (+0.0%)
Which increases the total savings of dropping msprouts:
code stack ctx
yes msprouts: 38508 2624 640
no msprouts: 38284 (-0.6%) 2624 (+0.0%) 640 (+0.0%)
While we do check for out-of-bound tags in lfsr_bd_readtag, we were
ignoring the returned size in lfsr_rbyd_fetch when reading cksum tags.
This meant it was possible for lfsr_rbyd_fetch to try to read past the
end-of-block if:
1. The cksum tag was malformed with size < 4.
2. The malformed cksum tag was < 4 bytes from the end-of-block.
A pretty rare case! Considering we don't even bother writing cksum tags
when we're that close to the end-of-block. This can only happen in our
tests if existing garbage happens to look like a cksum tag.
While every cksum tag _should_ have at least 4 bytes for the cksum, we
can't guarantee that if we're parsing garbage.
Found by our test_ck_spam_dir_fuzz test.
---
I've also added a couple test_mtree_truncated_* tests to catch similar
truncation issues and prevent a regression in the future. We can't
really rely on test_ck_spam_* to always find nuanced errors like this,
but it's neat it found this one.
Code changes:
code stack ctx
before: 38340 2624 640
after: 38344 (+0.0%) 2624 (+0.0%) 640 (+0.0%)
littlefs is intentionally designed to not rely on noise, even with cksum
collisions (hello, perturb bit!). So it makes sense for this to be an
optional feature, even if it's a small one.
Disabling revision count noise by default also helps with testing. The
whole point of revision count noise is to make cksum collisions less
likely, which is a bit counterproductive when that's something we want
to test!
This doesn't really change the revision count encoding:
vvvvrrrr rrrrrrnn nnnnnnnn nnnnnnnn
'-.''----.----''---------.--------'
'------|---------------|---------- 4-bit relocation revision
'---------------|---------- recycle-bits recycle counter
'---------- pseudorandom noise (optional)
I considered moving the recycle-bits down when we're not adding noise,
but the extra logic just isn't worth making the revision count a bit
more human-readable.
---
This saves a small bit of code in the default build, at the cost of some
code for the runtime checks in the LFS_NOISY build. Though I'm hoping
future config work will let users opt-out of these runtime checks:
code stack ctx
before: 38548 2624 640
default after: 38508 (-0.1%) 2624 (+0.0%) 640 (+0.0%)
LFS_NOISY after: 38568 (+0.1%) 2624 (+0.0%) 640 (+0.0%)
Honestly the thing I'm more worried about is using one of our precious
mount flags for this... There's not that many bits left!
This just exposes the gcksum to the user, but exposing the gcksum allows
the user to store it externally for an extra layer of protection against
filesystem corruption.
As far as I'm aware this is the only real way to protect against global
rollback issues, which is a problem for any filesystem with logs (aka
any powerloss-resilient filesystem).
This required a comically small amount of code:
code stack ctx
before: 38492 2624 640
after: 38500 (+0.0%) 2624 (+0.0%) 640 (+0.0%)
We really shouldn't have two names for the same thing, it just makes
things more confusing, even if the public name doesn't quite match the
internal usage. Especially now that we internally rely on these being
the same flag.
This renames LFS_i_UNTIDY -> LFS_I_MKCONSISTENT and drops the untidy/
mktidy naming internally.
No code changes.
- In lfsr_mtree_traverse, we traverse open file bshrubs/btrees before
we validate the gcksum, which means bugs/asserts can slip through
before we have a chance to detect something is wrong.
To work around this, I've added an explicit mdir cksum check right
before we start traversing an open mdir's bshrubs/btrees. If an open
mdir doesn't match the on-disk state, the on-disk state must contain
an error (or the RAM, but that's a different story and wayyy out of
scope).
It might be better to rearrange lfsr_mtree_traverse to check gcksums
first, but this will require another look at our traversal clobbering
logic.
- For a similar reason, ckfetches can't detect open bshrub/btree
corruption as is. As its name suggests, ckfetches only checks fetches,
so any corruption after we've fetched bshrubs/btrees in lfsr_file_open
will go undetected.
Fortunately this just means we need a full ckmeta-scan in
test_ck_spam* tests that keep open files.
In real use, full ckmeta-scans should be preferred anyways. Limiting
these scans to mtreeonly was just an attempt to better stress btree
ckfetches.
At least we're still testing ckmeta+mtreeonly+ckfetches in
test_ck_spam_dir_fuzz and test_ck_spam_file_fuzz.
This gets the test_ck_spam* tests running under all of the current
interesting ck-modes.
Code changes:
code stack ctx
before: 38560 2640 644
after 38572 (+0.0%) 2640 (+0.0%) 644 (+0.0%)
Now that gcksums are working and we can detect rollback issues, it's
worth revisiting our most aggressive bit-error tests.
Unfortunately, I think due to focusing on ckprogs, these were a bit less
ready-to-go than I had hoped. We still have the read-hole, so the sort
of errors we can expect to detect is a bit limited.
Still, managed to come up with some schemes that I think are
interesting:
- ckprogs - Limited to catching bit-errors during progs, but these tests
work great.
- ckdata - Limited to manual bit-errors, but can detect both metdata +
data errors.
- ckmeta+ckfetches - Limited to manual bit-errors, ckmeta detects
mtree errors, while ckfetches detects btree + data errors.
- ckmeta+ckdatacksums - Limited to manual bit-errors, ckmeta detects
metadata errors, while ckdatacksums detects data errors.
To make testing manual bit-errors a bit easier, and to avoid
reimplementing the bit randomizer in emubd, I added
LFS_EMUBD_BADBLOCK_MANUAL and lfs_emubd_flip to let the tests manually
control when bits flip.
---
Unfortunately open files are proving to be an issue for these tests,
since we don't really expect corrupted metadata after lfsr_file_open (
assuming no read-hole).
For now I've limited these new ck-modes to the tests without open files,
but we should probably revisit this.
This adds a check that the on-disk gcksum matches the in-RAM gcksum
in lfsr_mtree_traverse, so ckmeta/ckdata scans should now be able to
at least detect global-rollback issues that occur while mounted.
This also moves the LFS_I_CKMETA/CKDATA flag clearing logic from
lfsr_mtree_gc -> lfsr_mtree_traverse. There's no reason to not clear
these flags if we've made a successful traversal. We weren't actually
calling lfsr_mtree_traverse with the right flags for this to matter, but
it does let us drop an explicit flag clear in lfsr_fs_ck.
---
These changes were a part of adding the harder versions of our ckmeta/
ckdata tests, where we flip individual bits instead of clobbering the
entire block. These are more realistic errors and stress our gcksum
system.
Recalculating the gcksum required another gcksum copy in
lfsr_traversal_t, which adds a bit of code and ctx to our incremental-gc
build:
code stack ctx
default before: 38428 2640 644
default after: 38560 (+0.3%) 2640 (+0.0%) 644 (+0.0%)
gc before: 38484 2640 788
gc after: 38616 (+0.3%) 2640 (+0.0%) 792 (+0.5%)
Unfortunately we can't easily abuse the copies in lfs_t since
multiple traversals may be open at once.
This was quite a puzzle.
The problem: How do we detect corrupt mdirs?
Seems like a simple question, but we can't just rely on mdir cksums. Our
mdirs are independently updateable logs, and logs have this annoying
tendency to "rollback" to previously valid states when corrupted.
Rollback issues aren't littlefs-specific, but what _is_ littlefs-
specific is that when one mdir rolls back, it can disagree with other
mdirs, resulting in wildly incorrect filesystem state.
To solve this, or at least protect against disagreeable mdirs, we need
to somehow include the state of all other mdirs in each mdir commit.
---
The first thought: Why not use gstate?
We already have a system for storing distributed state. If we add the
xor of all of our mdir cksums, we can rebuild it during mount and verify
that nothing changed:
.--------. .--------. .--------. .--------.
.| mdir 0 | .| mdir 1 | .| mdir 2 | .| mdir 3 |
|| | || | || | || |
|| gdelta | || gdelta | || gdelta | || gdelta |
|'-----|--' |'-----|--' |'-----|--' |'-----|--'
'------|-' '------|-' '------|-' '------|-'
'--.------' '--.------' '--.------' '--.------'
cksum | cksum | cksum | cksum |
| | v | v | v |
'---------> xor -------> xor -------> xor -------> gcksum
| v v v =?
'---------> xor -------> xor -------> xor ---> gcksum
Unfortunately it's not that easy. Consider what this looks like
mathematically (g is our gcksum, c_i is an mdir cksum, d_i is a
gcksumdelta, and +/-/sum is xor):
g = sum(c_i) = sum(d_i)
If we solve for a new gcksumdelta, d_i:
d_i = g' - g
d_i = g + c_i - g
d_i = c_i
The gcksum cancels itself out! We're left with an equation that depends
only on the current mdir, which doesn't help us at all.
Next thought: What if we permute the gcksum with a function t before
distributing it over our gcksumdeltas?
.--------. .--------. .--------. .--------.
.| mdir 0 | .| mdir 1 | .| mdir 2 | .| mdir 3 |
|| | || | || | || |
|| gdelta | || gdelta | || gdelta | || gdelta |
|'-----|--' |'-----|--' |'-----|--' |'-----|--'
'------|-' '------|-' '------|-' '------|-'
'--.------' '--.------' '--.------' '--.------'
cksum | cksum | cksum | cksum |
| | v | v | v |
'---------> xor -------> xor -------> xor -------> gcksum
| | | | .--t--'
| | | | '-> t(gcksum)
| v v v =?
'---------> xor -------> xor -------> xor ---> t(gcksum)
In math terms:
t(g) = t(sum(c_i)) = sum(d_i)
In order for this to work, t needs to be non-linear. If t is linear, the
same thing happens:
d_i = t(g') - t(g)
d_i = t(g + c_i) - t(g)
d_i = t(g) + t(c_i) - t(g)
d_i = t(c_i)
This was quite funny/frustrating (funnistrating?) during development,
because it means a lot of seemingly obvious functions don't work!
- t(g) = g - Doesn't work
- t(g) = crc32c(g) - Doesn't work because crc32cs are linear
- t(g) = g^2 in GF(2^n) - g^2 is linear in GF(2^n)!?
Fortunately, powers coprime with 2 finally give us a non-linear function
in GF(2^n), so t(g) = g^3 works:
d_i = g'^3 - g^3
d_i = (g + c_i)^3 - g^3
d_i = (g^2 + gc_i + gc_i + c_i^2)(g + c_i) - g^3
d_i = (g^2 + c_i^2)(g + c_i) - g^3
d_i = g^3 + gc_i^2 + g^2c_i + c_i^3 - g^3
d_i = gc_i^2 + g^2c_i + c_i^3
---
Bleh, now we need to implement finite-field operations? Well, not
entirely!
Note that our algorithm never uses division. This means we don't need a
full finite-field (+, -, *, /), but can get away with a finite-ring (+,
-, *). And conveniently for us, our crc32c polynomial defines a ring
epimorphic to a 31-bit finite-field.
All we need to do is define crc32c multiplication as polynomial
multiplication mod our crc32c polynomial:
crc32cmul(a, b) = pmod(pmul(a, b), P)
And since crc32c is more-or-less just pmod(x, P), this lets us take
advantage of any crc32c hardware/tables that may be available.
---
Bunch of notes:
- Our 2^n-bit crc-ring maps to a 2^n-1-bit finite-field because our crc
polynomial is defined as P(x) = Q(x)(x + 1), where Q(x) is a 2^n-1-bit
irreducible polynomial.
This is a common crc construction as it provides optimal odd-bit/2-bit
error detection, so it shouldn't be too difficult to adapt to other
crc sizes.
- t(g) = g^3 is not the only function that works, but it turns out to be
a pretty good one:
- 3 and 2^(2^n-1)-1 are coprime, which means our function t(g) = g^3
provides a one-to-one mapping in the underlying fields of all crc
rings of size 2^(2^n).
We know 3 and 2^(2^n-1)-1 are coprime because 2^(2^n-1)-1 =
2^(2^n)-1 (a Fermat number) - 2^(2^n-1) (a power-of-2), and 3
divides Fermat numbers >=3 (A023394) and is not 2.
- Our delta, when viewed as a polynomial in g: d(g) = gc^2 + g^2c +
c^3, has degree 2, which implies there are at most 2 solutions or
1-bit of information loss in the underlying field.
This is optimal since the original definition already had 2
solutions before we even chose a function:
d(g) = t(g + c) - t(g)
d(g) = t(g + c) - t((g + c) - c)
d(g) = t((g + c) + c) - t(g + c)
d(g) = d(g + c)
Though note the mapping of our crc-ring to the underlying field
already represents 1-bit of information loss.
- If you're using a cryptographic hash or other non-crc, you should
probably just use an equal sized finite-field.
Though note changing from a 2^n-1-bit field to a 2^n-bit field does
change the math a bit, with t(g) = g^7 being a better non-linear
function:
- 7 is the smallest odd-number coprime with 2^n-1, a Fermat number,
which makes t(g) = g^7 a one-to-one mapping.
3 humorously divides all 2^n-1 Fermat numbers.
- Expanding delta with t(g) = g^7 gives us a 6 degree polynomial,
which implies at most 6 solutions or ~3-bits of information loss.
This isn't actually the best you can do, some exhaustive searching
over small fields (<=2^16) suggests t(g) = g^(2^(n-1)-1) _might_ be
optimal, but that's a heck of a lot more multiplications.
- Because our crc32cs preserve parity/are epimorphic to parity bits,
addition (xor) and multiplication (crc32cmul) also preserve parity,
which can be used to show our entire gcksum system preserves parity.
This is quite neat, and means we are guaranteed to detect any odd
number of bit-errors across the entire filesystem.
- Another idea was to use two different addition operations: xor and
overflowing addition (or mod a prime).
This probably would have worked, but lacks the rigor of the above
solution.
- You might think an RS-like construction would help here, where g =
sum(c_ia^i), but this suffers from the same problem:
d_i = g' - g
d_i = g + c_ia^i - g
d_i = c_ia^i
Nothing here depends on anything outside of the current mdir.
- Another question is should we be using an RS-like construction anyways
to include location information in our gcksum?
Maybe in another system, but I don't think it's necessary in littlefs.
While our mdir are independently updateable, they aren't _entirely_
independent. The location of each mdir is stored in either the mtree
or a parent mdir, so it always gets mixed into the gcksum somewhere.
The only exception being the mrootanchor which is always at the fixed
blocks 0x{0,1}.
- This does _not_ catch "global-rollback" issues, where the most recent
commit in the entire filesystem is corrupted, revealing an older, but
still valid, filesystem state.
But as far as I am aware this is just a fundamental limitation of
powerloss-resilient filesystems, short of doing destructive
operations.
At the very least, exposing the gcksum would allow the user to store
it externally and prevent this issue.
---
Implementation details:
- Our gcksumdelta depends on the rbyd's cksum, so there's a catch-22 if
we include it in the rbyd itself.
We can avoid this by including it in the commit tags (actually the
separate canonical cksum makes this easier than it would have been
earlier), but this does mean LFSR_TAG_GCKSUMDELTA is not an
LFSR_TAG_GDELTA subtype. Unfortunate but not a dealbreaker.
- Reading/writing the gcksumdelta gets a bit annoying with it not being
in the rbyd. For now I've extended the low-level lfsr_rbyd_fetch_/
lfsr_rbyd_appendcksum_ to accept an optional gcksumdelta pointer,
which is a bit awkward, but I don't know of a better solution.
- Unlike the grm, _every_ mdir commit involves the gcksum, which means
we either need to propagate the gcksumdelta up the mroot chain
correctly, or somehow keep track of partially flushed gcksumdeltas.
To make this work I modified the low-level lfsr_mdir_commit__
functions to accept start_rid=-2 to indicate when gcksumdeltas should
be flushed.
It's a bit of a hack, but I think it might make sense to extend this
to all gdeltas eventually.
The gcksum cost both code and RAM, but I think it's well worth it for
removing an entire category of filesystem corruption:
code stack ctx
before: 37796 2608 620
after: 38428 (+1.7%) 2640 (+1.2%) 644 (+3.9%)
Most of littlefs's metadata is encoded in leb128s now, with the
exception of tags (be16, sort of), revision counts (le32), cksums
(le32), and flags.
It makes sense for tags to be a special case, these are written and
rewritten _everywhere_, but less so for flags, which are only written to
the mroot and updated infrequently.
We might as well save a bit of code by reusing our le32 machinery.
---
This changes lfsr_format to just write out compat flags as le32s, saving
a tiny bit of code at the cost of a tiny bit of disk usage (the real
benefit being a tiny bit of code simplification):
code stack ctx
before: 37792 2608 620
after: 37772 (-0.1%) 2608 (+0.0%) 620 (+0.0%)
Compat already need to handle trailing zeros gracefully, so this doesn't
change anything at mount time.
Also had to switch from enums to #defines thanks to C's broken enums.
Wooh. We already use #defines for the other flags for this reason.
LFS_WCOMPAT_RDONLY seems generally useful for tools that just want to
mark a filesystem is read-only. This is a common flag that exists in
other filesystems (RO_COMPAT_READONLY in ext4 for example).
LFS_RCOMPAT_WRONLY, on the other hand, is a bit more of a joke, but
there could be some niche use cases for it (preventing double mounts?).
Fortunately, these flags require no extra code, and fall out naturally
from our wcompat/rcompat handling.
---
Originally, the idea was to also add LFS_F_RDONLY, to match LFS_M_RDONLY
and set the LFS_WCOMPAT_RDONLY flag during format.
But this doesn't really work with the current API, since lfsr_format
would just give you an empty filesystem you can't write to. Which is a
bit silly.
Maybe we should add something like lfsr_fs_mkrdonly in the future? This
is probably low-priority.
dbgerr.py and dbgtag.py have proven to be incredibly useful for quick
debugging/introspection, so I figured why not have more of that.
My favorite part is being able to quickly see all flags set on an open
file handle:
(gdb) p file.o.o.flags
$2 = 24117517
(gdb) !./scripts/dbgflags.py o 24117517
LFS_O_WRONLY 0x00000001 Open a file as write only
LFS_O_CREAT 0x00000004 Create a file if it does not exist
LFS_O_EXCL 0x00000008 Fail if a file already exists
LFS_O_DESYNC 0x00000100 Do not sync or recieve file updates
LFS_o_REG 0x01000000 Type = regular-file
LFS_o_UNFLUSH 0x00100000 File's data does not match disk
LFS_o_UNSYNC 0x00200000 File's metadata does not match disk
LFS_o_UNCREAT 0x00400000 File does not exist yet
The only concern is if dbgflags.py falls out-of-sync often, I suspect
flag encoding will have quite a bit more churn than flags/tags. But we
can always drop this script in the future if this turns into a problem.
---
While poking around this also ended up with a bunch of other small
changes:
- Added LFS_*_MODE masks for consistency with other "type<->flag
embeddings"
- Added compat flag comments
- Adopted lowercase prefix for internal flags (LFS_o_ZOMBIE), though
not sure if I'll keep this yet...
- Tweaked dbgerr.py to also match ERR_ prefixes and to ignore case
Since we dropped lfsr_gc_setflags/setsteps, it was no longer possible to
set gc_flags to zero (perfectly valid and useful for system bringup/
testing things). Supporting gc_flags=0 means it's not possible to
provide a default, but this is probably ok as users need to opt-in to
LFS_GC anyways.
Note that at least gc_steps=0 doesn't make sense, so the default there
is reasonable.
Fixing this also highlighted that gc_flags/steps are no longer mutable,
making the comment in lfs_init out-of-date. Dropping these saves a bit
of lfs_t size, so that's nice.
And then testing also revealed that LFS_GC_CKDATA implying LFS_GC_CKDATA
means it should probably clear the LFS_I_CKMETA flag as well.
---
And here I thought this was going to be just a simple test-writing
exercise!
Code changes:
code stack ctx
default before: 37792 2608 620
default after: 37792 (-0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37896 2608 768
gc after: 37848 (-0.1%) 2608 (+0.0%) 760 (-1.0%)
The argument for this flag is pretty brittle. Yes it's _technically_
possible to end up with a compactable filesystem during lfsr_format, but
it's pretty unlikely. And keeping LFS_F_COMPACT around means we'd always
need the lfsr_mtree_gc circuitry in lfsr_format, for such a niche
situation, that can be easily cleaned up in lfsr_mount.
So dropping for now.
No code changes, but this does mean one less feature to support:
code stack ctx
before: 37804 2608 620
after: 37804 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
Looking at future planned features, we're running into some real issues
fitting all these flags into 32 bits.
I think the only real use case for LFS_T_MTREEONLY is in
lfsr_traversal_t, where the depth of traversal can't be infered. So no
reason to keep this flag around in the other APIs.
No code changes:
code stack ctx
default before: 37804 2608 620
default after: 37804 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37940 2608 768
gc after: 37940 (+0.0%) 2608 (+0.0%) 768 (+0.0%)
- LFS_I_INCONSISTENT -> LFS_I_MKCONSISTENT
- LFS_I_CANLOOKAHEAD -> LFS_I_LOOKAHEAD
- LFS_I_UNCOMPACTED -> LFS_I_COMPACT
- LFS_I_CANCKMETA -> LFS_I_CKMETA
- LFS_I_CANCKDATA -> LFS_I_CKDATA
This just makes everything easier to read/pattern match, even if it's
a bit inaccurate english-wise. The imperative transformations were also
wildly inconsistent...
- lfsr_gc -> lfsr_fs_gc
- lfsr_gc_unck -> lfsr_fs_unck
lfsr_fs_unck is surprisingly still useful in non-gc builds, since we
still have ckmeta/ckdata state. These flags can still be queried with
lfsr_fs_stat and cleared with lfsr_fs_ckmeta/ckdata/lfsr_traversal_t, so
it seems useful to keep this function around.
It's also a relatively cheap function.
Though this does mean it deserves a rename. Dropping the gc prefix
hopefully makes it clearer this function is not entirely gc-specific.
And since we no longer have lfsr_gc_setflags/setsteps, it makes sense to
rename lfsr_gc back to lfsr_fs_gc, to be consistent with the other
filesystem-wide utilities.
Code changes, apparently lfsr_fs_unck costs 12 bytes:
code stack ctx
default before: 37792 2608 620
default after: 37804 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37938 2608 768
gc after: 37940 (+0.0%) 2608 (+0.0%) 768 (+0.0%)
LFS_GC_CKMETA and LFS_GC_CKDATA are a bit unique in that their work is
never really done.
Where LFS_GC_MKCONSISTENT/COMPACT can prove things about the system,
LFS_GC_CKMETA/CKDATA can't, because it's always possible for new
bit-errors to develop. Even _during_ an LFS_GC_CKMETA/CKDATA traversal.
But while this is technically true, it's not a very useful state of
things for our lfsr_gc API...
---
What we really want is some way to know if ckmeta/ckdata has completed
"recently" (for some definition of recently), and to let users indicate
when they need another ckmeta/ckdata scan.
To try to solve this:
1. Added LFS_I_CANCKMETA and LFS_I_CANCKDATA to indicate when lfsr_gc
has not checked metadata/data.
These are set during mount (unless mounting with
LFS_M_CKMETA/CKDATA), and cleared when either lfsr_gc completes or
lfsr_fs_ckmeta/data is called. Once cleared, littlefs will not reset
them on its own.
2. Added lfsr_gc_unck to allow users to explicitly reset LFS_I_CKMETA
and/or LFS_I_CKDATA, which will tell lfsr_gc to check metadata/data
again on the next call.
There is some subtlety around clobbering ongoing traversals, but a
mask and some tests should prevent this from being a problem.
Currently, lfsr_gc_unck also allows clearing of other gc flags, but
I'm not sure there's any real use-case for this...
Note that you can still get the previous behavior if you just call
lfsr_gc_unck after every lfsr_gc call.
This also changes info flag behavior slightly in default mode, with
LFS_I_CANCKMETA/CANCKDATA telling you if metadata/data has been checked
since mount. Which does seem useful? Maybe these flags deserve a better
name?
Code changes:
code stack ctx
default before: 37796 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
default after: 37792 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37896 2608 768
gc after: 37938 (+0.1%) 2608 (+0.0%) 768 (+0.0.%)
Now that you can provide gc_flags/gc_steps in lfs_config, I think it's a
bit more clear that _mutating_ the flags/steps is a niche feature, and
not worth implementing/testing.
It raises the question why not have a similar lfsr_setflags or
lfsr_file_setflags, and the answer there is it would be a pain-in-the-
ass to make sure all possible corner cases are covered.
It actually already was a pain-in-the-ass to test lfsr_gcsetflags/
setsteps... but just because we already did the work is not a good
reason for keeping complexity around.
---
Note that most of the use cases for lfsr_gc_setflags/setsteps can be
covered by either remounting the filesystem or through the
lfsr_traversal_t APIs directly.
The end result is a bit of code savings when incremental gc is enabled:
code stack ctx
default before: 37796 2608 620
default after: 37796 (+0.0%) 2608 (+0.0%) 620 (+0.0%)
gc before: 37944 2608 768
gc after 37896 (-0.1%) 2608 (+0.0%) 768 (+0.0%)
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%)
While they are a bit more annoying to call, init functions give the
compiler a chance to deduplicate common struct initialization logic. So
we should probably prefer init functions for any structs larger than a
couple words.
The cost of each init is small, but it really adds up!
code stack ctx
before: 38036 2608 752
after: 37844 (-0.5%) 2608 (+0.0%) 752 (+0.0%)
These should be the last implicit buffers in LFSR_DATA_* macros, leaving
only LFSR_RAT_* macros with implicit stack-allocations (which are wayyy
too useful to give up).
There's an argument to keep these macros implicit, since they represent
relatively small things, but a stack allocation is a stack allocation.
It's safer to make stack allocations explicit, though it does risk
buffer overflow if these fall out-of-sync...
I guess we're forced to choose our poison...
In the end consistency with other LFSR_DATA_* macros wins.
---
And, again, compound-literals are so poorly optimized this minor cleanup
somehow saves code:
code stack ctx
before: 38060 2608 752
after: 38000 (-0.2%) 2608 (+0.0%) 752 (+0.0%)
This was a disappointing failure of compount-literals.
These macros protect against mismatched buffer sizes, which is great for
preventing bugs caused by simple typos, but the overhead of compound-
literals requiring initialization make them simply unusable.
This commit leaves only a couple macros with implicit buffers:
LFSR_DATA_LEB128, and the LFSR_RAT_CAT/LFSR_RATS macros.
Even the tiny cleanup of the one remaining implicit-buffer macro still
in use, LFSR_DATA_GEOMETRY, saved some code:
code stack ctx
before: 38084 2608 752
after: 38060 (-0.1%) 2608 (+0.0%) 752 (+0.0%)
- Fixed issue where some overflowed compat flags could end up ignored.
A simple typo: incrementing by the unrelated d variable, meant we
were skipping overflowed compat flags whenever the previous logic sets
d > 1.
- Fixed issue where any zero padding was treated as overflowed compat
flags.
Note this hid the previous issue from our tests.
Added more tests to prevent a regression here. Letting bad compat flag
parsing through would be _very_ annoying in the future.
Code changes:
code stack ctx
before: 38148 2608 752
after: 38084 (-0.2%) 2608 (+0.0%) 752 (+0.0%)
To clarify this only checks data reads, and to makes space for future
theoretical ck-operations:
- ckmetaredund - likely
- ckdataredund - unlikely, expensive
- ckmetacksums - unlikely, expensive
- ckdatacksums - implemented
This also tweaks the relevant mount/format/info flags a bit:
LFS_M_CKPROGS 0x00100000 Check progs by reading back progged data
LFS_M_CKFETCHES 0x00200000 Check block checksums before first use
LFS_M_CKPARITY 0x00400000 Check metadata tag parity bits
LFS_M_CKMETAREDUND+ 0x01000000 Check metadata redund blocks on reads
LFS_M_CKDATAREDUND* 0x02000000 Check data redund blocks on reads
LFS_M_CKMETACKSUMS* 0x04000000 Check metadata checksums on reads
LFS_M_CKDATACKSUMS 0x08000000 Check data checksums on reads
+Planned
*Hypothetical
No code changes.
So... Long store short, checking metadata cksums is just intractably
slow.
But data cksums?
Yes checking data cksums is still O(b^2), but unlike metadata lookups,
which involve many small backwards reads, data reads are very easy to
cache. So instead of O(b^2), it's more like O(b^2/c), where c is your
rcache size.
Still O(b^2) when c << b, but I'm not sure that's avoidable without
adding more cksums.
At the very least, if you have enough RAM, c == b reduces this to O(b),
which is nice for "large" systems that want hardened reads without a
performance loss.
---
But why bother checking data cksums if we still have a read-hole with
metadata cksums?
Well, while considering the problem in the context of future features, I
noticed something _really interesting_:
- ckredund + metadata - reasonable ✓
- ckredund + data - impractical ✗, parity fanout + O(f+r) is bad
- ckcksums + metadata - impractical ✗, small reads + O(b^2) is bad
- ckcksums + data - reasonable ✓, assuming enough rcache
The current planned design for data redundancy makes it also intractably
slow to check every read, since it would require xoring all blocks that
contribute to the relevant parity block, but this isn't a problem for
metadata redundancy.
So while neither ckredund nor ckcksums can tractably close the read-hole
on their own, it looks like together they will be able to cover
everything without completely sacrificing performance. Neat!
Of course this isn't possible if ckcksums/ckredund imply checking both
metadata and data, so they need to be split apart.
And I don't really see a point in keeping the intractable variants
around in the codebase.
---
Dropping metadata ckcksums also means we can get rid of the ugly
lfsr_bd_ckrbydprefix and lfsr_bd_ckrbydsuffix functions, which were
basically duplicating all of lfsr_rbyd_fetch. That was quite a wart!
This saves a nice chunk of code when ckcksums is enabled:
code stack ctx
default before: 38128 2624 752
default after: 38128 (+0.0%) 2624 (+0.0%) 752 (+0.0%)
ckparity before: 39724 3048 764
ckparity after: 39700 (-0.1%) 3048 (+0.0%) 760 (-0.5%)
ckcksums before: 40612 3184 772
ckcksums after: 39396 (-3.0%) 3096 (-2.8%) 760 (-1.6%)
One of the unexpected side-effects of lazy file creation is that
suddenly LFS_O_EXCL doesn't make sense.
The standard definition: "Fail if the file exists", is easy enough to
implement, but doesn't really match what the user expects.
The user expects one of these calls to fail:
lfsr_file_open(&lfs, &file_a, "file.txt",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_open(&lfs, &file_b, "file.txt",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
But because we create files lazily (to prevent zero-length files after
powerloss), these both succeed.
---
I considered deferring the "file exists" check until we actually would
create the file, but while this _technically_ satisfies the
exclusitivity requirement, I decided against it as I think it just makes
the API way too confusing:
lfsr_file_open(&lfs, &file_a, "file.txt",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_open(&lfs, &file_b, "file.txt",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_close(&lfs, &file_a) => 0;
lfsr_file_close(&lfs, &file_b) => LFS_ERR_EXIST;
---
Instead, a simpler, more pragmatic approach: Fail if the file exists
_or_ if the file is open in a mode that will create the file:
lfsr_file_open(&lfs, &file_a, "file.txt",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_open(&lfs, &file_b, "file.txt",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => LFS_ERR_EXIST;
This explicitly does _not_ error on zombie/desync files:
lfsr_file_open(&lfs, &file_a, "file.txt",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_desync(&lfs, &file_a) => 0;
lfsr_file_open(&lfs, &file_b, "file.txt",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
And it does mean we aren't necessarily guaranteeing the file will be
created, but I think this does more-or-less what the user expects:
- open(a) -> desync(a) -> open(b) -> resync(a) is roughly equivalent to
opening a after creating b, which is perfectly fine with LFS_O_EXCL.
- open(a) -> open(b) (errors) -> desync(a) is one way to not actually
create the file, but is somewhat similar to removing the file after
creation.
If you're using desync files you should probably have a good
understanding of littlefs's sync model anyways.
And of course the user can always sync immediately after open to
guarantee file creation, while opting into the possibility of
zero-length files after powerloss.
Code changes:
code stack ctx
before: 38084 2624 752
after: 38128 (+0.1%) 2624 (+0.0%) 752 (+0.0%)
Christopher Haster