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

Extend lfs_fs_gc to compact metadata, compact_thresh #913

Merged
merged 5 commits into from
Jan 19, 2024
Merged

Conversation

geky
Copy link
Member

@geky geky commented Dec 21, 2023

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 in the future
  • 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.

This depends on: #912

geky added 3 commits December 20, 2023 00:17
- 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.
@geky-bot
Copy link
Collaborator

Tests passed ✓, Code: 16964 B (+0.9%), Stack: 1432 B (-1.1%), Structs: 804 B (+0.5%)
Code Stack Structs Coverage
Default 16964 B (+0.9%) 1432 B (-1.1%) 804 B (+0.5%) Lines 2380/2559 lines (-0.1%)
Readonly 6130 B (+0.0%) 448 B (+0.0%) 804 B (+0.5%) Branches 1223/1556 branches (-0.1%)
Threadsafe 17828 B (+0.8%) 1432 B (-1.1%) 812 B (+0.5%) Benchmarks
Multiversion 17028 B (+0.9%) 1432 B (-1.1%) 808 B (+0.5%) Readed 29369693876 B (+0.0%)
Migrate 18644 B (+0.8%) 1736 B (-0.9%) 808 B (+0.5%) Proged 1482874766 B (+0.0%)
Error-asserts 17636 B (+0.9%) 1424 B (-1.1%) 804 B (+0.5%) Erased 1568888832 B (+0.0%)

@geky-bot
Copy link
Collaborator

Tests passed ✓, Code: 16964 B (+0.9%), Stack: 1432 B (-1.1%), Structs: 804 B (+0.5%)
Code Stack Structs Coverage
Default 16964 B (+0.9%) 1432 B (-1.1%) 804 B (+0.5%) Lines 2380/2559 lines (-0.1%)
Readonly 6130 B (+0.0%) 448 B (+0.0%) 804 B (+0.5%) Branches 1223/1556 branches (-0.1%)
Threadsafe 17828 B (+0.8%) 1432 B (-1.1%) 812 B (+0.5%) Benchmarks
Multiversion 17028 B (+0.9%) 1432 B (-1.1%) 808 B (+0.5%) Readed 29369693876 B (+0.0%)
Migrate 18644 B (+0.8%) 1736 B (-0.9%) 808 B (+0.5%) Proged 1482874766 B (+0.0%)
Error-asserts 17636 B (+0.9%) 1424 B (-1.1%) 804 B (+0.5%) Erased 1568888832 B (+0.0%)

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.
@geky geky added next minor and removed needs minor version new functionality only allowed in minor versions labels Jan 17, 2024
@geky geky added this to the v2.9 milestone Jan 17, 2024
@geky geky changed the base branch from master to devel January 19, 2024 18:21
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.
@geky-bot
Copy link
Collaborator

Tests passed ✓, Code: 16972 B, Stack: 1432 B, Structs: 804 B
Code Stack Structs Coverage
Default 16972 B 1432 B 804 B Lines 2380/2559 lines
Readonly 6130 B 448 B 804 B Branches 1223/1556 branches
Threadsafe 17836 B 1432 B 812 B Benchmarks
Multiversion 17036 B 1432 B 808 B Readed 29369693876 B
Migrate 18652 B 1736 B 808 B Proged 1482874766 B
Error-asserts 17640 B 1424 B 804 B Erased 1568888832 B

@geky geky merged commit 09972a1 into devel Jan 19, 2024
109 checks passed
@geky geky mentioned this pull request Jan 19, 2024
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.

2 participants