Minor release: v2.3 #495
Merged
Minor release: v2.3 #495
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
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.
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
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
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.
…_cycles+1 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
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 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.
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.
Added littlefs-python to the related projects section
Fix several wear-leveling issues found in lfs_alloc_reset
lfs_bd_cmp() compares more bytes at one time
Assert that the file isnt open in lfs_file_opencfg
- undef unavailable function declarations altogether - even less code, assert on write attempts - remove LFS_O_WRONLY and other flags when compiling with LFS_READONLY - do not annotate #endif, as requested - move ifdef before comments blocks, rework dangling opening bracket - ifdef file flags that are not needed in read-only mode - slight refactor - ifdef LFS_F_ERRED out as well
Add LFS_READONLY define, to allow smaller builds providing read-only mode
- expand functions - add comment - rename functions - fix locking issue in format and mount - use global include - fix ac6 linker issue - use the global config file - address review comments - minor cleanup - minor cleanup - review comments
- Stayed on non-system include for lfs_util.h for now - Named internal functions "lfs_functionraw" - Merged lfs_fs_traverseraw - Added LFS_LOCK/UNLOCK macros - Changed LFS_THREADSAFE from 1/0 to defined/undefined to match LFS_READONLY
This removes quite a bit of extra code needed to entertwine the LFS_TRACE calls into the original funcions. Also changed temporary return type to match API declaration where necessary.
…-option Add thread safe wrappers
Instead of additional flag, we can just go through the mlist.
This indirectly solves an issue with lfs_file_rawclose asserting when lfs_file_opencfg errors since appending to the mlist occurs after open. It also may speed up some of the internal operations such as the lfs_file_write used to resolve unflushed data. The idea behind adopting mlist over flags is that realistically it's unlikely for the user to open a significant number of files (enough for big O to kick in). That being said, moving the mlist asserts into the API wrappers does protect some of the internal operations from scaling based on the number of open files.
Fix allocation-eviction issue when erase state is multiple of block_cycles+1
- Prefixing with raw is slightly more readable, follows common-prefix rule - Matches existing raw prefixes in testbd
This comment has been minimized.
This comment has been minimized.
|
Updated with the code costs of all architectures in CI for fun. Note these all have logging/asserts disabled:
|
|
Time to merge |
|
Ok, one of the next things to do is move to GitHub Actions, because Travis CI takes a very long time to run now... |
We recently did this as well, if you want inspiration see: https://github.com/micropython/micropython/tree/master/.github/workflows |
lorol
added a commit
to lorol/LITTLEFS
that referenced
this pull request
Dec 31, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
You may or may not have noticed a slight change to this repo's branches. There is now a devel branch, which for the near future, will act as a staging area for release before they are merged to master.
Is this an indirect method of having master/release branches? Maybe! But I'm still learning and this should help how fast I can merge PRs without committing to releases at the same time.
Anyways!
This PR brings in:
A draft of the release notes follows:
What's new?
Thanks to @maximevince, @renesas-billgesner, and @roykuper13, we now have two impactful compile-time options for littlefs!
LFS_READONLY
Define
LFS_READONLYto enable read-only mode. This will limit littlefs to functions that do not modify the disk, but will reduce the code cost to ~1/3. This is a great addition for bootloaders or other applications where a small, read-only storage blob is useful.LFS_THREADSAFE
Define
LFS_THREADSAFEto enable thread-safe mode. Note this needs thelockandunlockcallbacks in the lfs_config struct.When defined, littlefs will call the related
lockandunlockcallbacks when necessary to synchronize the filesystem. Allowing easy integration into an RTOS without needing an OS or wrapper layer.In addition, this release brings in a number of improvements:
And a number of other fixes.
Will update this with the related code sizes when CI finishes, it will be interesting to the see the different code costs.