Compare commits

...

11 Commits

Author SHA1 Message Date
Christopher Haster
b93cd18bf5 Tighten bound on mlist isopen asserts
Unbalanced open/close calls continue to be a pain point for users, it
doesn't help that this sometimes results in hard-to-debug infinite loops
caused by the open-file linked-list (the mlist) getting tangled up in
itself.

Moving the mlist isopen asserts lower into the actual list append/remove
functions will help:

1. Make sure coverage of potential linked-list issues is complete.

2. Also assert against multiple close calls, which isn't an issue for
   the mlist, but can result in double free and memory corruption.
2024-01-16 13:13:49 -06:00
Christopher Haster
3513ff1afc Merge pull request #911 from littlefs-project/fix-release-structs
Fix struct sizes missing from generated release notes
2023-12-21 00:08:16 -06:00
Christopher Haster
8a22bd6e67 Merge pull request #910 from littlefs-project/fix-superblock-expansion-thresh
Increase threshold for superblock expansion from ~50% -> ~88% full
2023-12-21 00:07:55 -06:00
Christopher Haster
9b82db72d8 Merge pull request #898 from zchen24/patch-1
Update DESIGN.md minor typo
2023-12-21 00:06:29 -06:00
Zihan Chen
99b84ee3db Update DESIGN.md, fix minor typo 2023-12-20 23:42:26 -06:00
Christopher Haster
e91a29d2b5 Fixed struct sizes missing from generated release notes
This script was missed during a struct -> structs naming change
2023-12-19 22:00:18 -06:00
Christopher Haster
b9b95ab4bc Increase threshold for superblock expansion from ~50% -> ~88% full
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
2023-12-19 16:51:17 -06:00
Zihan Chen
10bcff1af8 Update DESIGN.md minor typo 2023-11-26 11:10:24 -08:00
Christopher Haster
c733d9ec57 Merge pull request #884 from DvdGiessen/static-functions
lfs_fs_raw* functions should be static
2023-10-31 13:26:35 -05:00
Christopher Haster
8f3f32d1f3 Added -Wmissing-prototypes
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.
2023-10-24 12:04:54 -05:00
Daniël van de Giessen
92fc780f71 lfs_fs_raw* functions should be static 2023-10-23 13:35:34 +02:00
4 changed files with 16 additions and 21 deletions

View File

@@ -112,7 +112,7 @@ jobs:
table[$i,$j]=$c_camel
((j+=1))
for s in code stack struct
for s in code stack structs
do
f=sizes/thumb${c:+-$c}.$s.csv
[ -e $f ] && table[$i,$j]=$( \

View File

@@ -59,7 +59,7 @@ This leaves us with three major requirements for an embedded filesystem.
RAM to temporarily store filesystem metadata.
For ROM, this means we need to keep our design simple and reuse code paths
were possible. For RAM we have a stronger requirement, all RAM usage is
where possible. For RAM we have a stronger requirement, all RAM usage is
bounded. This means RAM usage does not grow as the filesystem changes in
size or number of files. This creates a unique challenge as even presumably
simple operations, such as traversing the filesystem, become surprisingly
@@ -626,7 +626,7 @@ log&#8322;_n_ pointers that skip to different preceding elements of the
skip-list.
The name comes from heavy use of the [CTZ instruction][wikipedia-ctz], which
lets us calculate the power-of-two factors efficiently. For a give block _n_,
lets us calculate the power-of-two factors efficiently. For a given block _n_,
that block contains ctz(_n_)+1 pointers.
```

View File

@@ -63,6 +63,7 @@ CFLAGS += -fcallgraph-info=su
CFLAGS += -g3
CFLAGS += -I.
CFLAGS += -std=c99 -Wall -Wextra -pedantic
CFLAGS += -Wmissing-prototypes
CFLAGS += -ftrack-macro-expansion=0
ifdef DEBUG
CFLAGS += -O0
@@ -354,6 +355,7 @@ summary-diff sizes-diff: $(OBJ) $(CI)
## Build the test-runner
.PHONY: test-runner build-test
test-runner build-test: CFLAGS+=-Wno-missing-prototypes
ifndef NO_COV
test-runner build-test: CFLAGS+=--coverage
endif
@@ -405,6 +407,7 @@ testmarks-diff: $(TEST_CSV)
## Build the bench-runner
.PHONY: bench-runner build-bench
bench-runner build-bench: CFLAGS+=-Wno-missing-prototypes
ifdef YES_COV
bench-runner build-bench: CFLAGS+=--coverage
endif

28
lfs.c
View File

@@ -505,6 +505,7 @@ static bool lfs_mlist_isopen(struct lfs_mlist *head,
#endif
static void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) {
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, mlist));
for (struct lfs_mlist **p = &lfs->mlist; *p; p = &(*p)->next) {
if (*p == mlist) {
*p = (*p)->next;
@@ -514,6 +515,7 @@ static void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) {
}
static void lfs_mlist_append(lfs_t *lfs, struct lfs_mlist *mlist) {
LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, mlist));
mlist->next = lfs->mlist;
lfs->mlist = mlist;
}
@@ -2151,9 +2153,11 @@ static int lfs_dir_splittingcompact(lfs_t *lfs, lfs_mdir_t *dir,
return size;
}
// do we have extra space? littlefs can't reclaim this space
// by itself, so expand cautiously
if ((lfs_size_t)size < lfs->block_count/2) {
// littlefs cannot reclaim expanded superblocks, so expand cautiously
//
// if our filesystem is more than ~88% full, don't expand, this is
// somewhat arbitrary
if (lfs->block_count - size > lfs->block_count/8) {
LFS_DEBUG("Expanding superblock at rev %"PRIu32, dir->rev);
int err = lfs_dir_split(lfs, dir, attrs, attrcount,
source, begin, end);
@@ -3030,8 +3034,7 @@ static int lfs_file_rawopencfg(lfs_t *lfs, lfs_file_t *file,
// allocate entry for file if it doesn't exist
lfs_stag_t tag = lfs_dir_find(lfs, &file->m, &path, &file->id);
if (tag < 0 && !(tag == LFS_ERR_NOENT && file->id != 0x3ff)) {
err = tag;
goto cleanup;
return tag;
}
// get id, add to list of mdirs to catch update changes
@@ -4999,7 +5002,7 @@ static int lfs_fs_forceconsistency(lfs_t *lfs) {
#endif
#ifndef LFS_READONLY
int lfs_fs_rawmkconsistent(lfs_t *lfs) {
static int lfs_fs_rawmkconsistent(lfs_t *lfs) {
// lfs_fs_forceconsistency does most of the work here
int err = lfs_fs_forceconsistency(lfs);
if (err) {
@@ -5046,7 +5049,7 @@ static lfs_ssize_t lfs_fs_rawsize(lfs_t *lfs) {
}
#ifndef LFS_READONLY
int lfs_fs_rawgrow(lfs_t *lfs, lfs_size_t block_count) {
static int lfs_fs_rawgrow(lfs_t *lfs, lfs_size_t block_count) {
// shrinking is not supported
LFS_ASSERT(block_count >= lfs->block_count);
@@ -5927,7 +5930,6 @@ int lfs_file_open(lfs_t *lfs, lfs_file_t *file, const char *path, int flags) {
}
LFS_TRACE("lfs_file_open(%p, %p, \"%s\", %x)",
(void*)lfs, (void*)file, path, flags);
LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
err = lfs_file_rawopen(lfs, file, path, flags);
@@ -5948,7 +5950,6 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file,
".buffer=%p, .attrs=%p, .attr_count=%"PRIu32"})",
(void*)lfs, (void*)file, path, flags,
(void*)cfg, cfg->buffer, (void*)cfg->attrs, cfg->attr_count);
LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
err = lfs_file_rawopencfg(lfs, file, path, flags, cfg);
@@ -5963,7 +5964,6 @@ int lfs_file_close(lfs_t *lfs, lfs_file_t *file) {
return err;
}
LFS_TRACE("lfs_file_close(%p, %p)", (void*)lfs, (void*)file);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
err = lfs_file_rawclose(lfs, file);
@@ -5979,7 +5979,6 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) {
return err;
}
LFS_TRACE("lfs_file_sync(%p, %p)", (void*)lfs, (void*)file);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
err = lfs_file_rawsync(lfs, file);
@@ -5997,7 +5996,6 @@ lfs_ssize_t lfs_file_read(lfs_t *lfs, lfs_file_t *file,
}
LFS_TRACE("lfs_file_read(%p, %p, %p, %"PRIu32")",
(void*)lfs, (void*)file, buffer, size);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
lfs_ssize_t res = lfs_file_rawread(lfs, file, buffer, size);
@@ -6015,7 +6013,6 @@ lfs_ssize_t lfs_file_write(lfs_t *lfs, lfs_file_t *file,
}
LFS_TRACE("lfs_file_write(%p, %p, %p, %"PRIu32")",
(void*)lfs, (void*)file, buffer, size);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
lfs_ssize_t res = lfs_file_rawwrite(lfs, file, buffer, size);
@@ -6033,7 +6030,6 @@ lfs_soff_t lfs_file_seek(lfs_t *lfs, lfs_file_t *file,
}
LFS_TRACE("lfs_file_seek(%p, %p, %"PRId32", %d)",
(void*)lfs, (void*)file, off, whence);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
lfs_soff_t res = lfs_file_rawseek(lfs, file, off, whence);
@@ -6050,7 +6046,6 @@ int lfs_file_truncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) {
}
LFS_TRACE("lfs_file_truncate(%p, %p, %"PRIu32")",
(void*)lfs, (void*)file, size);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
err = lfs_file_rawtruncate(lfs, file, size);
@@ -6066,7 +6061,6 @@ lfs_soff_t lfs_file_tell(lfs_t *lfs, lfs_file_t *file) {
return err;
}
LFS_TRACE("lfs_file_tell(%p, %p)", (void*)lfs, (void*)file);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
lfs_soff_t res = lfs_file_rawtell(lfs, file);
@@ -6095,7 +6089,6 @@ lfs_soff_t lfs_file_size(lfs_t *lfs, lfs_file_t *file) {
return err;
}
LFS_TRACE("lfs_file_size(%p, %p)", (void*)lfs, (void*)file);
LFS_ASSERT(lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file));
lfs_soff_t res = lfs_file_rawsize(lfs, file);
@@ -6126,7 +6119,6 @@ int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) {
return err;
}
LFS_TRACE("lfs_dir_open(%p, %p, \"%s\")", (void*)lfs, (void*)dir, path);
LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)dir));
err = lfs_dir_rawopen(lfs, dir, path);