This sort of reverts the addition of lfsr_bd_unprog, but with a slightly
better API. lfsr_bd_unprog was too much of a hack, and isn't really
generalizable. The align flag isn't necessarily any better, but at least
it's the simplest/least-confusing solution available.
And it's net savings, code-wise:
code stack lfs_t
before: 33690 2608 164
after: 33678 (-0.0%) 2600 (-0.3%) 160 (-2.4%)
While exploring the test_badblocks ERASENOOP failure more, I realized
the problem is that we are nesting crc32cs.
To be clear, using crc32cs to validate progs in general is not an issue,
that is perfectly fine on paper. The issue is that we were using crc32cs
to validate progs _that contain crc32cs_.
Looking at the collision, we can see the fully expanded lleb128s we use
for our cksum tags:
00 00 00 ff b0 02 00 87 80 80 00 3e c0 7f 7e => bdfa9b10
ab 77 de c2 b0 03 00 87 80 80 00 3e 38 d5 22 => bdfa9b10
'-.-' ^ '----.----' '----.----'
'----|------|-----------|-- cksum tag
'------|-----------|-- cksum weight (0)
'-----------|-- cksum size + padding
'-- cksum crc32c
So we ended up perfectly aligning the cksum's crc32c with our cache
line. Lucky us.
Unfortunately funny math makes it so that whenever a crc32c contains a
crc32c, the inner crc32c sort of cancels itself out from the outer
crc32c. So these two messages end up mathematically equivalent, even
though they contain different data:
crc(m) = m(x) x^|P|-1 mod P
crc(m ++ crc(m)) = (m(x) x^|P|-1 + (m(x) x^|P|-1 mod P)) x^|P|-1 mod P
crc(m ++ crc(m)) = (m(x) x^|P|-1 + m(x) x^|P|-1) x^|P|-1 mod P
crc(m ++ crc(m)) = 0 x^|P|-1 mod P
crc(m ++ crc(m)) = 0
So using a crc32c to check progs is not fit for purpose.
This leaves us with a couple options:
1. Use a different checksum, or do something like rearranging bytes to
avoid this cancelling out issue. Unfortunately this gets tricky since
crc32cs are linear, simply using an xor mask won't work...
2. Don't check progs at such a low-level, but at a high-level using the
rbyd/data block crc32cs. Since this would mean only one crc32c, this
would avoid nesting issues. Unfortunately this would probably come
with quite a high code cost to try to keep track of both the
before+after rbyd cksums everywhere...
3. Just read back the data into the rcache to compare at the byte-level,
which would mean clobbering our rcache when prog checking is enabled.
This commit goes with option 3., which is probably the simplest. It also
removes any question of crc32c collision, which could be a real nuisance
when debugging low-level block device operations, a use case where prog
checking will hopefully be quite valuable.
Clobbering the rcache also has the advantage of reverting the prog
>= read requirement, which is nice for flexibility. Though this needs to
be tested.
---
There was a bit of a hiccup, and that was how prog checking interacts
with lfsr_bd_cpy. lfsr_bd_cpy used the rcache to hold data being copied
to/from disk, but this data needs to be checked, and prog checking would
clobber the rcache. Problems! I guess this is one footgun of the
internal lfsr_bd_readnext API...
The solution is to instead turn this around and use the pcache to hold
any copied data, since this would not be clobbered when prog checking.
This has some other knock-on effects, mainly that we can't take
advantage of read hints in lfsr_bd_cpy, but has the added advantage of
potentially not clobbering the rcache at all when no checking progs.
Code changes were fairly minimal:
code stack
before: 33718 2608
after: 33690 (-0.1%) 2608 (+0.0%)
This configuration option enables the previous behavior of reading back
every prog to check that the data was written correctly.
Unfortunately, this brings a bit of baggage, thanks to our cache
interactions being more complicated now:
- We really want to reuse the rcache for prog validation, despite the
cache performance implications. Unfortunately, we simply can't, thanks
to the new bd utility functions tying up the rcache. lfsr_bd_cpy, for
example, does not expect rcache to be invalidated between a read and
prog, and if it is, things break (I may or may not have found this by
experience).
These bd utilities are valuable, so we really need some other way to
validate our progs.
- Since we can't rely on the rcache, this leaves checksumming as the
only option for validating progs. Checksumming isn't perfect, as there
is a decent chance of false negatives, but to be honest it's probably
good enough for anything that's not malicious.
- This also adds the new constraint that we need to be able to read back
any prog into the pcache, which implies read_size <= prog_size. This
constraint didn't exist when we could clobber our rcache, but this is
not worth throwing away the new bd utilities. Not to mention
clobbering our rcache could hurt cache performance.
Why not make read_size <= prog_size conditional on check_progs?
The main reason is convenience. One very compelling use case for
check_progs is to help debug unknown filesystem/integration failures,
buf if you can't enable check_progs without changing the filesystem
configuration, you can't really rely on check_progs for debugging.
This helps future proof what we expect from block devices, in case
future error detection/correction mechanisms can benefit from our
prog_size always being readable.
Code changes were not that significant, however there was a surprising
stack cost. This seems to be because lfsr_bd_read__ can now be called
from multiple places, causing it to no longer be inlined in
lfsr_bd_read_, costing a bit of stack for the additional function call:
before: 33566 2624
after: 33682 (+0.3%) 2640 (+0.6%)
The mleafweight naming is... not great...
Renaming mleaf_bits -> mdir_bits and replacing mleafweight with explicit
shifts of 1 << mdir_bits seems to get the job done without introducing a
new and potentially confusing name.
This was a lesson learned from recycle_bits. Sometimes more helpers just
makes code less, not more, readable.
This makes a bit more sense with the new block_recycles name.
block_recycles=0 (previously block_recycles=1) requires 1 erase, but it
doesn't really "recycle" the block. With this change, block_recycles=1
"recycles" the block once (2 erases in total) before relocating, which I
think is a bit more intuitive.
Note, this sort of messes with our power-of-2 rounding, as the
block_recycles is technically rounded down to the nearest power-of-2
after adding 1:
- block_recycles=1022 -> 512 erases
- block_recycles=1023 -> 1024 erases
- block_recycles=1024 -> 1024 erases
- block_recycles=1025 -> 1024 erases
But I'm going to keep the block_recycles description more-or-less as is
for now, as I think this extra detail is more confusing than useful,
powers-of-2 stay powers-of-2, and the <=block_recycles contraint is not
violated.
The original goal here was to restore all of the revision count/
wear-leveling features that were intentionally ignored during
refactoring, but over time a few other ideas to better leverage our
revision count bits crept in, so this is sort of the amalgamation of
that...
Note! None of these changes affect reading. mdir fetch strictly needs
only to look at the revision count as a big 32-bit counter to determine
which block is the most recent.
The interesting thing about the original definition of the revision
count, a simple 32-bit counter, is that it actually only needs 2-bits to
work. Well, three states really: 1. most recent, 2. less recent, 3.
future most recent. This means the remaining bits are sort of up for
grabs to other things.
Previously, we've used the extra revision count bits as a heuristic for
wear-leveling. Here we reintroduce that, a bit more rigorously, while
also carving out space for a nonce to help with commit collisions.
Here's the new revision count breakdown:
vvvvrrrr rrrrrrnn nnnnnnnn nnnnnnnn
'-.''----.----''---------.--------'
'------|---------------|---------- 4-bit relocation revision
'---------------|---------- recycle-bits recycle counter
'---------- pseudorandom nonce
- 4-bit relocation revision
We technically only need 2-bits to tell which block is the most
recent, but I've bumped it up to 4-bits just to be safe and to make
it a bit more readable in hex form.
- recycle-bits recycle counter
A user configurable counter, this counter tracks how many times a
metadata block has been erased. When it overflows we return the block
to the allocator to participate in block-level wear-leveling again.
This implements our copy-on-bounded-write strategy.
- pseudorandom nonce
The remaining bits we fill with a pseudorandom nonce derived from the
filesystem's prng. Note this prng isn't the greatest (it's just the
xor of all mdir cksums), but it gets the job done. It should also be
reproducible, which can be a good thing.
Suggested by ithinuel, the addition of a nonce should help with the
commit collision issue caused by noop erases. It doesn't completely
solve things, since we're only using crc32c cksums not collision
resistant cryptographic hashes, but we still have the existing
valid/perturb bit system to fall back on.
When we allocate a new mdir, we want to zero the recycle counter. This
is where our relocation revision is useful for indicating which block is
the most recent:
initial state: 10101010 10101010 10101010 10101010
'-.'
+1 zero random
v .----'----..---------'--------.
lfsr_rev_init: 10110000 00000011 01110010 11101111
When we increment, we increment recycle counter and xor in a new nonce:
initial state: 10110000 00000011 01110010 11101111
'--------.----''---------.--------'
+1 xor <-- random
v v
lfsr_rev_init: 10110000 00000111 01010100 01000000
And when the recycle counter overflows, we relocate the mdir.
If we aren't wear-leveling, we just increment the relocation revision to
maximize the nonce.
---
Some other notes:
- Renamed block_cycles -> block_recycles.
This is intended to help avoid confusing block_cycles with the actual
physical number of erase cycles supported by the device.
I've noticed this happening a few times, and it's unfortunately
equivalent to disabling wear-leveling completely. This can be improved
with better documentation, but also changing the name doesn't hurt.
- We now relocate both blocks in the mdir at the same time.
Previously we only relocated one block in the mdir per recycle. This
was necessary to keep our threaded linked-list in sync, but the
threaded linked-list is now no more!
Relocating both blocks is simpler, updates the mtree less often,
compatible with metadata redundancy, and avoids aliasing issues that
were a problem when relocating one block.
Note that block_recycles is internally multiplied by 2 so each block
sees the correct number of erase cycles.
- block_recycles is now rounded down to a power-of-2.
This makes the counter logic easier to work with and takes up less RAM
in lfs_t. This is a rough heuristic anyways.
- Moved the lfs->seed updates into lfsr_mountinited + lfsr_mdir_commit.
This avoids readonly operations affecting the seed and should help
reproducibility.
- Changed rev count in dbg scripts to render as hex, similar to cksums.
Now that we using most of the bits in the revision count, the decimal
version is, uh, not helpful...
Code changes:
code stack
before: 33342 2640
after: 33434 (+0.3%) 2640 (+0.0%)
The main change is moving away from applying gstate changes via special
attrs. Instead, gstate changes are applied implicitly, whenever the
relevant field in lfs_t differs from the gstate on-disk.
How do we recover from errors then? Well, we already need to track the
exact on-disk encoding of any gstate (grm_p) to avoid issues with minor
encoding differences, so if we encounter an error, we can revert any
changes to gstate by re-decoding the on-disk gstate. This is more
fragile: 1. all error paths in lfsr_mdir_commit need to revert gstate,
2. logic must not error between gstate updates and lfsr_mdir_commit, but
it gets the job done.
The benefit of this approach is that it's much easier to manipulate
gstate inside of lfsr_mdir_commit. No more hacky attr-list scanning to
patch grms mid-commit! It also in theory saves stack usage by dropping
an attr, but none of these attrs were on our stack hot-path.
Other gstate changes:
- Moved all grm adjustments into lfsr_mdir_commit.
This should deduplicate the messy grm adjust logic and make grms
easier to work with.
One hiccup though is the temporarily self-removing bookmark created in
lfsr_mkdir, which needs to create a grm referencing an mid that
doesn't exist yet. To work around this, lfsr_mdir_commit now
automatically creates grms for new bookmarks.
This might be a problem if we ever elide same-mdir mkdirs, but if so
we can solve that problem then.
- Dropped lfsr_data_t xoring, the added complexity wasn't really worth
it since all gstate should be small enough to buffer on the stack.
- Renamed several things:
- lfsr_grm_push/poprm -> lfsr_grm_push/pop
- lfsr_grm_isrm -> lfsr_grm_ispending
- grm_g -> grm_p
- grm.rms -> grm.mids
- Moved things around so grm/gstate logic is grouped together.
Unfortunately none of these attrs were on our stack hot-path, so no
stack savings. But thanks to the simpler logic, this does save quite a
bit of code:
code stack
before: 33514 2632
after: 33338 (+0.5%) 2640 (+0.3%)
So for example:
file->m.mdir.mid => file->o.mdir.mid
We already use "o" in opened-list iterations, so this is a bit more
consistent. And it doesn't increase the already obnoxious
file->o.mdir.rbyd.blocks[0] field names...
Now that lfsr_dir_t contains a single lfsr_opened_t, it makes sense for
lfsr_opened_t to always come first in lfsr_dir_t/lfsr_file_t for
consistency.
This also allows cheaper lfsr_file_t <-> lfsr_opened_t casts (noops),
which saves a bit of code:
code stack
before: 33582 2632
after: 33538 (-0.1%) 2632 (+0.0%)
This simplification comes from the observation that we don't actually
need to know the bookmark's mid to know if a given operation is in a
dir's range, just the dir's did. And since dids are immutable, we don't
need another opened-list entry or other shenanigans.
A dir's did is a bit harder to access, requiring a name lookup, but we
conveniently already fetch these in all relevant functions as a part of
path resolution.
This does mean more opened-list logic in the high-level functions:
function can zombie can create can remove
lfsr_mkdir y y n
lfsr_rename y y y
lfsr_remove y n y
lfsr_file_opencfg y y n
But I think this actually results in better code readability, since the
opened-list logic and high-level logic are closely related. I went ahead
and lifted the similar orphan/zombie opened-list logic up to this level
for this reason.
Unfortunately lifting this logic does result in a higher code cost, but
I think this is worth it for better readability and a significantly
reduced RAM cost for lfsr_dir_ts. Keep in mind these will probably
become very common for the future planned openat/*at functions:
code stack lfsr_dir_t
before: 33402 2632 80
after: 33582 (+0.5%) 2632 (+0.0%) 44 (-45.0%)
Also added a new test case, test_dread_read_rm_remkdir, to catch the
mistake of thinking the did is unique even when the dir is removed,
since that is now a concern.
Because of the invasive linked-lists, this was a bit more complicated
than the related move in lfs_t. But we already have similar
field-relative offsets in lfsr_dir_t for the dir + bookmark mdirs.
Added some helpers to help with this:
- lfsr_opened_dir
- lfsr_opened_constdir
- lfsr_opened_bookmark
- lfsr_opened_constbookmark
- lfsr_opened_file
- lfsr_opened_constfile
Unfortunately this resulted in less savings than in lfs_t, and actually
costs us code, likely because of how often we go from lfsr_file_t <->
lfsr_opened_t:
code stack
before: 33358 2632
after: 33402 (+0.1%) 2632 (+0.0%)
- Removed no longer used fields.
- Commented out related field asserts in lfs_init.
- Commented out pre-lfsr structs and function decls.
- Moved cfg to the first field in lfs_t.
Note that most of the code saves actually came from that last point.
Moving lfs.cfg, probably the currently most accessed field, resulted in
a surprising amount of code savings:
code stack lfs_t
before: 33702 2640 220
after+cfg last: 33592 (-0.3%) 2632 (-0.3%) 164 (-25.5%)
after+cfg first: 33358 (-1.0%) 2632 (-0.3%) 164 (-25.5%)
Maybe we should take a more rigorous/analytical approach to field
placement?
This would have been more valuable if the extra lfsr_data_t stack
allocation (12 bytes) wasn't already unioned with the btree's encoding
buffer allocation (18 bytes):
code stack
before: 33714 2640
after: 33702 (-0.0%) 2640 (+0.0%)
Oh well, this still might save some stack in the future if things shift
around.
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.
This limits the maximum size of a file, which is also implies the
maximum integer size required to mount.
The exact name is a bit of a toss-up. I originally went with size_limit
to avoid confusion around if file_limit reflected the file size or the
number of files, but since this ends up mapping to lfs_off_t and _not_
lfs_size_t, I think size_limit may be a bit of a bad choice.
The concern with block_size/4 is that it limits fragments to a single
fragment per-block. But while this may be inefficient, it's technically
not wrong, and may still work with other metadata (bptrs, file names,
uattrs, etc) taking up the remaining space.
This deserves benchmarking, but even if this ends up being a terrible
configuration, we should just discourage this via good defaults and
documentation.
Also added related asserts to lfs_init.
Note the fragment_size <= block_size/8 limit is to avoid wasteful corner
cases where only one fragment can fit in a block. The shrub_size <=
block_size/4 limit is looser because of how shrubs temporarily
overcommit.
As for the other limits, inline_size is bounded by shrub_size, and
crystal_thresh technically doesn't have a limit, though values >
block_size stop having an effect.
If we have control of the struct, such as in lfsr_data_t and lfsr_cat_t,
moving the common tag outside of the union avoids naming ambiguities.
Counter-example: This doesn't work for lfsr_bshrub_t, since the contents
of that union are also used as separate types elsewhere. Fortunately the
common initial sequence union rules kick in here.
No code changes, which is good:
code stack
before: 33652 2624
after: 33652 (+0.0%) 2624 (+0.0%)
So now, instead of one data type trying to do everything, we have two:
1. lfsr_data_t - Readable data, either in-RAM or on-disk
2. lfsr_cat_t - Concatenated data for progging, may be either a simple
in-RAM buffer or an indirect list of lfsr_data_ts
This comes from an observation that most lfsr_attr_t datas were either
simple buffers, NULL, or required the indirect concatenated datas
anyways (concatendated file fragments). By separating lfsr_cat_t and
lfsr_data_t, maybe we can save RAM in lfsr_attr_t by not needing the
three words necessary for the less-common disk references.
Note the interesting tradeoff:
Simple in-RAM buffers/NULL decrease by 1 word (4 bytes):
lfsr_data_t lfsr_cat_t
.---+---+---+---. .---+---+---+---.
|0| size | => |0| size |
+---+---+---+---+ +---+---+---+---+
| ptr | | ptr |
+---+---+---+---+ '---+---+---+---'
| (unused) |
'---+---+---+---'
'-------.-------' '-------.-------'
12 bytes 8 bytes
While on-disk references increase by 2 words (8 bytes):
lfsr_data_t lfsr_cat_t lfsr_data_t
.---+---+---+---. .---+---+---+---. .---+---+---+---.
|1| size | => |1| size | .>|1| size |
+---+---+---+---+ +---+---+---+---+ | +---+---+---+---+
| block | | ptr -------' | block |
+---+---+---+---+ '---+---+---+---' +---+---+---+---+
| off | | off |
'---+---+---+---' '---+---+---+---'
'-------.-------' '-----------------.-----------------'
12 bytes 20 bytes
Unless the on-disk references also need concatenation, in which case
this still saves 1 word (4 bytes).
Note I'm not sure this type split is generalizable to other systems. In
littlefs we can't use recursion, so progging concatenated datas already
required two nested functions, and we happen to never need to read
concatenated data, allowing us to completely omit that functionality. In
other systems, where maybe disk-reference attrs are more common, this
tradeoff may not make sense.
Some other things to note:
- We're also losing the inlined-data representation in this change.
Unfortunately earlier lfsr_data_t measurements showed that this didn't
really contribute much. It saved RAM in name attrs but added quite a
bit of complexity to lfsr_data_t operations.
- By separating simple/cat and RAM/disk, we reduce the abused size bits
from 2-bits down to 1-bit. This doesn't really matter for our current
31/28-bit littlefs impl, but is nice in that it reenables the
theoretical 31/31-bit littlefs impl without in-RAM data-structure
changes.
There are a few temporary hacks that need to be figured out, but this is
already showing code/stack savings. Which is fascinating considering the
new lfsr_cat_* functions and increased temporary allocations:
code stack
before: 33856 2824
after: 33812 (-0.1%) 2800 (-0.8%)
Note this is slightly different than cases where we use the sign-bit for
muxing two different types, such as `int err` and `lfsr_srid_t rid`. In
those cases we'd never extract the lower bits of the int
unconditionally.
This leads to fewer casts and I think signals the intention of these
sign-bit-is-flag ints a bit better. We aren't really interpreting these
as signed, and mask out other bits in some cases (lfsr_data_t).
This leads to more code in places, I'm guessing because of C treating
signed overflow as undefined behavior... Maybe this is a good thing:
code stack
before: 33856 2824
after: 33876 (+0.1%) 2824 (+0.0%)
This now properly encodes the different eoff/estimate field usage
between the two types.
In theory this could save some RAM, but we don't actually allocate
lfsr_shrub_t anywhere it's not unioned with lfsr_btree_t, so:
code stack
before: 33976 2824
after: 33976 (+0.0%) 2824 (+0.0%)
Long story short, rbyd checksums are now fully reproducible. If you
write the same set of tags to any block, you will end up with the same
checksum.
This is actually a bit tricky with littlefs's constraints.
---
The main problem boils down to erased-state. littlefs has a fairly
flexible model for erased-state, and this brings some challenges. In
littlefs, storage goes through 2 states:
1. Erase - Prepare storage for progging. Reads after an erase may return
arbitrary, but consistent, values.
2. Prog - Program storage with data. Storage must be erased and no progs
attempted. Reads after a prog must return the new data.
Note in this model erased-state may not be all 0xffs, though it likely
will be for flash. This allows littlefs to support a wide range of
other storage devices: SD, RAM, NVRAM, encryption, ECC, etc.
But this model also means erased-state may be different from block to
block, and even different on later erases of the same block.
And if that wasn't enough of a challenge, _erased-state can contain
perfectly valid commits_. Usually you can expect arbitrary valid cksums
to be rare, but thanks to SD, RAM, etc, modeling erase as a noop, valid
cksums in erased-state is actually very common.
So how do we manage erased-state in our rbyds?
First we need some way to detect it, since we can't prog if we're not
erased. This is accomplished by the forward-looking erased-state cksum
(ecksum):
.---+---+---+---. \
| commit | |
| | |
| | |
+---+---+---+---+ +-.
| ecksum -------. | | <-- ecksum - cksum of erased state
+---+---+---+---+ | / |
| cksum --------|---' <-- cksum - cksum of commit,
+---+---+---+---+ | including ecksum
| padding | |
| | |
+---+---+---+---+ \ |
| erased | +-'
| | /
. .
. .
You may have already noticed the start of our problems. The ecksum
contains the erased-state, which is different per-block, and our rbyd
cksum contains the ecksum. We need to include the ecksum so we know if
it's valid, but this means our rbyd cksum changes block to block.
Solving this is simple enough: Stop the rbyd's canonical cksum before
the ecksum, but include the ecksum in the actual cksum we write to disk.
Future commits will need to start from the canonical cksum, so the old
ecksum won't be included in new commits, but this shouldn't be a
problem:
.---+---+---+---. . . \ . \ . . . . .---+---+---+---. \ \
| commit | | | | commit | | |
| | | +- rbyd | | | |
| | | | cksum | | | |
+---+---+---+---+ +-. / +---+---+---+---+ | |
| ecksum -------. | | | ecksum | . .
+---+---+---+---+ | / | +---+---+---+---+ . .
| cksum --------|---' | cksum | . .
+---+---+---+---+ | +---+---+---+---+ . .
| padding | | | padding | . .
| | | | | . .
+---+---+---+---+ \ | . . . . . . . +---+---+---+---+ | |
| erased | +-' | commit | | |
| | / | | | +- rbyd
. . | | | | cksum
. . +---+---+---+---+ +-. /
| ecksum -------. | |
+---+---+---+---+ | / |
| cksum ------------'
+---+---+---+---+ |
| padding | |
| | |
+---+---+---+---+ \ |
| erased | +-'
| | /
. .
. .
The second challenge is the pesky possibility of existing valid commits.
We need some way to ensure that erased-state following a commit does not
accidentally contain a valid old commit.
This is where are tag's valid bits come into play: The valid bit of each
tag must match the parity of all preceding tags (equivalent to the
parity of the crc32c), and we can use some perturb bits in the cksum tag
to make sure any tags in our erased-state do _not_ match:
.---+---+---+---. \ . . . . . .---+---+---+---. \ \ \
|v| tag | | |v| tag | | | |
+---+---+---+---+ | +---+---+---+---+ | | |
| commit | | | commit | | | |
| | | | | | | |
+---+---+---+---+ +-----. +---+---+---+---+ +-. | |
|v|p| tag | | | |v|p| tag | | | | |
+---+---+---+---+ / | +---+---+---+---+ / | | |
| cksum | | | cksum | | . .
+---+---+---+---+ | +---+---+---+---+ | . .
| padding | | | padding | | . .
| | | | | | . .
+---+---+---+---+ . . . | . . +---+---+---+---+ | | |
|v---------------- != --' |v------------------' | |
| erased | +---+---+---+---+ | |
. . | commit | | |
. . | | | |
+---+---+---+---+ +-. +-.
|v|p| tag | | | | |
+---+---+---+---+ / | / |
| cksum ----------------'
+---+---+---+---+ |
| padding | |
| | |
+---+---+---+---+ |
|v---------------- != --'
| erased |
. .
. .
New problem! The rbyd cksum contains the valid bits, which contain the
perturb bits, which depends on the erased-state!
And you can't just derive the valid bits from the rbyd's canonical
cksum. This avoids erased-state poisoning, sure, but then nothing in the
new commit depends on the perturb bits! The catch-22 here is that we
need the valid bits to both depend on, and ignore, the erased-state
poisoned perturb bits.
As far as I can tell, the only way around this is to make the rybd's
canonical cksum not include the parity bits. Which is annoying, masking
out bits is not great for bulk cksum calculation...
But this does solve our problem:
.---+---+---+---. \ . . . . . .---+---+---+---. \ \ \ \
|v| tag | | |v| tag | | | o o
+---+---+---+---+ | +---+---+---+---+ | | | |
| commit | | | commit | | | | |
| | | | | | | | |
+---+---+---+---+ +-----. +---+---+---+---+ +-. | | |
|v|p| tag | | | |v|p| tag | | | | . .
+---+---+---+---+ / | +---+---+---+---+ / | | . .
| cksum | | | cksum | | . . .
+---+---+---+---+ | +---+---+---+---+ | . . .
| padding | | | padding | | . . .
| | | | | | . . .
+---+---+---+---+ . . . | . . +---+---+---+---+ | | | |
|v---------------- != --' |v------------------' | o o
| erased | +---+---+---+---+ | | |
. . | commit | | | +- rbyd
. . | | | | | cksum
+---+---+---+---+ +-. +-. /
|v|p| tag | | | o |
+---+---+---+---+ / | / |
| cksum ----------------'
+---+---+---+---+ |
| padding | |
| | |
+---+---+---+---+ |
|v---------------- != --'
| erased |
. .
. .
Note that because each commit's cksum derives from the canonical cksum,
the valid bits and commit cksums no longer contain the same data, so our
parity(m) = parity(crc32c(m)) trick no longer works.
However our crc32c still does tell us a bit about each tag's parity, so
with a couple well-placed xors we can at least avoid needing two
parallel calculations:
cksum' = crc32c(cksum, m)
valid' = parity(cksum' xor cksum) xor valid
This also means our commit cksums don't include any information about
the valid bits, since we mask these out before cksum calculation. Which
is a bit concerning, but as far as I can tell not a real problem.
---
An alternative design would be to just keep track of two cksums: A
commit cksum and a canonical cksum.
This would be much simpler, but would also require storing two cksums in
RAM in our lfsr_rbyd_t struct. A bit annoying for our 4-byte crc32cs,
and a bit more than a bit annoying for hypothetical 32-byte sha256s.
It's also not entirely clear how you would update both crc32cs
efficiently. There is a way to xor out the initial state before each
tag, but I think it would still require O(n) cycles of crc32c
calculation...
As it is, the extra bit needed to keep track of commit parity is easy
enough to sneak into some unused sign bits in our lfsr_rbyd_t struct.
---
I've also gone ahead and mixed in the current commit parity into our
cksum's perturb bits, so the commit cksum at least contains _some_
information about the previous parity.
But it's not entirely clear this actually adds anything. Our perturb
bits aren't _required_ to reflect the commit parity, so a very unlucky
power-loss could in theory still make a cksum valid for the wrong
parity.
At least this situation will be caught by later valid bits...
I've also carved out a tag encoding, LFSR_TAG_PERTURB, solely for adding
more perturb bits to commit cksums:
LFSR_TAG_CKSUM 0x3cpp v-11 cccc -ppp pppp
LFSR_TAG_CKSUM 0x30pp v-11 ---- -ppp pppp
LFSR_TAG_PERTURB 0x3100 v-11 ---1 ---- ----
LFSR_TAG_ECKSUM 0x3200 v-11 --1- ---- ----
LFSR_TAG_GCKSUMDELTA+ 0x3300 v-11 --11 ---- ----
+ Planned
This allows for more than 7 perturb bits, and could even mix in the
entire previous commit cksum, if we ever think that is worth the RAM
tradeoff.
LFSR_TAG_PERTURB also has the advantage that it is validated by the
cksum tag's valid bit before being included in the commit cksum, which
indirectly includes the current commit parity. We may eventually want to
use this instead of the cksum tag's perturb bits for this reason, but
right now I'm not sure this tiny bit of extra safety is worth the
minimum 5-byte per commit overhead...
Note if you want perturb bits that are also included in the rbyd's
canonical cksum, you can just use an LFSR_TAG_SHRUBDATA tag. Or any
unreferenced shrub tag really.
---
All of these changes required a decent amount of code, I think mostly
just to keep track of the parity bit. But the isolation of rbyd cksums
from erased-state is necessary for several future-planned features:
code stack
before: 33564 2816
after: 33916 (+1.0%) 2824 (+0.3%)
Unfortunately block-level erased-state checksums (becksums) don't really
work as intended.
An invalid becksum _does_ signal that a prog has been attempted, but a
valid becksum does _not_ prove that a prog has _not_ been attempted.
Rbyd ecksums work, but only thanks to a combination of prioritizing
valid commits and the use of perturb bits to force erased-state changes.
It _is_ possible to end up with an ecksum collision, but only if you
1. lose power before completing a commit, and 2. end up with a
non-trivial crc32c collision. If this does happen, at the very least the
resulting commit will likely end up corrupted and thrown away later.
Block-level becksums, at least as originally designed, don't have either
of these protections. To make matters worse, the blocks these becksums
reference contain only raw user data. Write 0xffs into a file and you
will likely end up with a becksum collision!
This is a problem for a couple of reasons:
1. Progging multiple times to erased-state is likely to result in
corrupted data, though this is also likely to get caught with
validating writes.
Worst case, the resulting data looks valid, but with weakened data
retention.
2. Because becksums are stored in the copy-on-write metadata of the
file, attempting to open a file twice for writing (or more advanced
copy-on-write operations in the future) can lead to a situation where
a prog is attempted on _already committed_ data.
This is very bad and breaks copy-on-write guarantees.
---
So clearly becksums are not fit for purpose and should be dropped. What
can we replace them with?
The first option, implemented here, is RAM-tracked erased state. Give
each lfsr_file_t its own eblock/eoff fields to track the last known good
erased-state. And before each prog, clear eblock/eoff so we never
accidentally prog to the same erased-state twice.
It's interesting to note we don't currently clear eblock/eoff in all
file handles, this is ok only because we don't currently share
eblock/eoff across file handles. Each eblock/eoff is exclusive to the
lfsr_file_t and does not appear anywhere else in the system.
The main downside of this approach is that, well, the RAM-tracked
erase-state is only tracked in RAM. Block-level erased-state effectively
does not persist across reboots. I've considered adding some sort of
per-file erased-state tracking to the mdir that would need to be cleared
before use, but such a mechanism ends up quite complicated.
At the moment, I think the best second option is to put erased-state
tracking in the future-planned bmap. This would let you opt-in to
on-disk tracking of all erased-state in the system.
One nice thing about RAM-tracked erased-state is that it's not on disk,
so it's not really a compatibility concern and won't get in the way of
additional future erased-state tracking.
---
Benchmarking becksums vs RAM-tracking has been quite interesting. While
in theory becksums can track much more erased-state, it's quite unlikely
anything but the most recent erased-state actually ends up used. The end
result is no real measurable performance loss, and actually a minor
speedup because we don't need to calculate becksums on every block
write.
There are some pathological cases, such as multiple write heads, but
these are out-of-scope right now (note! multiple explicit file handles
currently handle this case beautifully because we don't share
eblock/eoff!)
Becksums were also relatively complicated, and needed extra scaffolding
to pass around/propagate as secondary tags alongside the primary bptr.
So trading these for RAM-tracking also gives us a nice bit of code/stack
savings, albeit at a 2-word RAM cost in lfsr_file_t:
code stack structs
before: 33888 2864 1096
after: 33564 (-1.0%) 2816 (-1.7%) 1104 (+0.7%)
lfsr_file_t before: 104
lfsr_file_t after: 112 (+7.7%)
This is mostly just a lot of leb128s, though we do use be16 for tags and
le32 for cksums and revision counts.
There are several places we use single-byte leb128s, which really are
u8s with the top bit reserved. Still, notating this as leb128 indicates
that the top bit really is reserved, even if you don't need full leb128
encoding/decoding in practice.
The concern right now is small-block filesystems, anything in the 512B
to <4KiB range. With such small blocks, and rbyd's relatively high
per-attr overhead, there's a real risk that littlefs may just not be
able to function without quickly running to metadata limits.
I realize these are pretty rare geometries for flash, but they are still
common for anything that 1. pretends to be a spinny disks, SD cards,
FTLs, eMMCs, etc, and 2. mapping into RAM, which is surprisingly common.
It is possible to require this sort of geometry to pretend to be a
larger logical block-size, but since this is a regression from the
previous version of littlefs, it would be nice to avoid this if
possible.
Anyways, what actually is this commit. Consider our tag encoding:
.---+---+---+- -+- -+- -+- -+---+- -+- -+- -. tag: 2 bytes
| tag | weight | size | weight: <=5 bytes
'---+---+---+- -+- -+- -+- -+---+- -+- -+- -' size: <=4 bytes
total: <=11 bytes
With our current 32-bit (really 31-bit) version of littlefs, the worst
case tag encoding is 11 bytes.
This doesn't sound that bad, but with our current compaction algorithm we
need ~2.5 tags for each attr:
5t 5*11
a_1 = -- + 2 = ---- = 30 bytes
2 2
Are there any additional assumptions we can make to push our attr
estimate lower?
- tag - Ignoring a complete redesign of our tag encoding (which has
already been heavily iterated over), this just needs 2 bytes, which is
not that bad.
- weight - This is the real painful one because, for the most part,
weight=0. But weight _can_ store a full size, in the case it is the
root of a file's btree. So this is pretty much stuck at an annoying
5 bytes.
I suppose this could be tied to our size-limit. I hadn't thought about
that until writing this commit message. Maybe that can be a future
improvement, though it won't really have a big effect on most systems.
- size/jump - Now this field is interesting. When expressing both the
size of tag payloads, and the relative jump offset for alt-pointers,
this field should never exceed a single block.
We've already pushed this down to 4 bytes at compile time, by assuming
at most 28-bit block-sizes, but if we know the block-size, we could in
theory push this even lower.
This is extra enticing, because the block-sizes where the size/jump
field can be shrunk, are _also_ the block-sizes where the metadata
density is so critical!
So that's what this commit does. For the purpose of compaction estimates
(not stack allocations!) we calculate attr estimate based on our
runtime-determined block_size.
Here are some cutoff points for our new attr estimate:
block-size tag-estimate attr-estimate
512B => 9 bytes 25 bytes
16KiB => 10 bytes 27 bytes
2MiB => 11 bytes 30 bytes
256MiB => 12 bytes 32 bytes
There is a question of when to actually do this calculation. We always
know our block-size, so we could recalculate the attr-estimate every
time we need to estimate a compaction. But for now I'm just
precalculating the attr estimate in lfs_init and storing in the lfs_t
struct. It's only a byte after all.
If I did my math correctly, we won't exceed a byte until we have a
block-size of 2^1750, at which point we may have other problems.
Code changes:
code stack lfs_t
before: 34068 2880 216
after: 34104 (-0.1%) 2880 (+0.0%) 220 (+1.9%)
The jump in lfs_t cost is probably just from a word alignment boundary.
In the future, if we have compile-time block-sizes, the entire
attr-estimate could even be compile-time.
- It didn't save code.
- An inlined buffer is potentially more useful, even if only marginally,
and, uh, unproven yet.
- Requiring lfs_toleb128 in a readonly implementation is a hard ask.
The idea is that we can save on the cost of calling lfs_toleb128
everywhere we commit leb128s, by lazily encoding during progdata.
I original thought this would have too many small problems, but:
1. We can actually implement slice surprisingly easily by just shifting
the internal word 7 bits. This emulates byte-level slicing in the
encoded leb128.
This enables read/cmp, so we can implement all of the lfsr_data_t
functions, though it does make lfs_toleb128 required for a readonly
implementation, which isn't great. Sufficient creativity with ifdefs
likely makes this a non-problem though.
2. There's really very limited use cases for non-leb128 inlined datas.
We can use it to encode the version and compatflags during
lfs_format, but that's about it. And lfs_format is definitely not on
the stack hot-path, so there's no reason to not use on-stack buffers
for these.
The original motivation for this change was noticing a surprising amount
of code savings related to lazy leb128 encoding in another lfsr_data_t
refactor. Unfortunately this savings does not seem reproducible:
code stack
before: 33864 2880
after: 33912 (+0.1%) 2888 (+0.3%)
But that's ok, this is closer to what I expected. The lfs_sizeleb128
call we need to predict the leb128 size is close to the same cost as
calling lfs_toleb128 so the savings isn't really that much.
Topologically, this isn't really much of a change. We just moved the
flcksum -> lfs.pcksum and made the internal API a bit better.
But hey, a better internal API at ~no cost is always a good thing:
code stack lfs_t
before: 33868 2880 212
after: 33856 (-0.0%) 2880 (+0.0%) 216 (+1.9%)
Thanks to poor compound literal optimization, it's actually cheaper to
pass lfsr_data_t by value everywhere, than to make all LFSR_DATA_*
macros lvalues:
before: 34340 2896
after: 34292 (-0.1%) 2896 (+0.0%)
Why are these two design choices linked? If lfsr_data_t is
pass-by-address, the rvalue/lvalue disinction is important because we
need to take the address of LFSR_DATA_* macros. If lfsr_data_t is
pass-by-value, rvalue/lvalue doesn't really matter because we, well,
pass by value.
To be honest, this is a bit of an excuse for better lfsr_data_t
ergonomics. It _is_ generally worse code-size wise to pass lfsr_data_t
by value, because most ABI optimizations stop at 2 words and
lfsr_data_t requires 3 words. But always passing lfsr_data_t by value
even if it is suboptimal makes for more consistent internal interfaces.
This also helps side-step a mistake I made earlier where I though
cat/fromimm/fromleb128 were the only LFSR_DATA_* macros that needed to
be lvalues to be consistent. THERE ARE MANY MORE LFSR_DATA_* macros,
every LFSR_DATA_FROMBLAH macro to be specific, and the resulting code
cost would be MUCH WORSE.
---
This also add lfsr_sprout_t to complement lfsr_bptr_t/lfsr_shrub_t/etc.
Unlike lfsr_data_t, lfsr_sprout_t _is_ pass-by-address
Actually that's the only difference, haha. lfsr_sprout_t is a typedef.
Though to be fair, by being pass-by-addres, lfsr_sprout_t keeps the
internal sprout/shrub/bptr/btree inferfaces consistent, and saves a bit
of code.
Now that in-block fields are limited to 28-bits, we have a few more bits
in our lfsr_data_t size field to encoding things.
This commit uses the top 2-bits to encode one of our 4 different
lfsr_data_t encodings:
- 00--nnnn nnnnnnnn nnnnnnnn nnnnnnnn => buffer poiner
- 01--nnnn nnnnnnnn nnnnnnnn nnnnnnnn => inlined data
- 10--nnnn nnnnnnnn nnnnnnnn nnnnnnnn => on-disk reference
- 11--nnnn nnnnnnnn nnnnnnnn nnnnnnnn => concatenated data pointer
.--|-------|-------|-------'
| | .-|-------'
| | | '------.
| '-----|--------|--------.
v v v v
1nnnnnnn 1nnnnnnn 1nnnnnnn 0nnnnnnn <= leb128
Note this still works with a hypothetical 12-bit/10-bit littlefs
variant, where we'd only have 2 spare bits:
- 00nnnnnn nnnnnnnn => buffer poiner
- 01nnnnnn nnnnnnnn => inlined data
- 10nnnnnn nnnnnnnn => on-disk reference
- 11nnnnnn nnnnnnnn => concatenated data pointer
.|-------'
|'-------.
v v
1nnnnnnn 0nnnnnnn <= leb128
We don't really care about 8-bit/7-bit, can we even fit an rbyd in a
127-byte block?
The main benefit of this encoding is that lfsr_data_t's pointer fields
get the same space as the two words used to encode on-disk block+off.
This may be useful on systems where ptr=2-word, such as some 16-bit
word/32-bit address devices, and some 2-word CHERI pointer devices.
One interesting thing to note: This encoding is only possible thanks to
the observation that total data size is sufficent information to write
out concatendated datas. We don't really need to know the exact number
until prog time, and during prog we can just iterate over datas until
size is exhausted.
So the size field turns out to be sufficient enough for indicating how
many datas are referenced, saving a data-count field.
Code changes are negligible. It should be noted that _most_ machines
won't benefit from ptr=2-word optimizations, including Thumb, our
benchmark ISA:
code stack
before: 33948 2872
after: 33912 (-0.1%) 2872 (+0.0%)
This matches bptr's cksize/cksum a bit better and helps avoids confusion
when discussing the various size fields used to encode a commit's
various checksum tags.
I find these little diagrams useful for visualizing the actual on-disk
encoding, which doesn't really exist in the code outside of the
lfsr_data_from* and lfsr_data_read* functions.
This should really be unsigned, rbyd weights can not be negative.
Note this is different than data.size, etc, since the signedness there
is used to differentiate the underlying encoding. Accessing data.size
directly is usually an error, though we do access it directly in several
places when assuming the underlying encoding. Signedness warnings are
actually a good thing in that case.
We just don't use this error since we assert. Having it in the error
enum may give the wrong impression we return it at points.
If we even end up needing it, it can be readded to the list.
We have bleafs (bleaves?) now, so the mleaf name just makes too much
sense. Even though it's used nowhere else outside of mid decoding, and
may be a bit confusing.
After all this time it feels weird to use a const lfs_t parameter, but
that's really what the mid/mleaf functions should take. These functions
are a bit of a special case as lfsr_mleafweight really wants to just be
a constant.
Code size did not change.
We still have an lfsr_shrub_t, it's just a simple alias of lfsr_rbyd_t.
The only difference between these two structs was that lfsr_rbyd_t had
the eoff/cksum fields, to enable incremental commits, and lfsr_shrub_t
had the estimate field, to keep track of the current shrub estimate so
we evict before overflow.
Unfortunately C makes this overlap a bit annoying. We can either add a
union, making a mess of field accesses, or use probably problematic
casting of structs with common initial sequences.
Instead of dealing with this headache, I'm just going to shove the
shrub estimate into the rbyd's eoff field and ignore the name abuse.
In normal rbyd use, eoff does effectively contain the on-disk size of
the rbyd, so it's not too far from its intended use...
This does move our estimate to overlap the eoff field instead of the
cksum field, which means we need to be a bit more careful about setting
erased state for btrees. This adds a small code cost:
code stack
before: 33928 2912
after: 33956 (+0.1%) 2912 (+0.0%)
Shrubness should have always been a property of lfsr_rbyd_t.
You know you've made a good design decision when things just sort of
fall into place and the code somehow becomes cleaner.
The downside of this change is accessing rbyd trunks requires a mask,
which is annoying, but the upside is we don't need to signal shrubness
via extra booleans in internal functions anymore.
The funny thing is, the actual motivation for this change is was just to
free up a bit in our tag encoding. Simplifying some of the internal
functions was just a nice side effect.
code stack
before: 33940 2928
after: 33928 (-0.0%) 2912 (-0.5%)
This is just too useful a namespace to not have in the low-level file
code.
This also replaces the ftree namespace with bshrub, which is a bit of a
more concrete term?
Note that some of the bshrub functions still take lfsr_file_t instead
of lfsr_bshrub_t. In _theory_ these could take 3 pointers (mdir+bshrub+
bshrub_), but this adds a surprising amount of code cost and we really
don't gain anything.
Since we need these for lfsr_dir_t (named b and p), we might as well
adopt one in lfsr_file_t (named m). This at least avoids a cast when
enrolling/unenrolling in the opened-mdir list.
This is just too enticing a simplification to avoid, even at a RAM cost.
By tracking the dir's bookmark as a separate mdir, we can trivially
deduplicate the logic to update dirs' mdirs. But this does significantly
increase the size of our lfsr_dir_t with the bookmark's type/flags/rbyd/
etc, which usually goes unused.
I guess this does optimize lfsr_dir_rewind... Is that ever a bottleneck?
Code changes:
code stack lfsr_dir_t
before: 34048 2944 48
after: 33956 (-0.3%) 2944 (+0.0%) 80 (+66.7%)
I was originally avoiding naming these orphans, as they're _technically_
not orphans. They do exist in the mtree. But the name orphan just
describes this types purpose too well.
This does lead to some confusing terms, such as the fact that orphan
files can be non-orphaned if there are any in-device references. But I
think this makes sense?
- LFSR_TAG_SCRATCH -> LFSR_TAG_ORPHAN
- LFSR_F_UNCREAT -> LFSR_F_ORPHAN
- test_fscratch.toml -> test_forphan.toml
A "zombie file" is a term I just made up to describe what happens when
you remove a file that is currently open.
To match POSIX, the opened file handle should still be available for
reading/writing, even though the file doesn't really exist in the
filesystem anymore.
We don't have inodes, which makes this a bit more complicated, but this
is where scratch files are handy again. By creating a scratch file when
we remove an opened file, we preserve the mid slot for the file's
sprout/shrub. We also mark the opened file as desync, so the existing
orphan reclaimation circuitry kicks in when the last file handle is
closed.
Really the only difference between zombie files and desync files is what
happens when you call lfsr_file_sync:
- Desynced lfsr_file_sync => Become synced, broadcast file state.
- Zombied lfsr_file_sync => Return ENOENT, you can't sync a zombie.
This _is_ a bit different from POSIX, where sync on a removed file
returns 0. I considered returning 0 in this case, but with all the extra
behavior around sync/desync state, I figured returning ENOENT was
clearer at indicating to the user sync is no longer possible.
Worst case, ENOENT is not returned from sync for any other reason, so
users can always treat ENOENT and 0 as the same in higher layers. The
zombie file is already desynced, so close will never error.
---
Implementation wise, zombies get a bit crazy.
Fortunately they add little extra code, but they make up for it by
adding extra subtlety. Zombie files introduce a ton of corner cases, now
even directories can have zombied shrubs.
This means more tests.
- Seemingly unrelated operations need to be able to remove scratch files
(mkdir, rename, etc).
- UNCREAT state needs to be broadcasted in seemingly unrelated
operations (mkdir, rename, etc).
- Zombied files need to be copied over during seemingly unrelated rename
operations.
- And I'm sure more corner cases I'm already forgetting.
One interesting tweak that simplifies things that's worth mentioning is
the change to the implicitly file mid updates on rm in lfsr_mdir_commit.
For non-reg files, an rm attr causes lfsr_mdir_commit to increment the
mid to the next mid in the mtree. This is the correct behavior for dirs,
traversals, etc.
Previously, reg files were a special case that marks the mid as -1. But
by changing this to also increment the mid, as well as set the zombie
flag, upper layers can broadcast zombie changes by simply creating a new
file and then deleting the old file in the same commit.
This seems to Just Work^TM, and avoids needing to do additional state
broadcasting in upper layers, which gets tricky since we may not know
exactly what the new mid is post-mdir-commit.
Downside: The order matters, we need to create the new file first. This
violates the normal delete-then-insert order we use elsewhere to avoid
overflow issues. This isn't that bad here, since we increment by at
most 1. But it is something to be wary of...
Still, this is much better than any other option I can think of right
now.
---
Uh, ignore the test_fscratch_rename* tests for now. I somehow forgot
file renaming was not yet implemented...
"Scratch files" are a new file type added to solve the zero-sized
file problem. Though they have a few other uses that may be quite
valuable.
The "zero-sized file problem" is a common surprise for users, where what
seems like a simple file create+write operation:
lfs_file_open(&lfs, &file, "hi",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL);
lfs_file_write(&lfs, &file, "hello!", strlen("hello!"));
lfs_file_close(&lfs, &file);
Can end up create a zero-sized file under powerloss, breaking user
assumptions and their code.
The tricky thing is that this is actually correct behavior as defined by
POSIX. `open` with O_CREAT creats a file entry immediately, which is
initially zero-sized. And the fact that power can be lost between `open`
and `close` isn't really avoidable.
But this is a common enough footgun that it's probably worth deviating
from POSIX here.
But how to avoid zero-sized files exactly? First thought: Delay the file
creation until sync/close, tracking uncreated files in-device until
then. This solves the problem and avoids any intermediary state if we
lose power, but came with a number of headaches:
1. Since we delay file creation, we don't immediately write the filename
to disk on open. This implies we need to keep the filename allocated
in RAM until the first sync/close call.
The requirement to keep the filename allocated for new files until
first sync/close could be added to open, and with the option to call
sync immediately to save the filename (and accept the risk of
zero-sized files), I don't think it would be _that_ bad of an API.
But it would still be pretty bad. Extra bad because 1. there's no
way to warn on misuse at compile-time, 2. use-after-free bugs have a
tendency to go unnoticed annoyingly often, 3. it's a regression from
the previous API, and 4. who the heck reads the more-or-less same
`open` documentation for every filesystem they adopt.
2. Without an allocated mid, tracking files internally gets a lot
harder. The best option I could think of was to keep the opened-file
linked-list sorted by mid + (in-device) file name.
This did not feel like a great solutiona and was going to add more
code cost.
3. Handling mdir splits containing uncreated files adds another
headache. Complicated lfsr_mdir_estimate further as it needs to
decide in which mdir the uncreated files will end up, and potentially
split on a filename that isn't even created yet.
4. Since the number of uncreated files can be potentially unbounded, you
can't prevent an mdir from filling up with only uncreated files. On
disk this ends up looking like an "empty" mdir, which need specially
handling in littlefs to reclaim after powerloss.
Support for empty mdirs -- the orphaned mdir scan -- was already
added earlier. We already scan each mdir to build gstate, so it
doesn't really add much cost.
Notice that last bullet point? We already scan each mdir during mount.
Why not, instead of scanning for orphaned mdirs, scan for orphaned
files?
So this leads to the idea of "scratch files". Instead of actually
delaying file creation, fake it. Create a scratch file during open, and
on the first sync/close, convert it to a regular file. If we lose power,
scan for scratch files during mount, and remove them on first write.
Some tradeoffs:
1. The orphan scan for scratch files is a bit more expensive than for
mdirs on storage with large block sizes. We need to look at each file
entry vs just each mdir, which pushed the runtime up to O(BlogB) vs
O(B).
Though if you also consider large mtrees, the worst case is still
O(nlogn).
2. Creating intermediate scratch files adds another commit to file
creation.
This is probably not a big issue for flash, but may be more of a
concern on devices with large prog sizes.
3. Scratch files complicate unrelated mkdir/rename/etc code a bit, since
we need to consider what happens when the dest is a scratch file.
But the end result is simple. And simple is good. Both for
implementation headaches, and code size. Even if the on-disk state is
conceptually more complicated.
You may have noticed these scratch files are basically isomorphic to
just setting an "uncreated" flag on the file, and that's true. There may
have been a simpler route to end up with the design, but hey, as long as
it works.
As a plus, scratch files present a solution for a couple other things:
1. Removing an open file can become a scratch file until closed.
2. Scratch files can be used as temporary files. Open a file with
O_DESYNC and never call sync and you have yourself a temporary file.
Maybe in the future we should add O_TMPFILE to avoid the need for
unique filenames, but that is low priority.
This should, in theory, be a transparent change for users
(https://xkcd.com/1172).
The motivation for this change:
1. Basically everyone uses O_RDONLY=0, O_WRONLY=1, O_RDWR=2, so
deviating from this ad-hoc standard risks surprising POSIX-familiar
users, though may confused POSIX-unfamiliar users.
But for the latter, we really shouldn't allow them to fall into the
trap that O_RDONLY | O_WRONLY == O_RDWR, because this will not work
on basically any other POSIX-like system.
2. I realized one benefit of the POSIX encoding is that it reserves the
value 3. Maybe this could be useful in the future?
Being able to create a file that neither readable nor writable isn't
all that useful...
Also, if you really think about the literal meaning of O_RDONLY |
O_WRONLY, these are negations. So O_RDONLY | O_WRONLY means you can only
write and only read? That sounds like an oxymoron.
Of course no one should be relying on these exact values, but these are
embedded systems! Someone somewhere is going to hack something together
that expects these to be their historically expected value. And we
shouldn't make things any harder for them unless there's a good reason.
Christopher Haster