9f2f0b92e93e8d9c0b433bd477cb71fef7b6353b
623 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Christopher Haster
|
db85172211 |
Always calculate/show cksum in dbgblock.py
Now that most scripts show relevant cksums, it makes sense for dbgblock.py to just always show a cksum as well. It's not like this has any noticable impact on the script's runtime. Example: $ ./scripts/dbgblock.py disk -b4096 0 block 0x0, size 4096, cksum e6e3ad25 00000000: 01 00 00 00 80 03 00 08 6c 69 74 74 6c 65 66 73 ........littlefs 00000010: 80 04 00 02 00 00 80 05 00 02 fa 01 80 09 00 04 ................ ... |
||
|
Christopher Haster
|
8a75a68d8b |
Made rbyd cksums erased-state agnostic
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%)
|
||
|
Christopher Haster
|
c4fcc78814 |
Tweaked file types/name tag encoding to be a bit less quirky
The intention behind the quirky encoding was to leverage bit 1 to
indicate if the underlying file type would be backed by the common file
B-tree data structure. Looking forward, there may be several of these
types, compressed files, contiguous files, etc, that for all intents and
purposes are just normal files interpreted differently.
But trying to leverage too many bits like this is probably going to give
us a sparse, awkward, and confusing tag encoding, so I've reverted to a
hopefully more normal encoding:
LFSR_TAG_NAME 0x02tt v--- --1- -ttt tttt
LFSR_TAG_NAME 0x0200 v--- --1- ---- ----
LFSR_TAG_REG 0x0201 v--- --1- ---- ---1
LFSR_TAG_DIR 0x0202 v--- --1- ---- --1-
LFSR_TAG_SYMLINK* 0x0203 v--- --1- ---- --11
LFSR_TAG_BOOKMARK 0x0204 v--- --1- ---- -1--
LFSR_TAG_ORPHAN 0x0205 v--- --1- ---- -1-1
LFSR_TAG_COMPR* 0x0206 v--- --1- ---- -11-
LFSR_TAG_CONTIG* 0x0207 v--- --1- ---- -111
* Hypothetical
Note the carve-out for the hypothetical symlink tag. Symlinks are
actually incredibly low in the priority list, but they are also
the only current hypothetical file type that would need to be exposed to
users. Grouping these up makes sense.
This will get a bit messy if we ever end up with a 4th user-facing type,
but there isn't any in POSIX at least (ignoring non-fs types, socket,
fifo, character, block, etc).
The gap also helps line things up so reg/orphan are a single bit flip,
and the non-user facing types all share a bit.
This had no impact on code size:
code stack
before: 33564 2816
after: 33564 (+0.0%) 2816 (+0.0%)
|
||
|
Christopher Haster
|
6e5d314c20 |
Tweaked struct tag encoding so b*/m* tags are earlier
These b*/m* struct tags have a common pattern that would be good to
emphasize in the encoding. The later struct tags get a bit more messy as
they leave space for future possible extensions.
New encoding:
LFSR_TAG_STRUCT 0x03tt v--- --11 -ttt ttrr
LFSR_TAG_DATA 0x0300 v--- --11 ---- ----
LFSR_TAG_BLOCK 0x0304 v--- --11 ---- -1rr
LFSR_TAG_BSHRUB 0x0308 v--- --11 ---- 1---
LFSR_TAG_BTREE 0x030c v--- --11 ---- 11rr
LFSR_TAG_MROOT 0x0310 v--- --11 ---1 --rr
LFSR_TAG_MDIR 0x0314 v--- --11 ---1 -1rr
LFSR_TAG_MSHRUB* 0x0318 v--- --11 ---1 1---
LFSR_TAG_MTREE 0x031c v--- --11 ---1 11rr
LFSR_TAG_DID 0x0320 v--- --11 --1- ----
LFSR_TAG_BRANCH 0x032c v--- --11 --1- 11rr
* Hypothetical
Note that all shrubs currently end with 1---, and all btrees, including
the awkward branch tag, end with 11rr.
This had no impact on code size:
code stack
before: 33564 2816
after: 33564 (+0.0%) 2816 (+0.0%)
|
||
|
Christopher Haster
|
5fa85583cd |
Dropped block-level erased-state checksums for RAM-tracked erased-state
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%)
|
||
|
Christopher Haster
|
3e7a4e1d74 |
In dbgblock.py, renamed -x/--cksum -> -c/--cksum
I think this makes a bit more sense. I think the original reasoning for -x/--cksum was to match -x/--device in dbgrbyd.py, but that flag no longer exists. This could go all the way back to matching --xsum at some point, but I'm not sure. Common hash related utils, sha256sum, md5sum, etc, use -c/--check to validate their hash, so that's sort of prior art? |
||
|
Christopher Haster
|
81ccfbccd0 |
Dropped -x/--device from dbg*.py scripts
This hasn't really proven useful. At one point showing the cksums in dbgrbyd.py was useful, but this is now possible and easier with dbgblock.py -x/--cksum. |
||
|
Christopher Haster
|
86a8582445 |
Tweaked canonical altn to point to itself
By definition, altns should never be followed, so it doesn't really
matter where they point. But it's not like they can point literally
nowhere, so where should they point?
A couple options:
1. jump=jump - Wherever the old alt pointed
- Easy, literally a noop
- Unsafe, bugs could reveal outdated parts of the tree
- Encoding size eh
2. jump=0 - Point to offset=0
- Easier, +0 code
- Safer, branching to 0 should assert
- Worst possible encoding size
3. jump=itself - Point to itself
- A bit tricky, +4 code
- Safe, should assert, even without asserts worst case infinite loop
- Optimal encoding size
An infinite loop isn't the best failure state, but we can catch this
with an assert, which we would need for jump=0 anyways. And this is only
a concern if there are other fs bugs. jump=0 is actually slightly worse
if asserts are disabled, since we'd end up reading the revision count as
garbage.
Adopting jump=itself gives us the optimal 4-byte encoding:
altbn w0 = 40 00 00 00
'-+-' ^ ^
'----|--|-- tag = altbn
'--|-- weight = 0
'-- jump = itself (branch - 0)
This requires tweaking the alt encoder a bit, to avoid relative encoding
jump=0s, but this is pretty cheap:
code stack
jump=jump: 34068 2864
jump=0: 34068 (+0.0%) 2864 (+0.0%)
jump=itself: 34072 (+0.0%) 2864 (+0.0%)
I thought we may need to also tweak the decoder, so later trunk copies
don't accidentally point to the old location, but humorously our pruning
kicks in redundantly to reset altbn's jump=itself on every trunk.
Note lfsr_rbyd_lookupnext was also rearranged a bit to make it easier to
assert on infinite loops and this also added some code. Probably just
due to compiler noise:
code stack
before: 34068 2864
after: 34076 (+0.0%) 2864 (+0.0%)
Also note that we still accept all of the above altbn encoding options.
This only affects encoding and dbg scripts.
|
||
|
Christopher Haster
|
faf8c4b641 |
Tweaked alt-tag encoding to match color/dir naming order
This is mainly to avoid mistakes caused by names/encodings disagreeing:
LFSR_TAG_ALT 0x4kkk v1cd kkkk -kkk kkkk
^ ^^ '------+-----'
'-||--------|------- valid bit
'|--------|------- color
'--------|------- dir
'------- key
Notably, the LFSR_TAG_ALT() macro has already caused issues by being
both 1. ambiguous, and 2. not really type-checkable. It's easy to get
the order wrong and things not really break, just behave poorly, it's
really not great!
To be honest the exact order is a bit arbitrary, the color->dir naming
appeared by accident because I guess it felt more natural. Maybe because
of English's weird implicit adjective ordering? Maybe because of how
often conditions show up as the last part of the name in other
instruction sets?
At least one plus is that this moves the dir-bit next to the key. This
makes it so all of the condition information is encoding is the lowest
13-bits of the tag, which may lead to minor optimization tricks for
implementing flips and such.
Code changes:
code stack
before: 34080 2864
after: 34068 (-0.0%) 2864 (+0.0%)
|
||
|
Christopher Haster
|
37c45e1afc |
Fixed coloring conflicts in rbyd tree renderers
A bit of a hack, but rather than handling conditional alt branches, our dbg rbyd tree renderers just represent single-pointer alts as an alt with both branches pointing to the place. Unfortunately, the two branches technically have different colors. This resulted in a bit of contention when chosing how to color the tree. Basically Python's dict ordering would determine which color won. Which was a bit confusing when dbgrbyd.py displayed different tree colorings for the same rbyd. dbgrbyd.py should be idempotent! This is solved by adding another hack to check explicitly for same-destination branches. |
||
|
Christopher Haster
|
c3dc7cca10 |
Fixed underflow issue with truncating test/bench -C/--context
There was no check on context > stdout, so requesting more context than was actually printed by the test could result in a negative value. Python "helpfully" interpreted this as a negative index, resulting in somewhat random context lengths. This, combined with my tendency to just default to a large number like --context=100, led to me thinking a test was printing much less than it actually was... Don't get me wrong, I love Python, and I think Python's negative indices are a clever way to add flexibility to slice notation, but the value-dependent semantics are a pretty unfortunate footgun... |
||
|
Christopher Haster
|
2dcde5579b |
Fixed issue with test.py/bench.py -f/--fail not killing runners
While the -f/--fail logic was correctly terminating the test.py/bench.py runner thread, it was not terminating the actual underlying test process. This was causing test.py/bench.py to hang until the test runner completed all pending tests, which could take quite some time. This wasn't noticed earlier because test.py/bench.py still reports the test as failed, and most uses of -f/--fail involve specifying a specific test case, which usually terminates quite quickly. What's more interesting is this termination logic was copied from the handling of ctrl-C/SIGINT/KeyboardInterrupt, but this issue is not present there because SIGINT would be sent to all processes in the process tree, terminating the child process anyways. Fixed by adding an explicit proc.kill() to test.py/bench.py before tearing down the runner thread. |
||
|
Christopher Haster
|
8a646d5b8e |
Added dbgtag.py for easy tag decoding on the command-line
Example: $ ./scripts/dbgtag.py 0x3001 cksum 0x01 dbgtag.py inherits most of crc32c.py's decoding options. The most useful probably being -x/--hex: $ ./scripts/dbgtag.py -x e1 00 01 8a 09 altbgt 0x100 w1 -1162 dbgtag.py also supports reading from a block device if either -b/--block-size or --off are provided. This is mainly for consistency with the other dbg*.py scripts: $ ./scripts/dbgtag.py disk -b4096 0x2.1e4 bookmark w1 1 This should help when debugging and finding a raw tag/alt in some register. Manually decoding is just an unnecessary road bump when this happens. |
||
|
Christopher Haster
|
fe772e08cd |
Added dbgcat.py for extracted raw data from block devices
dbgcat.py is basically the same as dbgblock.py except:
- dbgcat.py pipes the block's contents directly to stdout.
The intention is to enable external tools with Unix pipes while
letting dbgcat.py take care of block address decoding:
$ ./scripts/dbgcat.py disk -b4096 0x1.8 -n8 | grep littlefs
littlefs
- Unlike dbgblock.py, dbgcat.py accepts multiple block addresses,
concatenating the data that resides at those blocks.
This is dbgCAT.py after all.
|
||
|
Christopher Haster
|
9905bd397a |
Extended crc32c.py to support hex sequences and strings
So now the following forms are supported: $ ./scripts/crc32c.py -x 41 42 43 44 fb9f8872 $ ./scripts/crc32c.py -s abcd fb9f8872 $ echo '00: 41 42 43 44' | xxd -r | ./scripts/crc32c.py fb9f8872 Hopefully this will make crc32c.py more useful. It hasn't seen very much use, though that may just be because of the difficulty marshalling data into a format crc32c.py can operate on. That and dbgblock.py's -x/--cksum flag covering one of the main use cases. |
||
|
Christopher Haster
|
54a03cfe3b |
Enabled both pruning/non-pruning dbg reprs, -t/--tree and -R/--rbyd
Now that altns/altas are more important structurally, including them in
our dbg script's tree renderers is valuable for debugging. On the other
hand, they do add quite a bit of visual noise when looking at large
multi-rbyd trees topologically.
This commit gives us the best of both worlds by making both tree
renderings available under different options:
-t/--tree, a simplified rbyd tree renderer with altn/alta pruning:
.-> 0 reg w1 4
.-+-> uattr 0x01 2
| .-> uattr 0x02 2
.---+-+-> uattr 0x03 2
| .-> uattr 0x04 2
| .-+-> uattr 0x05 2
| .-+---> uattr 0x06 2
+-+-+-+-+-> 1 reg w1 4
| | '-> 2 reg w1 4
| '---> uattr 0x01 2
'---+-+-+-> uattr 0x02 2
| | '-> uattr 0x03 2
| '-+-> uattr 0x04 2
| '-> uattr 0x05 2
| .-> uattr 0x06 2
| .-+-> uattr 0x07 2
| | .-> uattr 0x08 2
'-+-+-> uattr 0x09 2
-R/--rbyd, a full rbyd tree renderer:
.---> 0 reg w1 4
.---+-+-> uattr 0x01 2
| .---> uattr 0x02 2
.-+-+-+-+-> uattr 0x03 2
| .---> uattr 0x04 2
| .-+-+-> uattr 0x05 2
| .-+---+-> uattr 0x06 2
+---+-+-+-+-+-> 1 reg w1 4
| | '-> 2 reg w1 4
| '-----> uattr 0x01 2
'-+-+-+-+-+-+-> uattr 0x02 2
| | '---> uattr 0x03 2
| '---+-+-> uattr 0x04 2
| '---> uattr 0x05 2
| .---> uattr 0x06 2
| .-+-+-> uattr 0x07 2
| | .-> uattr 0x08 2
'-----+---+-> uattr 0x09 2
And of course -B/--btree, a simplified B-tree renderer (more useful for
multi-rbyds):
+-> 0 reg w1 4
| uattr 0x01 2
| uattr 0x02 2
| uattr 0x03 2
| uattr 0x04 2
| uattr 0x05 2
| uattr 0x06 2
|-> 1 reg w1 4
'-> 2 reg w1 4
uattr 0x01 2
uattr 0x02 2
uattr 0x03 2
uattr 0x04 2
uattr 0x05 2
uattr 0x06 2
uattr 0x07 2
uattr 0x08 2
uattr 0x09 2
|
||
|
Christopher Haster
|
abe68c0844 |
rbyd-rr: Reworking rbyd range removal to try to preserve rby structure
This is the start of (yet another) rework of rybd range removals, this
time in an effort to preserve the rby structure that maps to a balanced
2-3-4 tree. Specifically, the property that all search paths have the
same number of black edges (2-3-4 nodes).
This is currently incomplete, as you can probably tell from the mess,
but this commit at least gets a working altn/alta encoding in place
necessary for representing empty 2-3-4 nodes. More on that below.
---
First the problem:
My assumption, when implementing the previous range removal algorithms,
was that we only needed to maintain the existing height of the tree.
The existing rbyd operations limit the height to strictly log n. And
while we can't _reduce_ the height to maintain perfect balance, we can
at least avoid _increasing_ the height, which means the resulting tree
should have a height <= log n. Since our rbyds are bounded by the
block_size b, this means worst case our rbyd can never exceed a height
<= log b, right?
Well, not quite.
This is true the instance after the remove operation. But there is an
implicit assumption that future rbyd operations will still be able to
maintain height <= log n after the remove operation. This turns out to
not be true.
The problem is that our rbyd appends only maintain height <= log n if
our rby structure is preserved. If the rby structure is broken, rbyd
append assumes an rby structure that doesn't exist, which can lead to an
increasingly unbalanced tree.
Consider this happily balanced tree:
.-------o-------. .--------o
.---o---. .---o---. .---o---. |
.-o-. .-o-. .-o-. .-o-. .-o-. .-o-. |
.o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. |
a b c d e f g h i j k l m n o p => a b c d e f g h i
'------+------'
remove
After a range removal it looks pretty bad, but note the height is still
<= log n (old n not the new n). We are still <= log b.
But note what happens if we start to insert attrs into the short half of
the tree:
.--------o
.---o---. |
.-o-. .-o-. |
.o. .o. .o. .o. |
a b c d e f g h i
.-----o
.--------o .-+-r
.---o---. | | | |
.-o-. .-o-. | | | |
.o. .o. .o. .o. | | | |
a b c d e f g h i j'k'l'
.-------------o
.---o .---+-----r
.--------o .-o .-o .-o .-+-r
.---o---. | | | | | | | | | |
.-o-. .-o-. | | | | | | | | | |
.o. .o. .o. .o. | | | | | | | | | |
a b c d e f g h i j'k'l'm'n'o'p'q'r'
Our right side is generating a perfectly balanced tree as expected, but
the left side is suddenly twice as far from the root! height(r')=3,
height(a)=6!
The problem is when we append l', we don't really know how tall the tree
is. We only know l' has one black edge, which assuming rby structure is
preserved, means all other attrs must have one black edge, so creating a
new root is justified.
In reality this just makes the tree grow increasingly unbalanced,
increasing the height of the tree by worst case log n every range
removal.
---
It's interesting to note this was discovered while debugging
test_fwrite_overwrite, specifically:
test_fwrite_overwrite:1181h1g2i1gg2l15o10p11r1gg8s10
It turns out the append fragments -> delete fragments -> append/carve
block + becksum loop contains the perfect sequence of attrs necessary to
turn this tree inbalance into a linked-list!
.-> 0 data w1 1
.-b-> 1 data w1 1
| .-> 2 data w1 1
.-b-b-> 3 data w1 1
| .-> 4 data w1 1
| .-b-> 5 data w1 1
| | .-> 6 data w1 1
.---b-b-b-> 7 data w1 1
| .-> 8 data w1 1
| .-b-> 9 data w1 1
| | .-> 10 data w1 1
| .-b-b-> 11 data w1 1
| .-b-----> 12 data w1 1
.-y-y-------> 13 data w1 1
| .-> 14 data w1 1
.-y---------y-> 15 data w1 1
| .-> 16 data w1 1
.-y-----------y-> 17 data w1 1
| .-> 18 data w1 1
.-y-------------y-> 19 data w1 1
| .-> 20 data w1 1
.-y---------------y-> 21 data w1 1
| .-> 22 data w1 1
.-y-----------------y-> 23 data w1 1
| .-> 24 data w1 1
.-y-------------------y-> 25 data w1 1
| .---> 26 data w1 1
| | .-> 27-2047 block w2021 10
b-------------------r-b-> becksum 5
Note, to reproduce this you need to step through with a breakpoint on
lfsr_bshrub_commit. This only shows up in the file's intermediary btree,
which at the time of writing ends up at block 0xb8:
$ ./scripts/test.py \
test_fwrite_overwrite:1181h1g2i1gg2l15o10p11r1gg8s10 \
-ddisk --gdb -f
$ ./scripts/watch.py -Kdisk -b \
./scripts/dbgrbyd.py -b4096 disk 0xb8 -t
(then b lfsr_bshrub_commit and continue a bunch)
---
So, we need to preserve the rby structure.
Note pruning red/yellow alts is not an issue. These aren't black, so we
aren't changing the number of black edges in the tree. We've just
effectively reduced a 3/4 node into a 2/3 node:
.-> a
.---b-> b .-> a <- 2 black
| .---> c .-b-> b
| | .-> d | .-> c
b-r-b-> e <- rm => b-b-> d <- 2 black
The tricky bit is pruning black alts. Naively this changes the number of
black edges/2-3-4 nodes in the tree, which is bad:
.-> a
.-b-> b .-> a <- 2 black
| .-> c .-b-> b
b-b-> d <- rm => b---> c <- 1 black
It's tempting to just make the alt red at this point, effectively
merging the sibling 2-3-4 node. This maintains balance in the subtree,
but still removes a black edge, causing problems for our parent:
.-> a
.-b-> b .-> a <- 3 black
| .-> c .-b-> b
.-b-b-> d | .-> c
| .-> e .-b-b-> d
| .-b-> f | .---> e
| | .-> g | | .-> f
b-b-b-> h <- rm => b-r-b-> g <- 2 black
In theory you could propagate this all the way up to the root, and this
_would_ probably give you a perfect self-balancing range removal
algorithm... but it's recursive... and littlefs can't be recursive...
.-> s
.-b-> t .-> s
| .-> u .-----b-> t
.-b-b-> v | .-> u
| .-> w | .---b-> v
| .-b-> x | | .---> w
| | | | .-> y | | | | | | | .-> x
b-b- ... b-b-b-> z <- rm => r-b-r-b- ... r-b-r-b-> y
So instead, an alternative solution. What if we allowed black alts that
point nowhere? A sort of noop 2-3-4 node that serves only to maintain
the rby structure?
.-> a
.-b-> b .-> a <- 2 black
| .-> c .-b-> b
b-b-> d <- rm => b-b-> c <- 2 black
I guess that would technically make this 1-2-3-4 tree.
This does add extra overhead for writing noop alts, which are otherwise
useless, but it seems to solve most of our problems: 1. does not
increase the height of the tree, 2. maintains the rby structure, 3.
tail-recursive.
And, thanks to the preserved rby structure, we can say that in the worst
case our rbyds will never exceed height <= log b again, even with range
removals.
If we apply this strategy to our original example, you can see how the
preserved rby structure sort of "absorbs" new red alts, preventing
further unbalancing:
.-------o-------. .--------o
.---o---. .---o---. .---o---. o
.-o-. .-o-. .-o-. .-o-. .-o-. .-o-. o
.o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. .o. o
a b c d e f g h i j k l m n o p => a b c d e f g h i
'------+------'
remove
Reinserting:
.--------o
.---o---. o
.-o-. .-o-. o
.o. .o. .o. .o. o
a b c d e f g h i
.----------------o
.---o---. o
.-o-. .-o-. .------o
.o. .o. .o. .o. .o. .-+-r
a b c d e f g h i j'k'l'm'
.----------------------------o
.---o---. .-------------o
.-o-. .-o-. .---o .---+-----r
.o. .o. .o. .o. .-o .-o .-o .-o .-+-r
a b c d e f g h i j'k'l'm'n'o'p'q'r's'
Much better!
---
This commit makes some big steps towards this solution, mainly codifying
a now-special alt-never/alt-always (altn/alta) encoding to represent
these noop 1 nodes.
Technically, since null (0) tags are not allowed, these already exist as
altle 0/altgt 0 and don't need any extra carve-out encoding-wise:
LFSR_TAG_ALT 0x4kkk v1dc kkkk -kkk kkkk
LFSR_TAG_ALTN 0x4000 v10c 0000 -000 0000
LFSR_TAG_ALTA 0x6000 v11c 0000 -000 0000
We actually already used altas to terminate unreachable tags during
range removals, but this behavior was implicit. Now, altns have very
special treatment as a part of determining bounds during appendattr
(both unreachable gt/le alts are represented as altns). For this reason
I think the new names are warranted.
I've also added these encodings to the dbg*.py scripts for, well,
debuggability, and added a special case to dbgrby.py -j to avoid
unnecessary altn jump noise.
As a part of debugging, I've also extended dbgrbyd.py's tree renderer to
show trivial prunable alts. Unsure about keeping this. On one hand it's
useful to visualize the exact alt structure, on the other hand it likely
adds quite a bit of noise to the more complex dbg scripts.
The current state of things is a mess, but at least tests are passing!
Though we aren't actually reclaiming any altns yet... We're definitely
_not_ preserving the rby structure at the moment, and if you look at the
output from the tests, the resulting tree structure is hilarious bad.
But at least the path forward is clear.
|
||
|
Christopher Haster
|
531c2bcc4c |
Quieted test.py/bench.py status when stdout is aimed at stdout
This is a condition for specifically the -O- pattern. Doing anything fancier would be too much, so anything clever such as -O/dev/stdout will still be clobbered. This was a common enough pattern and the status updates clobbering stdout was annoying enough that I figured this warranted a special case. |
||
|
Christopher Haster
|
76593711ab |
Added -f/--fail to test.py/bench.py
This just tells test.py/bench.py to pretend the test failed and trigger any conditional utilities. This can be combined with --gdb to easily inspect a test that isn't actually failing. Up until this point I've just been inserting assert(false) when needed, which is clunky. |
||
|
Christopher Haster
|
62de865103 |
Eliminated null tag reachability in dbg scripts
This was throwing off tree rendering in dbglfs.py, we attempt to lookup the null tag because we just want to first tag in the tree to stitch things together. Null tag reachability is tricky! You only notice if the tree happens to create a hole, which isn't that common. I think all lookup implementations should have this max(tag, 1) pattern from now on to avoid this. Note that most dbg scripts wouldn't run into this because we usually use the traversal tag+1 pattern. Still, the inconsistency in impl between the dbg scripts and lfs.c is bad. |
||
|
Christopher Haster
|
3eb4ccdde7 |
Fixed block/becksum related tree rendering in dbglfs.py
The were a couple issues mixing high-level and low-level bptrs
representations:
1. The high-level vs low-level block representation needed to have
ordering priority over the actual tag in order for inner-node tree
renderings to make sense.
2. We need to flatten all tags to BLOCK/DATA/other data tags when _not_
rendering inner-nodes so interleaved becksums/other util tags don't
mess with the tree rendering.
Now things look like this:
littlefs v0.0 4096x256 0x{0,1}.a0c, rev 15, weight 0.512
{0000,0001}: -1.1 hello reg 1113, btree 0x27.8d8
0000.0a11: + 0-1112 btree w1113 9
0027.08d8: | .-+ 0-1112 block w1113 11
'-+-| > becksum 5
'-> 0-1112 block w1113 0x95.0 1113
Maybe a bit weird looking at first, but correct.
|
||
|
Christopher Haster
|
9366674416 |
Replaced separate BLOCKSIZE/BLOCKCOUNT attrs with single GEOMETRY attr
This saves a bit of rbyd overhead, since these almost always come
together.
Perhaps more interesting, it carves out space for storing mroot-anchor
redundancy information. This uses the lowest two bits of the GEOMETRY
tag to indicate how many redundant blocks belong to the mroot-anchor:
LFSR_TAG_GEOMETRY 0x0008 v--- ---- ---- 1-rr
This solves a bit of a hole in our redundancy encoding. The plan is for
this info to be stored in the lowest two bits of every pointer, but the
mroot-anchor doesn't really have a pointer.
Though this is just future plans. Right now the redundancy information
is unused. Current implementations should use the GEOMETRY tag 0x0009,
which you may notice implied redundancy level-1. This matches our
current 2-block per mdir default.
Geometry attr encoding:
.---+---+---+---. tag (0x0008+r): 1 be16 2 bytes
|x0008+r| 0 |siz| weight (0): 1 leb128 1 byte
+---+---+---+---+ size: 1 leb128 1 byte
| block_size | block_size: 1 leb128 <=4 bytes
+---+- -+- -+- -+- -.
| block_count | block_count: 1 leb128 <=5 bytes
'---+- -+- -+- -+- -' total: <=13 bytes
Code changes:
code stack
before: 34092 2880
after: 34040 (-0.2%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
130281ac05 |
Reworked compat flags a bit
Now with a bit more granularity for possibly-future-optional on-disk
data structures:
LFSR_RCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved)
LFSR_RCOMPAT_MLEAF 0x0002 ---- ---- ---- --1-
LFSR_RCOMPAT_MSHRUB 0x0004 ---- ---- ---- -1-- (reserved)
LFSR_RCOMPAT_MTREE 0x0008 ---- ---- ---- 1---
LFSR_RCOMPAT_BSPROUT 0x0010 ---- ---- ---1 ----
LFSR_RCOMPAT_BLEAF 0x0020 ---- ---- --1- ----
LFSR_RCOMPAT_BSHRUB 0x0040 ---- ---- -1-- ----
LFSR_RCOMPAT_BTREE 0x0080 ---- ---- 1--- ----
LFSR_RCOMPAT_GRM 0x0100 ---- ---1 ---- ----
LFSR_WCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved)
LFSR_OCOMPAT_NONSTANDARD 0x0001 ---- ---- ---- ---1 (reserved)
This adds a couple reserved flags:
- LFSR_*COMPAT_NONSTANDARD - This flag will never be set by a standard
version of littlefs. The idea is to allow implementations with
non-standard extensions a way to signal potential compatibility issues
without worrying about future compat flag conflicts.
This is limited to a single bit, but hey, it's not like it's possible
to predict all future extensions.
If a non-standard extension needs more granularity, reservations of
standard compat flags can always be requested, even if they don't end
up implemented in standard littlefs. (Though such reservations will
need a strong motivation, it's not like these flags are free).
- LFSR_RCOMPAT_MSHRUB - In theory littlefs supports a shrubbed mtree,
where the root is inlined into the mroot. But in practice this turned
out to be more complicated than it was worth. Still, a future
implementation may find an mshrub useful, so preserving a compat flag
for such a case makes sense.
That being said, I have no plans to add support for mshrubs even in
the dbg scripts.
I would like the expected feature-set for debug tools to be
well-defined, but also conservative. This gets a bit tricky with
theoretical features like the mshrubs, but until mshrubs are actually
implemented in littlefs, I would like to consider them non-standard.
The implication of this is that, while LFSR_RCOMPAT_MSHRUB is
currently "reserved", it may be repurposed for some other meaning in
the future.
These changes also rename *COMPATFLAGS -> *COMPAT, and reorder the tags
by decreasing importance. This ordering seems more valuable than the
original intention of making rcompat/wcompat a single bit flip.
Implementation-wise, it's interesting to note the internal-only
LFSR_*COMPAT_OVERFLOW flag. This gets set when out-of-range bits are set
on-disk, and allows us to detect unrepresentable compat flags without
too much extra complexity.
The extra encoding/decoding overhead does add a bit of cost though:
code stack
before: 33944 2880
after: 34124 (+0.5%) 2880 (+0.0%)
|
||
|
Christopher Haster
|
d8d6052d90 |
Dropped -m/--mleaf-weight from dbg scripts
Now that we're assuming a perfect compaction algorithm, and an infinitely compatible mleaf-bits, there really shouldn't be any reason to support non-standard mleaf-bits in our scripts, right? If a configurable mleaf-bits becomes necessary, we can always add this back in the future. |
||
|
Christopher Haster
|
23aab1a238 |
Increased mleaf-bits to account for better compaction algorithms
As defined previously, mleaf-bits depended on the attr estimate, which
depended on the details of our compaction algorithm:
block_size
m = ----------
a_0
Assuming t=4, the _minimum_ tag encoding:
block_size block_size
m = ---------- = ----------
3*4 + 4 16
However, with our new compaction algorithm, our attr estimate changes:
block_size block_size block_size
m = ---------- = ----------- = ----------
a_1 (5/2)*4 + 2 12
But tying our mleaf-bits to our attr estimate is a bit fragile. Unlike
attr estimate, the calculated mleaf-bits MUST be the same across all
littlefs implementations, or else the filesystem may not be mountable.
We _could_ store mleaf-bits as an fs attr in the mroot, like we do with
name-limit, size-limit, block-size, etc, but I'd prefer to not add fs
attrs unless strictly required. Each fs attr adds complexity to mounting,
which has a non-zero cost and headache.
Instead, we can assume our compaction algorithm is perfect:
block_size block_size block_size
m = ---------- = ---------- = ----------
a_inf 2*4 8
This isn't actually achievable without unbounded RAM. But just because
our current implementation is limited to bounded RAM, does not prevent
some other implementation from pushing things further with unbounded
RAM.
In theory, since this is a perfect compaction algorithm, and builds
perfect rbyd trunks, this should be the maximum possible mleaf-bits
achievable in littlefs's current design, and should be compatible with
any future implementation.
---
Worst case, we can always add mleaf-bits as an fs attr retroactively
without breaking backwards compatibility. You would just need to assume
the above block_size-dependent value if the hypothetical mleaf-bits attr
is missing.
This is one nice thing about our fs attr system, it's very flexible.
|
||
|
Christopher Haster
|
6c9ce4e8f1 |
Reverted raw-byte comparisons for rbyd/btree namelookups
Implementing raw-byte name comparisons ended up having more negative
effects on implementation requirements than I thought it would:
1. We would never actually concatenate the did + name, as that would
require dynamic memory. Instead we need to express the concatenated
relationship using our internal lfsr_data_t representation.
I thought this wouldn't be too bad since we already have a
concatenated lfsr_data_t representation, but:
1. It was limited in scope, specifically only lfsr_data_prog was
supported. It's actually not even possible to implement
lfsr_data_read (I think) since we can't mutate the indirect
lfsr_data_ts.
2. It's not actually required. We really only use our concatenated
representation to coalesce file fragments. You could in theory
omit this representation at the cost of not being able to limit
inlined shrub overhead.
Asking all future littlefs implementations to implement a
concatenated data representation (or dynamically allocate D:) for the
basic task of file-name lookup is sort of a big ask.
2. A readonly implementation suddenly needs a toleb128 function.
Which is an unexpected implication of requiring raw-byte leb128
comparisons for file-name lookup.
3. Raw-byte comparisons require that dids are always stored in their
canonical encoding (smallest leb128), though this is probably a good
idea anyways.
And for what? A theoretical future-planned feature (content-tree)?
Let's think about the hypothetical content-tree for a second:
1. It's an advanced, opt-in feature. Which means higher code/storage-cost
should be expected.
2. Basicall all littlefs implementations need file-name lookup, so
keeping file-name lookup cheap is a much higher priority than the
opt-int content-tree.
3. Worst case, the content-tree, and any future named trees, can just
set did=0. This will cost one byte per name (and may leave room for
future extensions).
So I'm reverting this for now.
There is still time before stabilization, so if it becomes clear there
is a better way to implement name lookups, we can still change this.
(Optimistically, the content-tree may be implemented before
stabilization, since it currently looks like it's required for data
redundancy).
Code changes:
code stack
before: 34292 2896
after: 34028 (-0.8%) 2896 (+0.0%)
|
||
|
Christopher Haster
|
94f7d2549f |
Changed rbyd/btree namelookups to only compare raw bytes
This is a simplification of the rbyd/btree layers, but implies
behavioral changes to the mtree/mdir layers.
Instead of ordering by leb128 did + name:
82 02 61 61 61 < 81 04 62 62 62
(0x102, "aaa") (0x201, "bbb")
We now order by the raw encoding, lexicographically:
82 02 61 61 61 > 81 04 62 62 62
(0x102, "aaa") (0x201, "bbb")
This may be unintuitive, but note:
1. Files _within_ a directory are still ordered, since they share a did
prefix.
2. We don't really care about the relative ordering of dids, just
that they are unique. Changing the ordering at this level does not
interfere with any of our did-related functions.
3. The only thing we may care about is that the root, did=0, is the
first mtree entry. This is still true. No leb128 encoding is < 0x00
even after encoding.
The motivation for this change is to allow for other named-btrees in the
system that may used non-did-prefixed names. At least one of these makes
sense for a sort of "content-tree" (cksum -> data block mapping).
As a plus, this change makes it possible to compare names and do btree
namelookups without needing to decode the leb128 prefix. Although I'm
struggling a bit to figure out exactly where this is useful...
One downside, this ordering only works if dids are always stored in
their canonical encoding, that is, the smallest leb128 encoding possible
for a given did. I think this is a reasonable requirement for just our
dids.
Another downside is this did add a decent chunk of code.
I did try limiting the changes to lfsr_data_namecmp, but it didn't have
much impact. I guess most of the cost comes from the reworked
lfsr_data_cmp function, which, to be fair, is quite a bit more
complicated now (it now supports limited data<=>data comparisons):
code stack
before: 34148 2896
namecmp: 34324 (+0.5%) 2896 (+0.0%)
after: 34340 (+0.6%) 2896 (+0.0%)
|
||
|
Christopher Haster
|
5128522fe2 |
Renamed script flag -Z/--depth -> -z/--depth
Previously, the intention of upper case -Z was the match -W/--width and -H/--height, which are uppercase to avoid conflicts with -h/--help. But -z/--depth isn't _really_ related to -W/-H. This avoids a conflict with -Z/--lebesgue, but may conflict with -z/--cat. Fortunately we don't currently have any conflicts with the latter. Since -z/--depth and -Z/--lebesgue are both disk-layout related, the risk of conflicts are probably much higher there. |
||
|
Christopher Haster
|
2d2c0f19ff |
Renamed block-size flag in scripts from -B -> b
So now these should be invoked like so: $ ./scripts/dbglfs.py -b4096x256 disk The motivation for this change is to better match other filesystem tooling. Some prior art: - mkfs.btrfs - -n/--nodesize => node size in bytes, power of 2 >= sector - -s/--sectorsize => sector size in bytes, power of 2 - zfs create - -b => block size in bytes - mkfs.xfs - -b => block size in bytes, power of 2 >= sector - -s => sector size in bytes, power of 2 >= 512 - mkfs.ext[234] - -b => block size in bytes, power of 2 >= 1024 - mkfs.ntfs - -c/--cluster-size => cluster size in bytes, power of 2 >= sector - -s/--sector-size => sector size in bytes, power of 2 >= 256 - mkfs.fat - -s => cluster size in sectors, power of 2 - -S => sector size in bytes, power of 2 >= 512 Why care so much about the flag naming for internal scripts? The intention is for external tooling to eventually use the same set of flags. And maybe even create publically consumable versions of the dbg scripts. It's important that if/when this happens flags stay consistent. Everyone familiar with the ssh -p/scp -P situation knows how annoying this can be. It's especially important for littlefs's -b/--block-size flag, since this will likely end up used everywhere. Unlike other filesystems, littlefs can't mount without knowing the block-size, so any tool that mounts littlefs is going to need the -b/--block-size flag. --- The original motivation for -B was to avoid conflicts with the -b/--by flag that was already in use in all of the measurement scripts. But these are internal, and not really littlefs-related, so I don't think that's a good reason any more. Worst case we can just make the --by flag -B, or just not have a short form (--by is only 4 letters after all). Somehow we ended up with no scripts needing both -b/--block-size and -b/--by so far. Some other conflicts/inconsistencies tweaks were needed, here are all the flag changes: - -B/--block-size -> -b/--block-size - -M/--mleaf-weight -> -m/--mleaf-weight - -b/--btree -> -B/--btree - -C/--block-cycles -> -c/--block-cycles (in tracebd.py) - -c/--coalesce -> -S/--coalesce (in tracebd.py) - -m/--mdirs -> -M/--mdirs (in dbgbmap.py) - -b/--btrees -> -B/--btrees (in dbgbmap.py) - -d/--datas -> -D/--datas (in dbgbmap.py) |
||
|
Christopher Haster
|
7d95a2ff29 |
Added ability to disable default patterns in prettyasserts.py
- -n/--no-defaults - disable default patterns
The default patterns can be brought back explicitly with:
- -a/--assert - enable assert pattern
- -u/--unreachable - enable unreachable pattern
- -A/--arrow - enable arrow patterns
Technically the default configuration is equivalent to the follow:
$ ./scripts/prettyasserts.py \
-a assert \
-a __builtin_assert \
-u unreachable \
-u __builtin_unreachable \
-A \
input.a.c -o output.c
This isn't really useful for littlefs, but may be useful elsewhere
|
||
|
Christopher Haster
|
738dd86339 |
Extended prettyasserts.py to support unreachable statements
The main benefit is control over error reporting and avoiding the dive into stdlib layers when debugging thanks to __builtin_trap(). This changes -p/--pattern -> -a/--assert And adds -u/--unreachable |
||
|
Christopher Haster
|
1422a61d16 |
Made generated prettyasserts more debuggable
The main star of the show is the adoption of __builtin_trap() for aborting on assert failure. I discovered this GCC/Clang extension recently and it integrates much, _much_ better with GDB. With stdlib's abort(), GDB drops you off in several layers of internal stdlib functions, which is a pain to navigate out of to get to where the assert actually happened. With __builtin_trap(), GDB stops immediately, making debugging quick and easy. This is great! The pain of debugging needs to come from understanding the error, not just getting to it. --- Also tweaked a few things with the internal print functions to make reading the generated source easier, though I realize this is a rare thing to do. |
||
|
Christopher Haster
|
06a360462a |
Simplified test/bench suite finding logic in test.py/bench.py
These just take normal paths now, we weren't even using the magic test/bench suite finding logic since it's easier to just pass everything explicitly in our Makefile. The original test/bench suite finding logic was a bad idea anyways. This is what globs are for, and having custom path chasing logic is inconsistent and risks confusion. |
||
|
Christopher Haster
|
a124ee54e7 |
Reworked test/bench defines to map to global variables
Motivation: - Debuggability. Accessing the current test/bench defines from inside gdb was basically impossible for some dumb macro-debug-info reason I can't figure out. In theory, GCC provides a .debug_macro section when compiled with -g3. I can see this section with objdump --dwarf=macro, but somehow gdb can't seem to find any definitions? I'm guess the #line source remapping is causing things to break somehow... Though even if macro-debugging gets fixed, which would be valuable, accessing defines in the current test/bench runner can trigger quite a bit of hidden machinery. This risks side-effects, which is never great when debugging. All of this is quite annoying because the test/bench defines is usually the most important piece of information when debugging! This replaces the previous hidden define machinery with simple global variables, which gdb can access no problem. - Also when debugging we no longer awkwardly step into the test_define function all the time! - In theory, global variables, being a simple memory access, should be quite a bit faster than the hidden define machinery. This does matter because running tests _is_ a dev bottleneck. In practice though, any performance benefit is below the noise floor, which isn't too surprising (~630s +-~20s). - Using global variables for defines simplifies the test/bench runner quite a bit. Though some of the previous complexity was due to a whole internal define caching system, which was supposed to lazily evaluate test defines to avoid evaluating defines we don't use. This all proved to be useless because the first thing we do when running each test is evaluate all defines to generate the test id (lol). So now, instead of lazily evaluating and caching defines, we just generate global variables during compilation and evaluate all defines for each test permutation immediately before running. This relies heavily on __attribute__((weak)) symbols, and lets the linker really shine. As a funny perk this also effectively interns all test/bench defines by the address of the resulting global variable. So we don't even need to do string comparisons when mapping suite-level defines to the runner-level defines. --- Perhaps the more interesting thing to note, is the change in strategy in how we actually evaluate the test defines. This ends up being a surprisingly tricky problem, due to the potential of mutual recursion between our defines. Previously, because our define machinery was lazy, we could just evaluate each define on demand. If a define required another define, it would lazily trigger another evaluation, implicitly recursing through C's stack. If cyclic, this would eventually lead to a stack overflow, but that's ok because it's a user error to let this happen. The "correct" way, at least in terms of being computationally optimal, would be to topologically sort the defines and evaluate the resulting tree from the leaves up. But I ain't got time for that, so the solution here is equal parts hacky, simple, and effective. Basically, we just evaluate the defines repeatedly until they stop changing: - Initially, mutually recursive defines may read the uninitialized values of their dependencies, and end up with some arbitrarily wrong result. But as the defines are repeatedly evaluated, assuming no cycles, the correct results should eventually bubble up the tree until all defines converge to the correct value. - This is O(n*e) vs O(n+e), but our define graph is usually quite shallow. - To prevent non-halting, we error after an arbitrary 1000 iterations. If you hit this, it's likely because there is a cycle in the define graph. This is runtime configurable via the new --define-depth flag. - To keep things consistent and reproducible, we zero initialize all defines before the first evaluation. I don't think this is strictly necessary, but it's important for the test runner to have the exact same results on every run. No one wants a "works on my machine" situation when the tests are involved. Experimentation shows we only need an evaluation depth of 2 to successfully evaluate the current set of defines: $ ./runners/test_runner --list-defines --define-depth=2 And any performance impact is negligible (~630s +-~20s). |
||
|
Christopher Haster
|
e8b1be17fe |
Added bshrub support to dbgbmap.py
Forgot about this script. |
||
|
Christopher Haster
|
bea13dcf8e |
Use sign bit of rbyd.trunk to indicate shrubness of rbyds
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%)
|
||
|
Christopher Haster
|
15593ccc49 |
Renamed scratch files -> orphan files
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 |
||
|
Christopher Haster
|
ba505c2a37 |
Implemented scratch file basics
"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.
|
||
|
Christopher Haster
|
724fc5fc91 |
Hide gdb info header from test.py/bench.py --gdb
This was too noisy when intermingled with other debug output test.py/bench.py prints when dropping into gdb. |
||
|
Christopher Haster
|
b5e264bec4 |
Fixed issue with pointer comparisons in prettyasserts.py
We end up passing intmax_t pointers around, but without a cast. This results in a warning. Adding a cast fixes the warning. This is in the printing logic, not the actual comparison, so hiding warnings with this cast is not a concern here. I also flipped the type we compare with to use the right-hand side. The pretty-assert code already treats the right-hand as the "expected" value (I wonder if this is an english language quirk), so I think it makes sense to use the right-hand side as the "expected" type. |
||
|
Christopher Haster
|
8eea06286f |
Added optionally crc32c calculation to dbgblock.py
This is useful for debugging checksum mismatches on disk. And since dbgblock.py has some relatively flexible options for slicing the disk, this can be used to find the checksum of any on-disk data pretty easily. |
||
|
Christopher Haster
|
161cd9e6da |
Fixed race condition killing test processes in test/bench.py
Note sure why we weren't hitting this earlier, but I've been hitting this race condition a bunch recently and it's annoying. Now every failed process kills the other test processes unconditionally. It's not clear if this actually _fixes_ the race condition or just makes it less likely, but it's good enough to keep the test script user friendly. |
||
|
Christopher Haster
|
f29a4982c4 |
Added block-level erased-state checksums
Much like the erased-state checksums in our rbyds (ecksums), these
block-level erased-state checksums (becksums) allow us to detect failed
progs to erased parts of a block and are key to achieving efficient
incremental write performance with large blocks and frequent power
cycles/open-close cycles.
These are also key to achieving _reasonable_ write performance for
simple writes (linear, non-overwriting), since littlefs now relies
solely on becksums to efficiently append to blocks.
Though I suppose the previous block staging logic used with the CTZ
skip-list could be brought back to make becksums optional and avoid
btree lookups during simple writes (we do a _lot_ of btree
lookups)... I'll leave this open as a future optimization...
Unlike in-rbyd ecksums, becksums need to be stored out-of-band so our
data blocks only contain raw data. Since they are optional, an
additional tag in the file's btree makes sense.
Becksums are relatively simple, but they bring some challenges:
1. Adding becksums to file btrees is the first case we have for multiple
struct tags per btree id.
This isn't too complicated a problem, but requires some new internal
btree APIs.
Looking forward, which I probably shouldn't be doing this often,
multiple struct tags will also be useful for parity and content ids
as a part of data redundancy and data deduplication, though I think
it's uncontroversial to consider this both heavier-weight features...
2. Becksums only work if unfilled blocks are aligned to the prog_size.
This is the whole point of crystal_size -- to provide temporary
storage for unaligned writes -- but actually aligning the block
during writes turns out to be a bit tricky without a bunch of
unecesssary btree lookups (we already do too many btree lookups!).
The current implementation here discards the pcache to force
alignment, taking advantage of the requirement that
cache_size >= prog_size, but this is corrupting our block checksums.
Code cost:
code stack
before: 31248 2792
after: 32060 (+2.5%) 2864 (+2.5%)
Also lfsr_ftree_flush needs work. I'm usually open to gotos in C when
they improve internal logic, but even for me, the multiple goto jumps
from every left-neighbor lookup into the block writing loop is a bit
much...
|
||
|
Christopher Haster
|
c4d75efa40 |
Added bptr checksums
Looking forward, bptr checksums provide an easy mechanism to validate
data residing in blocks. This extends the merkle-tree-like nature of the
filesystem all the way down to the data level, and is common in other
COW filesystems.
Two interesting things to note:
1. We don't actually check data-level checksums yet, but we do calculate
data-level checksums unconditionally.
Writing checksums is easy, but validating checksums is a bit more
tricky. This is made a bit harder for littlefs, since we can't hold
an entire block of data in RAM, so we have to choose between separate
bus transactions for checksum + data reads, or extremely expensive
overreads every read.
Note this already exists at the metadata-level, the separate bus
transactions for rbyd fetch + rbyd lookup means we _are_ susceptible
to a very small window where bit errors can get through.
But anyways, writing checksums is easy. And has basically no cost
since we are already processing the data for our write. So we might
as well write the data-level checksums at all times, even if we
aren't validating at the data-level.
2. To make bptr checksums work cheaply we need an additional cksize
field to indicate how much data is checksummed.
This field seems redundant when we already have the bptr's data size,
but if we didn't have this field, we would be forced to recalculate
the checksum every time a block is sliced. This would be
unreasonable.
The immutable cksize field does mean we may be checksumming more data
than we need to when validating, but we should be avoiding small
block slices anyways for storage cost reasons.
This does add some stack cost because our bptr struct is larger now:
code stack
before: 31200 2768
after: 31272 (+0.2%) 2800 (+1.1%)
|
||
|
Christopher Haster
|
9f02cbb26b |
Tweaked mount/format/dbg littlefs info print
The info should now be ordered more-or-less by decreasing importance:
littlefs v2.0 4096x256 0x{0,1}.36d w12.256
^ ^ ^ ^ ^ '-.-' ^ ^ ^
'--|-|----|---|-----|---|-----|---|-- littlefs
'-|----|---|-----|---|-----|---|-- on-disk major version
'----|---|-----|---|-----|---|-- on-disk minor version
'---|-----|---|-----|---|-- block size
'-----|---|-----|---|-- block count
'---|-----|---|-- mroot blocks
'-----|---|-- mroot trunk
'---|-- mtree weight
'-- mweight
|
||
|
Christopher Haster
|
6ccd9eb598 |
Adopted different strategy for hypothetical future configs
Instead of writing every possible config that has the potential to be useful in the future, stick to just writing the configs that we know are useful, and error if we see any configs we don't understand. This prevents unnecessary config bloat, while still allowing configs to be introduced in a backwards compatible way in the future. Currently unknown configs are treated as a mount error, but in theory you could still try to read the filesystem, just with potentially corrupted data. Maybe this could be behind some sort of "FORCE" mount flag. littlefs must never write to the filesystem if it finds unknown configs. --- This also creates a curious case for the hole in our tag encoding previously taken up by the OCOMPATFLAGS config. We can query for any config > SIZELIMIT with lookupnext, but the OCOMPATFLAGS flag would need an extra lookup which just isn't worth it. Instead I'm just adding OCOMPATFLAGS back in. To support OCOMPATFLAGS littlefs has to do literally nothing, so this is really more of a documentation change. And who know, maybe OCOMPATFLAGS will have some weird use case in the future... |
||
|
Christopher Haster
|
337bdf61ae |
Rearranged tag encodings to make space for BECKSUM, ORPHAN, etc
Also: - Renamed GSTATE -> GDELTA for gdelta tags. GSTATE tags added as separate in-device flags. The GSTATE tags were already serving this dual purpose. - Renamed BSHRUB* -> SHRUB when the tag is not necessarily operating on a file bshrub. - Renamed TRUNK -> BSHRUB The tag encoding space now has a couple funky holes: - 0x0005 - Hole for aligning config tags. I guess this could be used for OCOMPATFLAGS in the future? - 0x0203 - Hole so that ORPHAN can be a 1-bit difference from REG. This could be after BOOKMARK, but having a bit to differentiate littlefs specific file types (BOOKMARK, ORPHAN) from normal file types (REG, DIR) is nice. I guess this could be used for SYMLINK if we ever want symlinks in the future? - 0x0314-0x0318 - Hole so that the mdir related tags (MROOT, MDIR, MTREE) are nicely aligned. This is probably a good place for file-related tags to go in the future (BECKSUM, CID, COMPR), but we only have two slots, so will probably run out pretty quickly. - 0x3028 - Hole so that all btree related tags (BTREE, BRANCH, MTREE) share a common lower bit-pattern. I guess this could be used for MSHRUB if we ever want mshrubs in the future? |
||
|
Christopher Haster
|
04c6b5a067 |
Added grm rcompat flag, dropped ocompat, tweaked compat flags a bit
I'm just not seeing a use case for optional compat flags (ocompat), so
dropping for now. It seems their *nix equivalent, feature_compat, is
used to inform fsck of things, but this doesn't really make since in
littlefs since there is no fsck. Or from a different perspective,
littlefs is always running fsck.
Ocompat flags can always be added later (since they do nothing).
Unfortunately this really ruins the alignment of the tag encoding. For
whatever reason config limits tend to come in pairs. For now the best
solution is just leave tag 0x0006 unused. I guess you can consider it
reserved for hypothetical ocompat flags in the future.
---
This adds an rcompat flag for the grm, since in theory a filesystem
doesn't need to support grms if it never renames files (or creates
directories?). But if a filesystem doesn't support grms and a grms gets
written into the filesystem, this can lead to corruption.
I think every piece of gstate will end up with its own compat flag for
this reason.
---
Also renamed r/w/oflags -> r/w/ocompatflags to make their purpose
clearer.
---
The code impact of adding the grm rcompat flag is minimal, and will
probably be less for additional rcompat flags:
code stack
before: 31528 2752
after: 31584 (+0.2%) 2752 (+0.0%)
|
||
|
Christopher Haster
|
d485795336 |
Removed concept of geometries from test/bench runners
This turned out to not be all that useful. Tests already take quite a bit to run, which is a good thing! We have a lot of tests! 942.68s or ~15 minutes of tests at the time of writing to be exact. But simply multiplying the number of tests by some number of geometries is heavy handed and not a great use of testing time. Instead, tests where different geometries are relevant can parameterize READ_SIZE/PROG_SIZE/BLOCK_SIZE at the suite level where needed. The geometry system was just another define parameterization layer anyways. Testing different geometries can still be done in CI by overriding the relevant defines anyways, and it _might_ be interesting there. |
||
|
Christopher Haster
|
b8d3a5ef46 |
Fixed inotify race conditions and fd leak in scripts
Since we were only registering our inotify reader after the previous operation completed, it was easy to miss modifications that happened faster than our scripts. Since our scripts are in Python, this happened quite often and made it hard to trust the current state of scripts with --keep-open, sort of defeating the purpose of --keep-open... I think previously this race condition wasn't avoided because of the potential to loop indefinitely if --keep-open referenced a file that the script itself modified, but it's up to the user to avoid this if it is an issue. --- Also while fixing this, I noticed our use of the inotify_simple library was leaking file descriptors everywhere! I just wasn't closing any inotify objects at all. A bit concerning since scripts with --keep-open can be quite long lived... |