By "luck" the previous code somehow managed to not be broken, though it
was possible to traverse the same file twice in lfs_fs_traverse/size
(which is not an error).
The problem was an underlying assumption in lfs_dir_get that it would
never be called when the requested id is pending removal because of a
powerloss. The assumption was either:
1. lfs_dir_find would need to be called first to find the id, and it
would correctly toss out pending-rms with LFS_ERR_NOENT.
2. lfs_fs_mkconsistent would be implicitly called before any filesystem
traversals, cleaning up any pending-rms. This is at least true for
allocator scans.
But, as noted by andriyndev, both lfs_fs_traverse and lfs_fs_size can
call lfs_fs_get with a pending-rm id if called in a readonly context.
---
By "luck" this somehow manages to not break anything:
1. If the pending-rm id is >0, the id is decremented by 1 in lfs_fs_get,
returning the previous file entry during traversal. Worst case, this
reports any blocks owned by the previous file entry twice.
Note this is not an error, lfs_fs_traverse/size may return the same
block multiple times due to underlying copy-on-write structures.
2. More concerning, if the pending-rm id is 0, the id is decremented by
1 in lfs_fs_get and underflows. This underflow propagates into the
type field of the tag we are searching for, decrementing it from
0x200 (LFS_TYPE_STRUCT) to 0x1ff (LFS_TYPE_INTERNAL(UNUSED)).
Fortunately, since this happens to underflow to the INTERNAL tag
type, the type intended to never exist on disk, we should never find
a matching tag during our lfs_fs_get search. The result? lfs_dir_get
returns LFS_ERR_NOENT, which is actually what we want.
Also note that LFS_ERR_NOENT does not terminate the mdir traversal
early. If it did we would have missed files instead of duplicating
files, which is a slightly worse situation.
---
The fix is to add an explicit check for pending-rms in lfs_dir_get, just
like in lfs_dir_find. This avoids relying on unintended underflow
propagation, and should make the internal API behavior more consistent.
This is especially important for potential future gc extensions.
Found by andriyndev
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')
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.
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.
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.
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.
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).
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.
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.
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.
- 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).
Superblock expansion is an irreversible operation. In an effort to
prevent superblock expansion from claiming valuable scratch space
(important for small, <~8 block filesystems), littlefs prevents
superblock expansion when the disk is "mostly full".
In true computer-scientist fashion, this "mostly full" threshold was
set to ~50%.
As pointed out by gbolgradov and rojer, >~50% utilization is not
uncommon, and it can lead to a situation where superblock expansion does
not occur in a relatively healthy filesystem, causing focused wear at
the root.
To remedy this, the threshold is now increased to ~88% (7/8) full.
This may change in the future and should probably be eventually user
configurable.
Found by gbolgradov and rojer
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!
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.
This warning is useful for catching the easy mistake of missing the
keyword static on functions intended to be internal-only.
Missing the static keyword risks symbol polution and misses potential
compiler optimizations.
This is an interesting warning, while useful for libraries such as
littlefs, it's perfectly valid C to not predeclare all functions, and
common in final application binaries.
Relatedly, this warning is re-disabled for the test/bench runner. There
may be a better way to organize the CFLAGS, maybe into separate
LIB/RUNNER CFLAGS, but I'll leave this to future work if our CFLAGS grow
more complicated.
This was motivated by non-static internal-only functions leaking into a
release. Found and fixed by DvdGiessen.
The idea is in the future this function may be extended to support other
block janitorial work. In such a case calling this lfs_fs_gc provides a
more general name that can include other operations.
This is currently just wishful thinking, however.
- Test that the code actually runs.
- Test that lfs_fs_findfreeblocks does not break block allocations.
- Test that lfs_fs_findfreeblocks does not error when no space is
available, it should only errors when the block is actually needed.
The initial implementation for this was provided by kaetemi, originally
as a mount flag. However, it has been modified here to be self-contained
in an explicit runtime function that can be called after mount.
The reasons for an explicit function:
1. lfs_mount stays a strictly readonly operation, and avoids pulling in
all of the write machinery.
2. filesystem-wide operations such as lfs_fs_grow can be a bit risky,
and irreversable. The action of growing the filesystem should be very
intentional.
---
One concern with this change is that this will be the first function
that changes metadata in the superblock. This might break tools that
expect the first valid superblock entry to contain the most recent
metadata, since only the last superblock in the superblock chain will
contain the updated metadata.
Christopher Haster