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

Minor release: v2.9 #929

Merged
merged 25 commits into from
Jan 23, 2024
Merged

Minor release: v2.9 #929

merged 25 commits into from
Jan 23, 2024

Conversation

geky
Copy link
Member

@geky geky commented Jan 19, 2024

This release brings in some heavily requested minor features. Thanks to everyone who has contributed/provided feedback!

Bringing in:

Draft of release notes follows:


Itsy-bitsy teeny-tiny breaking changes:

  • lfs_rename now returns LFS_ERR_NOTDIR if the source is a regular file and the destination is a directory. This better aligns with POSIX.

  • Defining LFS_FILE_MAX > 2147483647 is no longer supported. It's unclear if this ever worked, and at least half the file API was unusable in this state.

What's new?

  • lfs_fs_gc can now compact metadata (#913)

    This is controlled by the new optional compact_thresh configuration option. mdirs above compact_thresh will be eagerly compacted during lfs_fs_gc, making it less likely (but still possible) for compactions to occur during file writes.

    By default, compact_thresh=0, littlefs will compact mdirs > ~88% block_size. This value may change in the future.

    compact_thresh=-1 disables mdir compaction during lfs_fs_gc.

    Note that compact_thresh has no effect on mdir compaction outside of lfs_fs_gc. mdirs must compact when full.

  • Added inline_max, which allows explicit user control over the size of inlined files (#914)

    Decreasing inline_max may improve some metadata-related performance around mdir traversal. But decreasing inline_max may also hurt metadata-related performance around mdir compaction and increase storage consumption. It is up to users to determine the best value per-application.

    By default, inline_max=0, uses the largest possible inline_max. This matches the previous behavior and is probably a good default for most use cases.

    inline_max=-1 disables inlined files.

  • Added easier util overrides for common system functions: LFS_MALLOC, LFS_FREE, LFS_CRC (#909)

    These can now be overridden with simple defines:

    -DLFS_MALLOC=my_malloc
    -DLFS_FREE=my_free
    -DLFS_CRC=my_crc
    

    Note: Overriding LFS_CRC with a non-CRC32 checksum is discouraged. This is only intended for hardware acceleration, etc.

  • Thanks to @tomscii, lfs_rename now returns LFS_ERR_NOTDIR if the source is a regular file and the destination is a directory, which better matches POSIX (#917)

  • Thanks to @BrianPugh, defining overridable limits (LFS_FILE_MAX, LFS_NAME_MAX, LFS_ATTR_MAX) to incompatible values will now result in a compile error (#886)

  • Relaxed lookahead buffer alignment to only be byte-aligned (#912)

  • Relaxed lfs_malloc alignment requirements to only be byte-aligned (#912)

BrianPugh and others added 25 commits October 29, 2023 13:50
Now you can override littlefs's malloc with some simple defines:

  -DLFS_MALLOC=my_malloc
  -DLFS_FREE=my_free

This is probably what most users expected when wanting to override
malloc/free in littlefs, but it hasn't been available, since instead
littlefs provides a file-level override of builtin utils.

The thinking was that there's just too many builtins that could be
overriden, lfs_max/min/alignup/npw2/etc/etc/etc, so allowing users to
just override the util file provides the best flexibility without a ton
of ifdefs.

But it's become clear this is awkward for users that just want to
replace malloc.

Maybe the original goal was too optimistic, maybe there's a better way
to structure this file, or maybe the best API is just a bunch of ifdefs,
I have no idea! This will hopefully continue to evolve.
Now you can override littlefs's CRC implementation with some simple
defines:

  -DLFS_CRC=lfs_crc

The motivation for this is the same for LFS_MALLOC/LFS_FREE. I think
these are the main "system-level" utils that users want to override.

Don't override with this something that's not CRC32! Your filesystem
will no longer be compatible with other tools! This is only intended for
provided hardware acceleration!
- Renamed lfs.free      -> lfs.lookahead
- Renamed lfs.free.off  -> lfs.lookahead.start
- Renamed lfs.free.i    -> lfs.lookahead.next
- Renamed lfs.free.ack  -> lfs.lookahead.ckpoint
- Renamed lfs_alloc_ack -> lfs_alloc_ckpoint

These have been named a bit confusingly, and I think the new names make
their relevant purposes a bit clearer.

At the very it's clear lfs.lookahead is related to the lookahead buffer.
(and doesn't imply a closed free-bitmap).
Some of this is just better documentation, some of this is reworking the
logic to be more intention driven... if that makes sense...
This drops the lookahead buffer from operating on 32-bit words to
operating on 8-bit bytes, and removes any alignment requirement. This
may have some minor performance impact, but it is unlikely to be
significant when you consider IO overhead.

The original motivation for 32-bit alignment was an attempt at
future-proofing in case we wanted some more complex on-disk data
structure. This never happened, and even if it did, it could have been
added via additional config options.

This has been a significant pain point for users, since providing
word-aligned byte-sized buffers in C can be a bit annoying.
If we already have to bump this version as GitHub phases out older
Ubuntu runners (which is reasonable), I don't really see the value of
pinning a specific version. We might as well just respond to any
broken dependencies caused by GitHub's implicit updates as they
happen...

It's not like CI is truly continuous.
The only reason we needed this alignment was for the lookahead buffer.

Now that the lookahead buffer is relaxed to operate on bytes, we can
relax our malloc alignment requirement all the way down to the byte
level, since we mainly use lfs_malloc to allocate byte-level buffers.

This does introduce a risk that we might need word-level mallocs in the
future. If that happens we will need to decide if changing the malloc
alignment is a breaking change, or gate alignment requirements behind
user provided defines.

Found by HiFiPhile.
lfs_init handles the checks/asserts of most configuration, moving these
checks near lfs_init attempts to keep all of these checks nearby each
other.

Also updated the comments to avoid somtimes-ambiguous range notation.

And removed negative bounds checks. Negative bounds should be obviously
incorrect, and 0 is _technically_ not illegal for any define (though
admittedly unlikely to be correct).
I think realistically no one is using this. It's already only partially
supported and untested.

Worst case, if someone does depend on this we can always revert.
When lfs_rename() is called trying to rename (move) a file to an
existing directory, LFS_ERR_ISDIR is (correctly) returned. However, in
the opposite case, if one tries to rename (move) a directory to a path
currently occupied by a regular file, LFS_ERR_NOTDIR should be
returned (since the error is that the destination is NOT a directory),
but in reality, LFS_ERR_ISDIR is returned in this case as well.

This commit fixes the code so that in the latter case, LFS_ERR_NOTDIR
is returned.
Add value-range checks for user-definable macros at compile-time
Add some easier util overrides: LFS_MALLOC/FREE/CRC
This extends lfs_fs_gc to now handle three things:

1. Calls mkconsistent if not already consistent
2. Compacts metadata > compact_thresh
3. Populates the block allocator

Which should be all of the janitorial work that can be done without
additional on-disk data structures.

Normally, metadata compaction occurs when an mdir is full, and results in
mdirs that are at most block_size/2.

Now, if you call lfs_fs_gc, littlefs will eagerly compact any mdirs that
exceed the compact_thresh configuration option. Because the resulting
mdirs are at most block_size/2, it only makes sense for compact_thresh to
be >= block_size/2 and <= block_size.

Additionally, there are some special values:

- compact_thresh=0  => defaults to ~88% block_size, may change
- compact_thresh=-1 => disables metadata compaction during lfs_fs_gc

Note that compact_thresh only affects lfs_fs_gc. Normal compactions
still only occur when full.
Relaxed lookahead alignment, other internal block alloc readability improvements
Extend lfs_fs_gc to compact metadata, compact_thresh
Inlined files live in metadata and decrease storage requirements, but
may be limited to improve metadata-related performance. This is
especially important given the current plague of metadata performance.

Though decreasing inline_max may make metadata more dense and increase
block usage, so it's important to benchmark if optimizing for speed.

The underlying limits of inlined files haven't changed:
1. Inlined files need to fit in RAM, so <= cache_size
2. Inlined files need to fit in a single attr, so <= attr_max
3. Inlined files need to fit in 1/8 of a block to avoid metadata
   overflow issues, this is after limiting by metadata_max,
   so <= min(metadata_max, block_size)/8

By default, the largest possible inline_max is used. This preserves
backwards compatibility and is probably a good default for most use
cases.

This does have the awkward effect of requiring inline_max=-1 to
indicate disabled inlined files, but I don't think there's a good
way around this.
Add inline_max, to optionally limit the size of inlined files
Change CI to just run on ubuntu-latest
So instead of lfs_file_rawopencfg, it's now lfs_file_opencfg_.

The "raw" prefix is annoying, doesn't really add meaning ("internal"
would have been better), and gets in the way of finding the relevant
function implementations.

I have been using _s as suffixes for unimportant name collisions in
other codebases, and it seems to work well at reducing wasted brain
cycles naming things. Adopting it here avoids the need for "raw"
prefixes.

It's quite a bit like the use of prime symbols to resolve name
collisions in math, e.g. x' = x + 1. Which is even supported in Haskell
and is quite nice there.

And the main benefit: Now if you search for the public API name, you get
the internal function first, which is probably what you care about.

Here is the exact script:

  sed -i 's/_raw\([a-z0-9_]*\)\>/_\1_/g' $(git ls-tree -r HEAD --name-only | grep '.*\.c')
Rename internal functions _raw* -> _*_
@geky geky added this to the v2.9 milestone Jan 19, 2024
@geky-bot
Copy link
Collaborator

Tests passed ✓, Code: 16972 B (+0.9%), Stack: 1432 B (-1.1%), Structs: 812 B (+1.5%)
Code Stack Structs Coverage
Default 16972 B (+0.9%) 1432 B (-1.1%) 812 B (+1.5%) Lines 2387/2566 lines (-0.1%)
Readonly 6186 B (+0.9%) 448 B (+0.0%) 812 B (+1.5%) Branches 1239/1576 branches (-0.1%)
Threadsafe 17836 B (+0.8%) 1432 B (-1.1%) 820 B (+1.5%) Benchmarks
Multiversion 17036 B (+0.9%) 1432 B (-1.1%) 816 B (+1.5%) Readed 29369693876 B (+0.0%)
Migrate 18664 B (+0.8%) 1736 B (-0.9%) 816 B (+1.5%) Proged 1482874766 B (+0.0%)
Error-asserts 17660 B (+1.0%) 1424 B (-1.1%) 812 B (+1.5%) Erased 1568888832 B (+0.0%)

@geky geky merged commit f53a0cc into master Jan 23, 2024
109 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants