Previously we only bumped the revision count if an eviction would occur
immediately (and possibly corrupt littlefs). This works, but does risk
an unoptimal superblock size if an almost-exhausted superblock was
allocated during lfs_format.
As pointed out by tim-nordell-nimbelink, we can align the revision count
to maximize the number of block cycles without breaking the existing
requirements of increasing revision counts.
As an added benefit, littlefs's wear-leveling should behave more
consistently after this change.
This bug was exposed by the bad-block tests due to changes to block
allocation, but could have been hit before these changes.
In flash, when blocks fail, they don't fail in a predictable manner. To
account for this, the bad-block tests check a number of failure
behaviors. The interesting one here is "LFS_TESTBD_BADBLOCK_ERASENOOP",
in which bad blocks can not be erased or programmed, and are stuck with
the data written at the time the blocks go bad.
This is actually a pretty realistic failure behavior, since flash needs a
large voltage to force the electrons of the floating gates. Though
realistically, such a failure would like corrupt the data a bit, not leave the
underlying data perfectly intact.
LFS_TESTBD_BADBLOCK_ERASENOOP is rather interesting to test for because it
means bad blocks can end up with perfectly valid CRCs after a failed write,
confusing littlefs.
---
In this case, we had the perfect series of operations such that a test
was repeatedly writing the same sequence of metadata commits to the same
block, which eventually goes bad, leaving the block stuck with metadata
that occurs later in the sequence.
What this means is that after the first commit, the metadata block
contained both the first and second commits, even though the loop in the
test hadn't reached that point yet.
expected actual
.----------. .----------.
| commit 1 | | commit 1 |
| crc 1 | | crc 1 |
| | | commit 2 <-- (from previous iteration)
| | | crc 2 |
'----------' '----------'
To protect against this, littlefs normally compares the written CRC
against the expected CRC, but because this was the exact same data that
it was going to write, this CRCs end up the same.
Ah! But doesn't littlefs also encode the state of the next page to keep
track of if the next page has been erased or not? Wouldn't that change
between iterations?
It does! In a single bit in the CRC-tag. But thanks to some incorrect
logic attempting to avoid an extra condition in the loop for writing out
padding commits, the CRC that littlefs checked against was the CRC
immediately before we include the "is-next-page-erased" bit.
Changing the verification check to use the same CRC as what is used to
verify commits on fetch solves this problem.
Not sure how this went unnoticed, I guess this is the first bug that
needed in-depth inspection after the a last-minute argument cleanup
in the debug scripts.
This rather interesting corner-case arises in lfs_dir_alloc anytime the
uninitialized revision count happens to be a multiple of block_cycles+1.
For example, the source of the bug found by tim-nordell-nimbelink:
rev = 2742492087
block_cycles = 100
2742492087 % (100+1) = 0
The reason for this weird block_cycles+1 case is due to a fix for a
previous bug in fe957de. To avoid aliasing, which would cause metadata
pairs to wear unevenly, block_cycles incremented to the next odd number.
Normally, littlefs tweaks the revision count of blocks during
lfs_dir_alloc in order to make sure evictions can't happen on the first
compact. Otherwise, higher-level logic such as lfs_format would break.
However, this wasn't updated with the aliasing fix in fe957de, so
lfs_dir_alloc was only rounding the revision count to the nearest even
number.
The current fix is to change the logic in lfs_dir_alloc to explicitly
check for the eviction condition and increment if eviction would occur.
Found by tim-nordell-nimbelink
As noted by gtaska, we are sitting on a better hash-combining function
than xor: CRC. Previous issues with xor were solvable, but relying on
xor for this isn't really worth the risk when we already have a CRC
function readily available.
To quote a study found by gtaska:
https://michiel.buddingh.eu/distribution-of-hash-values
> CRC32 seems to score really well, but its graph is skewed by the results
> of Dataset 5 (binary numbers), which may or may not be too synthetic to
> be considered a fair benchmark. But even if you substract the results
> from that test, it does not fare significantly worse than other,
> cryptographic hash functions.
On first read, randomizing the allocators offset may seem appropriate
for lfs_alloc_reset. However, it ends up using the filesystem-fed
pseudorandom seed in situations it wasn't designed for.
As noted by gtaska, the combination of using xors for feeding the seed
and multiple traverses of the same CRCs can cause the seed to flip to
zeros with concerning frequency.
Removed the randomization from lfs_alloc_reset, leaving it in only
lfs_mount.
Found by gtaska
Modulus of the offset by block_size was clearly a typo, and should be
block_count. Interesting to note that later moduluses during alloc
calculations prevents this from breaking anything, but as gtaska notes it
could skew the wear-leveling distribution.
Found by guiserle and gtaska
As introduced in #297, I created a python wrapper for littlefs. The wrapper supports two API's: A C-like API which is the same as in C and a more pythonic API which is easier to use if you are more the python guy. The wrapper is built with littlefs 2.2.1 at the moment.
__VA_ARGS__ are frustrating in C. Even for their main purpose (printf),
they fall short in that they don't have a _portable_ way to have zero
arguments after the format string in a printf call.
Even if we detect compilers and use ##__VA_ARGS__ where available, GCC
emits a warning with -pedantic that is _impossible_ to explicitly
disable.
This commit contains the best solution we can think of. A bit of
indirection that adds a hidden "%s" % "" to the end of the format
string. This solution does not work everywhere as it has a runtime
cost, but it is hopefully ok for debug statements.
- Standardized littlefs debug statements to use hex prefixes and
brackets for printing pairs.
- Removed the entry behavior for readtree and made -t the default.
This is because 1. the CTZ skip-list parsing was broken, which is not
surprising, and 2. the entry parsing was more complicated than useful.
This functionality may be better implemented as a proper filesystem
read script, complete with directory tree dumping.
- Changed test.py's --gdb argument to take [init, main, assert],
this matches the names of the stages in C's startup.
- Added printing of tail to all mdir dumps in readtree/readmdir.
- Added a print for if any mdirs are corrupted in readtree.
- Added debug script side-effects to .gitignore.
Block device tracing has a lot of potential uses, of course debugging,
but it can also be used for profiling and externally tracking littlefs's
usage of the block device. However, block device tracing emits a massive
amount of output. So keeping block device tracing on by default limits
the usefulness of the filesystem tracing.
So, instead, I've moved the block device tracing into a separate
LFS_TESTBD_YES_TRACE define which switches on the LFS_TESTBD_TRACE
macro. Note that this means in order to get block device tracing, you
need to define both LFS_YES_TRACE and LFS_TESTBD_YES_TRACE. This is
needed as the LFS_TRACE definition is gated by LFS_YES_TRACE in
lfs_util.h.
These two features have been much requested by users, and have even had
several PRs proposed to fix these in several cases. Before this, these
error conditions usually were caught by internal asserts, however
asserts prevented users from implementing their own workarounds.
It's taken me a while to provide/accept a useful recovery mechanism
(returning LFS_ERR_CORRUPT instead of asserting) because my original thinking
was that these error conditions only occur due to bugs in the filesystem, and
these bugs should be fixed properly.
While I still think this is mostly true, the point has been made clear
that being able to recover from these conditions is definitely worth the
code cost. Hopefully this new behaviour helps the longevity of devices
even if the storage code fails.
Another, less important, reason I didn't want to accept fixes for these
situations was the lack of tests that prove the code's value. This has
been fixed with the new testing framework thanks to the additional of
"internal tests" which can call C static functions and really take
advantage of the internal information of the filesystem.
It's very slowly to compare one byte at one time. Here are the
performance I get from 128M spinand with NFTL by sequential writing.
| file size | buffer size | write speed |
| 10 MB | 0 B | 3206.01 KB/s |
| 10 MB | 1 B | 2434.04 KB/s |
| 10 MB | 2 B | 2685.78 KB/s |
| 10 MB | 4 B | 2857.94 KB/s |
| 10 MB | 8 B | 3060.68 KB/s |
| 10 MB | 16 B | 3155.30 KB/s |
| 10 MB | 64 B | 3193.68 KB/s |
| 10 MB | 128 B | 3230.62 KB/s |
| 10 MB | 256 B | 3153.03 KB/s |
| 70 MB | 0 B | 2258.87 KB/s |
| 70 MB | 1 B | 1827.83 KB/s |
| 70 MB | 2 B | 1962.29 KB/s |
| 70 MB | 4 B | 2074.01 KB/s |
| 70 MB | 8 B | 2147.03 KB/s |
| 70 MB | 64 B | 2179.92 KB/s |
| 70 MB | 256 B | 2179.96 KB/s |
The 0 Byte size means no validation and the 1 Byte size is how
littlefs do before. Based on the above table and to save memory,
comparing 8 bytes at one time is more wonderful.
Signed-off-by: WeiXiong Liao <liaoweixiong@allwinnertech.com>
- Changed readmdir.py to print the metadata pair and revision count,
which is useful when debugging commit issues.
- Added truncated data view to readtree.py by default. This does mean
readtree.py must read all files on the filesystem to show the
truncated data, hopefully this does not end up being a problem.
- Made overall representation hopefully more readable, including moving
superblock under the root dir, userattrs under files, fixing a gstate
rendering issue.
- Added rendering of soft-tails as dotted-arrows, hopefully this isn't
too noisy.
- Fixed explode_asserts.py off-by-1 in #line mapping caused by a strip
call in the assert generation eating newlines. The script matches
line numbers between the original+modified files by emitting assert
statements that use the same number of lines. An off-by-1 here causes
the entire file to map lines incorrectly, which can be very annoying.
With the superblock expansion stuff, the test_format tests have grown
to test more advanced superblock-related features. This is fine but
deserves a rename so it's more clear.
Also fixed a typo that meant tests never ran with block cycles.
Byte-level writes are expensive and not suggested (caches >= 4 bytes
make much more sense), however there are many corner cases with
byte-level writes that can be easy to miss (power-loss leaving single
bytes written to disk).
Unfortunately, byte-level writes mixed with power-loss testing, the
Travis infrastructure, and Arm Thumb instruction set simulation
exceeds the 50-minute budget Travis allocates for jobs.
For now I'm disabling the byte-level tests under Qemu, with the hope that
performance improvements in littlefs will let us turn these tests back
on in the future.
- Added caching to Travis install dirs, because otherwise
pip3 install fails randomly
- Increased size of littlefs-fuse disk because test script has
a larger footprint now
- Skip a couple of reentrant tests under byte-level writes because
the tests just take too long and cause Travis to bail due to no
output for 10m
- Fixed various Valgrind errors
- Suppressed uninit checks for tests where LFS_BLOCK_ERASE_VALUE == -1.
In this case rambd goes uninitialized, which is fine for rambd's
purposes. Note I couldn't figure out how to limit this suppression
to only the malloc in rambd, this doesn't seem possible with Valgrind.
- Fixed memory leaks in exhaustion tests
- Fixed off-by-1 string null-terminator issue in paths tests
- Fixed lfs_file_sync issue caused by revealed by fixing memory leaks
in exhaustion tests. Getting ENOSPC during a file write puts the file
in a bad state where littlefs doesn't know how to write it out safely.
In this case, lfs_file_sync and lfs_file_close return 0 without
writing out state so that device-side resources can still be cleaned
up. To recover from ENOSPC, the file needs to be reopened and the
writes recreated. Not sure if there is a better way to handle this.
- Added some quality-of-life improvements to Valgrind testing
- Fit Valgrind messages into truncated output when not in verbose mode
- Turned on origin tracking
The core of littlefs's CI testing is the full test suite, `make test`, run
under a number of configurations:
- Processor architecture:
- x86 (native)
- Arm Thumb
- MIPS
- PowerPC
- Storage geometry:
- rs=16 ps=16 cs=64 bs=512 (default)
- rs=1 ps=1 cs=64 bs=4KiB (NOR flash)
- rs=512 ps=512 cs=512 bs=512 (eMMC)
- rs=4KiB ps=4KiB cs=4KiB bs=32KiB (NAND flash)
- Other corner cases:
- no intrinsics
- no inline
- byte-level read/writes
- single block-cycles
- odd block counts
- odd block sizes
The number of different configurations we need to test quickly exceeds the
50 minute time limit Travis has on jobs. Fortunately, we can split these
tests out into multiple jobs. This seems to be the intended course of
action for large CI "builds" in Travis, as this gives Travis a finer
grain of control over limiting builds.
Unfortunately, this created a couple issues:
1. The Travis configuration isn't actually that flexible. It allows a
single "matrix expansion" which can be generated from top-level lists
of different configurations. But it doesn't let you generate a matrix
from two seperate environment variable lists (for arch + geometry).
Without multiple matrix expansions, we're stuck writing out each test
permutation by hand.
On the bright-side, this was a good chance to really learn how YAML
anchors work. I'm torn because on one hand anchors add what feels
like unnecessary complexity to a config language, on the other hand,
they did help quite a bit in working around Travis's limitations.
2. Now that we have 47 jobs instead of 7, reporting a separate status
for each job stops making sense.
What I've opted for here is to use a special NAME variable to
deduplicate jobs, and used a few state-less rules to hopefully have
the reported status make sense most of the time.
- Overwrite "pending" statuses so that the last job to start owns the
most recent "pending" status
- Don't overwrite "failure" statuses unless the job number matches
our own (in the case of CI restarts)
- Don't write "success" statuses unless the job number matches our
own, this should delay a green check-mark until the last-to-start
job finishes
- Always overwrite non-failures with "failure" statuses
This does mean a temporary "success" may appear if the last job
terminates before earlier jobs. But this is the simpliest solution
I can think of without storing some complex state somewhere.
Note we can only report the size this way because it's cheap to
calculate in every job.
RAM-backed testing is faster than file-backed testing. This is why
test.py uses rambd by default.
So why add support for tmpfs-backed disks if we can already run tests in
RAM? For reentrant testing.
Under reentrant testing we simulate power-loss by forcefully exiting the
test program at specific times. To make this power-loss meaningful, we need to
persist the disk across these power-losses. However, it's interesting to
note this persistence doesn't need to be actually backed by the
filesystem.
It may be possible to rearchitecture the tests to simulate power-loss a
different way, by say, using coroutines or setjmp/longjmp to leave
behind ongoing filesystem operations without terminating the program
completely. But at this point, I think it's best to work with what we
have.
And simply putting the test disks into a tmpfs mount-point seems to
work just fine.
Note this does force serialization of the tests, which isn't required
otherwise. Currently they are only serialized due to limitations in
test.py. If a future change wants to perallelize the tests, it may need
to rework RAM-backed reentrant tests.
Moved .travis.yml over to use the new test framework. A part of this
involved testing all of the configurations ran on the old framework
and deciding which to carry over. The new framework duplicates some of
the cases tested by the configurations so some configurations could be
dropped.
The .travis.yml includes some extreme ones, such as no inline files,
relocations every cycle, no intrinsics, power-loss every byte, unaligned
block_count and lookahead, and odd read_sizes.
There were several configurations were some tests failed because of
limitations in the tests themselves, so many conditions were added
to make sure the configurations can run on as many tests as possible.
This is a bit of a strange case that can be caused by storage with
very large prog sizes, such as NAND flash. We only have 10 bits to store
the size of our padding, so when the prog_size gets larger than 1024
bytes, we have to use multiple padding tags to commit to the next
prog_size boundary.
This causes some complication for the new logic that checks CRCs in case
our block becomes "readonly" and contains existing commits that just happen
to match our new commit size.
Here we just check the CRC of the first commit. This isn't perfect but
does protect against pure "readonly" blocks.
These should probably have been cleaned up in each commit to allow
cherry-picking, but due to time I haven't been able to.
- Went with creating an mdir copy in lfs_dir_commit. This handles a
number of related cleanup issues in lfs_dir_compact and it does so
more robustly. As a plus we can use the copy to update dependencies
in the mlist.
- Eliminated code left by the ENOSPC file outlining
- Cleaned up TODOs and lingering comments
- Changed the reentrant many directory create/rename/remove test to use
a smaller set of directories because of space issues when
READ/PROG_SIZE=512
