Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LittleFSv2: Bring in v2.2 #12783

Merged
merged 5 commits into from
May 5, 2020
Merged

LittleFSv2: Bring in v2.2 #12783

merged 5 commits into from
May 5, 2020

Conversation

VeijoPesonen
Copy link
Contributor

@VeijoPesonen VeijoPesonen commented Apr 9, 2020

Summary of changes

Introduces LittleFSv2.2 developed by @geky. Mbed OS adaption (LittleFSv1 -> LittleFSv2) is based on work done by @pilotak. The only thing the undersigned has done was to create this PR.

What's new?

This is an abbreaviated list of the new features introduced by LittleFSv2. For the full list of new features and bug fixes please see the original LittleFSv2.0-v2.2.1 release notes.

  • Metadata logging
    Metadata pairs now act as small two block logs. This allows incremental updates without erasing metadata blocks, which is a significant performance improvement. Additionally, this enables dynamically sized entries, which is necessary for the two following features.

  • Custom attributes
    It's now possible to get and set custom attributes on files, directories, and the even the filesystem itself. Custom attributes use a byte as an identifier and can be up to 1022 bytes large. This is useful for things such as timestamps, permissions, hashes, or other attributes.

  • Inline files
    Files can now be inlined directly in the directory block. This allows multiple small files to be coalesced into a single block instead of wasting an entire block for each file. This opens up the possibility for littlefs to be used on NAND flash and MCU internal flash, both which often have block sizes >16KiB. Note that inline files are limited by the cache size.

  • Independent cache sizes
    littlefs now has a separate cache_size config option. This config option determines the size of the read cache, prog cache, and file caches independently of the storage's physical read/prog size. This allows more granular reads and writes without artificially limiting the cache, improving performance and log utilization.

  • XOR global state, no more move problem
    XOR global state is a mechanism that allows littlefs to store a small amount of global state distributed across the filesystem. This global state is now used internal to fix moves in O(1), a significant improvement over the previous O(n) search. Additionally XOR global state may be useful for future improvements.

  • True dynamic wear-leveling
    littlefs now proactively levels wear over dynamic blocks instead of reacting only to bad blocks. This offers better protection against wear that causes stored data to deteriorate.
    Dynamic wear-leveling is provided by evicting blocks after a bounded number of writes determined by the block_cycles config option. A combination of random offset and incremental allocation of blocks provide a uniform distribution of wear over the storage.

  • Expanding superblocks
    The superblock no longer has a flat cost of 2 blocks. Instead, the superblock doubles as the root directory until block_cycles are passed. After every block_cycles number of erases, the superblock is expanded to include another metadata pair if space is available. This allows littlefs to use as few superblocks as is needed to avoid an early death.
    At the smallest, littlefs can now fit in only 2 blocks.

Debugging improvements
  • asserts against operations on closed files
  • LFS_TRACE statements that can be turned on by defining LFS_YES_TRACE at build time
  • Added asserts help stop positive error codes from block devices from causing the filesystem to break, this has been a reoccurring issue [NUCLEO_F302R8] Add cmsis and hal files + change F401RE clock to 84MHz #215, Allow USBDevice::connect() to be non-blocking #169
  • Significantly improved handling of devices with large prog sizes (prog_size >1KiB)
  • Changed block_cycles = 0 to assert, replaced with block_cycles = -1 (see above)
  • A revamped testing framework intended to improve the rate of bug isolation and fixes. There are new runtime contexts for tests such as simulated power-loss and Valgrind based memory checking, and additional scripts for printing out metadata to debug the filesystem.
Other changes
  • Introduced rambd and filebd, two block devices that replace emubd for simulating block devices locally
  • Changed asserts on corrupted filesystem to return errors, allowing additional user recover
  • Reenabled cache bypassing if the filesystem is handling data that is aligned to the block device's requirements

Impact of changes

Migration actions required

Brings in LittleFSv2 which will coexist with LittleFSv1 already found from Mbed OS. If you already a v1 filesystem you might try converting it to v2, but this isn't guaranteed to succeed. You should preferably create a new v2 filesystem instead.

Create a new filesystem

Example code how to take LittleFSv2 into use:

// Physical block device, can be any device that supports the BlockDevice API
SPIFBlockDevice bd(PTE2, PTE4, PTE1, PTE5);
// Storage for the littlefs
LittleFileSystem2 fs("fs");
int main() {
    // Mount the filesystem
    int err = fs.mount(&bd);
    FILE *f = fopen("/fs/boot_count", "r+");
    ...
}
Optional migration from v1

This is the help the introduction of littlefs v2, which is disk
incompatible with littlefs v1. While v2 can't mount v1, what we can
do is provide an optional migration [1], which can convert v1 into v2
partially in-place.

At worse, we only need to carry over the readonly operations on v1,
which are much less complicated than the write operations, so the extra
code cost may be as low as 25% of the v1 code size. Also, because v2
contains only metadata changes, it's possible to avoid copying file
data during the update.

Enabling the migration requires two steps

  1. Defining LFS_MIGRATE
  2. Call lfs_migrate (only available with the above macro)

Each macro multiplies the number of configurations needed to be tested,
so I've been avoiding macro controlled features since there's still work
to be done around testing the single configuration that's already
available. However, here the cost would be too high if we included migration
code in the standard build. We can't use the lfs_migrate function for
link time gc because of a dependency between the allocator and v1 data
structures.

So how does lfs_migrate work? It turned out to be a bit complicated, but
the answer is a multistep process that relies on mounting v1 readonly and
building the metadata skeleton needed by v2.

  1. For each directory, create a v2 directory
  2. Copy over v1 entries into v2 directory, including the soft-tail entry
  3. Move head block of v2 directory into the unused metadata block in v1
    directory. This results in both a v1 and v2 directory sharing the
    same metadata pair.
  4. Finally, create a new superblock in the unused metadata block of the
    v1 superblock.

Just like with normal metadata updates, the completion of the write to
the second metadata block marks a succesful migration that can be
mounted with littlefs v2. And all of this can occur atomically, enabling
complete fallback if power is lost of an error occurs.

Note there are several limitations with this solution.

  1. While migration doesn't duplicate file data, it does temporarily
    duplicate all metadata. This can cause a device to run out of space if
    storage is tight and the filesystem as many files. If the device was
    created with >~2x the expected storage, it should be fine.

  2. The current implementation is not able to recover if the metadata
    pairs develop bad blocks. It may be possilbe to workaround this, but
    it creates the problem that directories may change location during
    the migration. The other solutions I've looked at are complicated and
    require superlinear runtime. Currently I don't think it's worth
    fixing this limitation.

  3. Enabling the migration requires additional code size. Currently this
    looks like it's roughly 11% at least on x86.

And, if any failure does occur, no harm is done to the original v1
filesystem on disk.

[1] Added migration from littlefs v1

Documentation

LittleFSv1 and LittleFSv2 will coexist and this needs to be made clear in Mbed OS 6 documentation. For further details about LittleFSv2.2 the upstream repository should be checked.


Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

@geky
@SeppoTakalo
@pilotak
@bulislaw


@SeppoTakalo
Copy link
Contributor

@VeijoPesonen Migration section should contain notes how the user might want to take LittleFS2 into use.

And, perhaps release notes.

SeppoTakalo
SeppoTakalo previously approved these changes Apr 9, 2020
@mergify mergify bot added the needs: work label Apr 9, 2020
@mbed-ci
Copy link

mbed-ci commented Apr 9, 2020

Test run: FAILED

Summary: 2 of 3 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-ARM

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 9, 2020

Hold on with CI restart (after the initial failures), we need to complete 5.15.2 (one more fix to CI needed there), will restart all CI master jobs later today

@VeijoPesonen
Copy link
Contributor Author

@SeppoTakalo I took the original release notes and added an abbreaviated list of new features.

@VeijoPesonen
Copy link
Contributor Author

Seems to be that I need to update the astyle ignorefile.

@mergify mergify bot dismissed SeppoTakalo’s stale review April 14, 2020 09:00

Pull request has been modified.

@VeijoPesonen
Copy link
Contributor Author

VeijoPesonen commented Apr 14, 2020

@adbridge Should everything what is supposed to go in Release Notes be in plain text instead of markdown? I couldn't find an answer from the guidelines for Pull requests.

@VeijoPesonen
Copy link
Contributor Author

@adbridge There seems to be an astyle script issue.

git diff --name-only --diff-filter=d FETCH_HEAD..HEAD \
  | ( grep '.\(c\|cpp\|h\|hpp\)$' || true ) \
  | ( grep -v -f .astyleignore || true ) \
  | while read file; do astyle -n --options=.astylerc "${file}"; done
Unchanged  features/storage/filesystem/littlefsv2/LittleFileSystem2.cpp
Unchanged  features/storage/filesystem/littlefsv2/LittleFileSystem2.h
Unchanged  features/storage/filesystem/littlefsv2/TESTS/COMMON/atomic_usage.cpp
...
Unchanged  features/storage/filesystem/littlefsv2/TESTS/filesystem_retarget/seek/main.cpp
Formatted  features/storage/filesystem/littlefsv2/TESTS/util/clean.sh
The command "git diff --name-only --diff-filter=d FETCH_HEAD..HEAD \
  | ( grep '.\(c\|cpp\|h\|hpp\)$' || true ) \
  | ( grep -v -f .astyleignore || true ) \
  | while read file; do astyle -n --options=.astylerc "${file}"; done" exited with 0.
0.03s$ git diff --exit-code --diff-filter=d --color
diff --git a/features/storage/filesystem/littlefsv2/TESTS/util/clean.sh b/features/storage/filesystem/littlefsv2/TESTS/util/clean.sh
index ba9862db28..d9ba62b826 100755
--- a/features/storage/filesystem/littlefsv2/TESTS/util/clean.sh
+++ b/features/storage/filesystem/littlefsv2/TESTS/util/clean.sh
@@ -1,4 +1,4 @@
 #!/bin/bash
 
-rm -f main.cpp
-rm -f template_all_names.txt
+rm - f main.cpp
+rm - f template_all_names.txt
The command "git diff --exit-code --diff-filter=d --color" exited with 1.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 14, 2020

I'll check that shortly. .sh script should be excluded by grep there.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 14, 2020

A problem is in grep '.\(c\|cpp\|h\|hpp\)$', it also selects .sh, need to fix that to exclude anything besides .h

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 14, 2020

@VeijoPesonen The best would be to patch it here (separate commit), replace this in travis.yml:

grep '.\(c\|cpp\|h\|hpp\)$' with grep '.*\.\(c\|cpp\|h\|hpp\)$'

@VeijoPesonen
Copy link
Contributor Author

@0xc0170 Done.

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 14, 2020

travis-ci/astyle — Success!

Looks good now 👍

@adbridge
Copy link
Contributor

@VeijoPesonen Your 'whats new' text should really go into the Summary of Changes section rather than the migration guide ?

@VeijoPesonen
Copy link
Contributor Author

@adbridge Fixed.

@0xc0170 0xc0170 requested a review from bulislaw April 16, 2020 11:59
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 16, 2020

I scheduled CI meanwhile

0xc0170
0xc0170 previously approved these changes Apr 16, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 16, 2020

@pilotak Well done!

@mergify mergify bot added needs: CI and removed needs: review labels Apr 16, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 16, 2020

@VeijoPesonen Migration section should contain notes how the user might want to take LittleFS2 into use.

And, perhaps release notes.

@VeijoPesonen this has not yet been addressed here , has it ? I followed the link provided in migration section and was not redirected to how to migrate (just saw recent release notes), can we be more specific? It would be good to have this in the migration itself so when someone gets the release notes, it is all in there.

@mergify mergify bot added needs: work and removed needs: CI labels Apr 16, 2020
@mbed-ci
Copy link

mbed-ci commented Apr 30, 2020

Test run: FAILED

Summary: 1 of 6 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@mergify mergify bot added needs: work and removed needs: CI labels Apr 30, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Apr 30, 2020

Can anyone look at wear leveling issue? It is still present in the test failures (one network flaky).

@adbridge
Copy link
Contributor

adbridge commented May 1, 2020

Will retry the CI to see if it is a transient CI issue

@adbridge
Copy link
Contributor

adbridge commented May 1, 2020

CI restarted

@mbed-ci
Copy link

mbed-ci commented May 1, 2020

Test run: FAILED

Summary: 1 of 6 test jobs failed
Build number : 7
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_greentea-test

@adbridge
Copy link
Contributor

adbridge commented May 1, 2020

@VeijoPesonen This looks like a related failure could you please investigate

@0xc0170
Copy link
Contributor

0xc0170 commented May 4, 2020

The same issue still present :(

@0xc0170
Copy link
Contributor

0xc0170 commented May 5, 2020

Tests restarted (fix is on master)

Veijo Pesonen added 5 commits May 5, 2020 14:24
Adds littlefs-directory under littlefsv2 to .astyleignore as it's an
external component brought to Mbed OS. Issues with the adaption layer
fixed.
@0xc0170
Copy link
Contributor

0xc0170 commented May 5, 2020

CI restarted after the update

@mbed-ci
Copy link

mbed-ci commented May 5, 2020

Test run: FAILED

Summary: 1 of 6 test jobs failed
Build number : 8
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_cloud-client-pytest

@0xc0170
Copy link
Contributor

0xc0170 commented May 5, 2020

Please ignore ^^ , I had to abort the job due to the rebase

@0xc0170
Copy link
Contributor

0xc0170 commented May 5, 2020

jenkins-ci/greentea-test — passed 💯

@mbed-ci
Copy link

mbed-ci commented May 5, 2020

Test run: SUCCESS

Summary: 6 of 6 test jobs passed
Build number : 9
Build artifacts

@VeijoPesonen
Copy link
Contributor Author

PR #12919 was required to get wear leveling tests to work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants