From 26bee8ad36030c89b1300d5095f56fa635b53b95 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Mon, 7 Apr 2025 16:06:01 +0900 Subject: [PATCH 01/15] drop a few unsupported CFLAGS for clang --- Makefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 51e9f98e..1fd856f9 100644 --- a/Makefile +++ b/Makefile @@ -59,12 +59,15 @@ BENCH_PERF := $(BENCH_RUNNER:%=%.perf) BENCH_TRACE := $(BENCH_RUNNER:%=%.trace) BENCH_CSV := $(BENCH_RUNNER:%=%.csv) -CFLAGS += -fcallgraph-info=su CFLAGS += -g3 CFLAGS += -I. CFLAGS += -std=c99 -Wall -Wextra -pedantic CFLAGS += -Wmissing-prototypes +ifeq ($(shell $(CC) --version | grep clang),) CFLAGS += -ftrack-macro-expansion=0 +CFLAGS += -fcallgraph-info=su +endif + ifdef DEBUG CFLAGS += -O0 else From 0d861b7916809861b62840c42a4df6e05844dd65 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Mon, 7 Apr 2025 16:07:17 +0900 Subject: [PATCH 02/15] adapt the linker sections usage to mach-o "make test" on macOS: ``` using runner: ./runners/test_runner found 19 suites, 188 cases, 11242/11770 permutations running test_alloc: 12/12 cases, 207/207 perms running test_attrs: 4/4 cases, 20/20 perms running test_badblocks: 4/4 cases, 300/300 perms running test_bd: 5/5 cases, 85/85 perms running test_compat: 17/17 cases, 205/205 perms running test_dirs: 15/15 cases, 450/450 perms, 1756pls! running test_entries: 8/8 cases, 32/32 perms running test_evil: 8/8 cases, 105/105 perms running test_exhaustion: 5/5 cases, 85/85 perms running test_files: 10/10 cases, 7155/7155 perms, 9410pls! running test_interspersed: 4/4 cases, 190/190 perms, 2835pls! running test_move: 17/17 cases, 161/161 perms, 157pls! running test_orphans: 6/6 cases, 50/50 perms, 846pls! running test_paths: 33/33 cases, 325/325 perms running test_powerloss: 2/2 cases, 21/21 perms running test_relocations: 4/4 cases, 68/68 perms, 1612pls! running test_seek: 10/10 cases, 195/195 perms, 1050pls! running test_superblocks: 17/17 cases, 318/318 perms, 1437pls! running test_truncate: 7/7 cases, 1270/1270 perms, 9691pls! done: 11242/11242 passed, 0/11242 failed, 28794pls!, in 585.76s ``` --- runners/bench_runner.c | 5 +++++ runners/test_runner.c | 5 +++++ scripts/bench.py | 5 ++++- scripts/test.py | 5 ++++- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/runners/bench_runner.c b/runners/bench_runner.c index d49f9761..e27c1899 100644 --- a/runners/bench_runner.c +++ b/runners/bench_runner.c @@ -123,8 +123,13 @@ typedef struct bench_id { // bench suites are linked into a custom ld section +#if defined(__APPLE__) +extern struct bench_suite __start__bench_suites __asm("section$start$__DATA$_bench_suites"); +extern struct bench_suite __stop__bench_suites __asm("section$end$__DATA$_bench_suites"); +#else extern struct bench_suite __start__bench_suites; extern struct bench_suite __stop__bench_suites; +#endif const struct bench_suite *bench_suites = &__start__bench_suites; #define BENCH_SUITE_COUNT \ diff --git a/runners/test_runner.c b/runners/test_runner.c index 37cd1e7d..76cb1497 100644 --- a/runners/test_runner.c +++ b/runners/test_runner.c @@ -136,8 +136,13 @@ typedef struct test_id { // test suites are linked into a custom ld section +#if defined(__APPLE__) +extern struct test_suite __start__test_suites __asm("section$start$__DATA$_test_suites"); +extern struct test_suite __stop__test_suites __asm("section$end$__DATA$_test_suites"); +#else extern struct test_suite __start__test_suites; extern struct test_suite __stop__test_suites; +#endif const struct test_suite *test_suites = &__start__test_suites; #define TEST_SUITE_COUNT \ diff --git a/scripts/bench.py b/scripts/bench.py index f22841ea..0ed24825 100755 --- a/scripts/bench.py +++ b/scripts/bench.py @@ -404,12 +404,15 @@ def compile(bench_paths, **args): f.writeln() # create suite struct - # + f.writeln('#if defined(__APPLE__)') + f.writeln('__attribute__((section("__DATA,_bench_suites")))') + f.writeln('#else') # note we place this in the custom bench_suites section with # minimum alignment, otherwise GCC ups the alignment to # 32-bytes for some reason f.writeln('__attribute__((section("_bench_suites"), ' 'aligned(1)))') + f.writeln('#endif') f.writeln('const struct bench_suite __bench__%s__suite = {' % suite.name) f.writeln(4*' '+'.name = "%s",' % suite.name) diff --git a/scripts/test.py b/scripts/test.py index e7f78e4c..0b3e68dd 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -412,12 +412,15 @@ def compile(test_paths, **args): f.writeln() # create suite struct - # + f.writeln('#if defined(__APPLE__)') + f.writeln('__attribute__((section("__DATA,_test_suites")))') + f.writeln('#else') # note we place this in the custom test_suites section with # minimum alignment, otherwise GCC ups the alignment to # 32-bytes for some reason f.writeln('__attribute__((section("_test_suites"), ' 'aligned(1)))') + f.writeln('#endif') f.writeln('const struct test_suite __test__%s__suite = {' % suite.name) f.writeln(4*' '+'.name = "%s",' % suite.name) From b82372842038af7714770b1d3fc5fcae46fe9974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20de=20Giessen?= Date: Wed, 16 Apr 2025 18:17:21 +0200 Subject: [PATCH 03/15] lfs_crc should be static if LFS_CRC is defined --- lfs_util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lfs_util.h b/lfs_util.h index 0aec4885..0a0668a4 100644 --- a/lfs_util.h +++ b/lfs_util.h @@ -231,8 +231,8 @@ static inline uint32_t lfs_tobe32(uint32_t a) { // Calculate CRC-32 with polynomial = 0x04c11db7 #ifdef LFS_CRC -uint32_t lfs_crc(uint32_t crc, const void *buffer, size_t size) { - return LFS_CRC(crc, buffer, size) +static inline uint32_t lfs_crc(uint32_t crc, const void *buffer, size_t size) { + return LFS_CRC(crc, buffer, size); } #else uint32_t lfs_crc(uint32_t crc, const void *buffer, size_t size); From 2105e502c526ebd4d0cc7e2dca63e6586f1e884f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 16 Apr 2025 17:46:34 +0200 Subject: [PATCH 04/15] Add support for shrinking a filesystem This PR adds a new `lfs_fs_shrink`, which functions similarly to `lfs_fs_grow`, but supports reducing the block count. This functions first checks that none of the removed block are in use. If it is the case, it will fail. --- lfs.c | 89 ++++++++++++++++++++++-------- lfs.h | 11 ++++ tests/test_shrink.toml | 104 +++++++++++++++++++++++++++++++++++ tests/test_superblocks.toml | 106 ++++++++++++++++++++++++++++++++++++ 4 files changed, 288 insertions(+), 22 deletions(-) create mode 100644 tests/test_shrink.toml diff --git a/lfs.c b/lfs.c index 28a632b6..874715be 100644 --- a/lfs.c +++ b/lfs.c @@ -5233,38 +5233,67 @@ static int lfs_fs_gc_(lfs_t *lfs) { #endif #ifndef LFS_READONLY +static int lfs_fs_rewrite_block_count(lfs_t *lfs, lfs_size_t block_count) { + lfs->block_count = block_count; + + // fetch the root + lfs_mdir_t root; + int err = lfs_dir_fetch(lfs, &root, lfs->root); + if (err) { + return err; + } + + // update the superblock + lfs_superblock_t superblock; + lfs_stag_t tag = lfs_dir_get(lfs, &root, LFS_MKTAG(0x7ff, 0x3ff, 0), + LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), + &superblock); + if (tag < 0) { + return tag; + } + lfs_superblock_fromle32(&superblock); + + superblock.block_count = lfs->block_count; + + lfs_superblock_tole32(&superblock); + err = lfs_dir_commit(lfs, &root, LFS_MKATTRS( + {tag, &superblock})); + if (err) { + return err; + } + return 0; +} + static int lfs_fs_grow_(lfs_t *lfs, lfs_size_t block_count) { // shrinking is not supported LFS_ASSERT(block_count >= lfs->block_count); if (block_count > lfs->block_count) { - lfs->block_count = block_count; + return lfs_fs_rewrite_block_count(lfs, block_count); + } - // fetch the root - lfs_mdir_t root; - int err = lfs_dir_fetch(lfs, &root, lfs->root); + return 0; +} + +static int lfs_shrink_check_block(void * data, lfs_block_t block) { + lfs_size_t threshold = *((lfs_size_t *) data); + if (block >= threshold) { + return LFS_ERR_NOTEMPTY; + } + return 0; +} + +static int lfs_fs_shrink_(lfs_t *lfs, lfs_size_t block_count) { + if (block_count != lfs->block_count) { + + lfs_block_t threshold = block_count; + + int err = lfs_fs_traverse_(lfs, lfs_shrink_check_block, &threshold, true); if (err) { return err; } - // update the superblock - lfs_superblock_t superblock; - lfs_stag_t tag = lfs_dir_get(lfs, &root, LFS_MKTAG(0x7ff, 0x3ff, 0), - LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), - &superblock); - if (tag < 0) { - return tag; - } - lfs_superblock_fromle32(&superblock); - - superblock.block_count = lfs->block_count; - - lfs_superblock_tole32(&superblock); - err = lfs_dir_commit(lfs, &root, LFS_MKATTRS( - {tag, &superblock})); - if (err) { - return err; - } + return lfs_fs_rewrite_block_count(lfs, block_count); } return 0; @@ -6485,6 +6514,22 @@ int lfs_fs_grow(lfs_t *lfs, lfs_size_t block_count) { } #endif +#ifndef LFS_READONLY +int lfs_fs_shrink(lfs_t *lfs, lfs_size_t block_count) { + int err = LFS_LOCK(lfs->cfg); + if (err) { + return err; + } + LFS_TRACE("lfs_fs_shrink(%p, %"PRIu32")", (void*)lfs, block_count); + + err = lfs_fs_shrink_(lfs, block_count); + + LFS_TRACE("lfs_fs_shrink -> %d", err); + LFS_UNLOCK(lfs->cfg); + return err; +} +#endif + #ifdef LFS_MIGRATE int lfs_migrate(lfs_t *lfs, const struct lfs_config *cfg) { int err = LFS_LOCK(cfg); diff --git a/lfs.h b/lfs.h index 45315603..072b290b 100644 --- a/lfs.h +++ b/lfs.h @@ -772,6 +772,17 @@ int lfs_fs_gc(lfs_t *lfs); int lfs_fs_grow(lfs_t *lfs, lfs_size_t block_count); #endif +#ifndef LFS_READONLY +// Shrinks the filesystem to a new size, updating the superblock with the new +// block count. +// +// Note: This first checks that none of the blocks that are being removed are in use +// and will fail if it is the case +// +// Returns a negative error code on failure. +int lfs_fs_shrink(lfs_t *lfs, lfs_size_t block_count); +#endif + #ifndef LFS_READONLY #ifdef LFS_MIGRATE // Attempts to migrate a previous version of littlefs diff --git a/tests/test_shrink.toml b/tests/test_shrink.toml new file mode 100644 index 00000000..0c317d66 --- /dev/null +++ b/tests/test_shrink.toml @@ -0,0 +1,104 @@ +# simple shrink +[cases.test_shrink_simple] +defines.BLOCK_COUNT = [10, 15, 20] +defines.AFTER_BLOCK_COUNT = [5, 10, 15, 19] +if = "AFTER_BLOCK_COUNT <= BLOCK_COUNT" +code = ''' + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + lfs_mount(&lfs, cfg) => 0; + lfs_fs_shrink(&lfs, AFTER_BLOCK_COUNT) => 0; + lfs_unmount(&lfs); + if (BLOCK_COUNT != AFTER_BLOCK_COUNT) { + lfs_mount(&lfs, cfg) => LFS_ERR_INVAL; + } + lfs_t lfs2 = lfs; + struct lfs_config cfg2 = *cfg; + cfg2.block_count = AFTER_BLOCK_COUNT; + lfs2.cfg = &cfg2; + lfs_mount(&lfs2, &cfg2) => 0; + lfs_unmount(&lfs2) => 0; +''' + +# shrinking full +[cases.test_shrink_full] +defines.BLOCK_COUNT = [10, 15, 20] +defines.AFTER_BLOCK_COUNT = [5, 7, 10, 12, 15, 17, 20] +defines.FILES_COUNT = [7, 8, 9, 10] +if = "AFTER_BLOCK_COUNT <= BLOCK_COUNT && FILES_COUNT + 2 < BLOCK_COUNT" +code = ''' + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + // create FILES_COUNT files of BLOCK_SIZE - 50 bytes (to avoid inlining) + lfs_mount(&lfs, cfg) => 0; + for (int i = 0; i < FILES_COUNT + 1; i++) { + lfs_file_t file; + char path[1024]; + sprintf(path, "file_%03d", i); + lfs_file_open(&lfs, &file, path, + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + char wbuffer[BLOCK_SIZE]; + memset(wbuffer, 'b', BLOCK_SIZE); + // Ensure one block is taken per file, but that files are not inlined. + lfs_size_t size = BLOCK_SIZE - 0x40; + sprintf(wbuffer, "Hi %03d", i); + lfs_file_write(&lfs, &file, wbuffer, size) => size; + lfs_file_close(&lfs, &file) => 0; + } + + int err = lfs_fs_shrink(&lfs, AFTER_BLOCK_COUNT); + if (err == 0) { + for (int i = 0; i < FILES_COUNT + 1; i++) { + lfs_file_t file; + char path[1024]; + sprintf(path, "file_%03d", i); + lfs_file_open(&lfs, &file, path, + LFS_O_RDONLY ) => 0; + lfs_size_t size = BLOCK_SIZE - 0x40; + char wbuffer[size]; + char wbuffer_ref[size]; + // Ensure one block is taken per file, but that files are not inlined. + memset(wbuffer_ref, 'b', size); + sprintf(wbuffer_ref, "Hi %03d", i); + lfs_file_read(&lfs, &file, wbuffer, BLOCK_SIZE) => size; + lfs_file_close(&lfs, &file) => 0; + for (lfs_size_t j = 0; j < size; j++) { + wbuffer[j] => wbuffer_ref[j]; + } + } + } else { + assert(err == LFS_ERR_NOTEMPTY); + } + + lfs_unmount(&lfs) => 0; + if (err == 0 ) { + if ( AFTER_BLOCK_COUNT != BLOCK_COUNT ) { + lfs_mount(&lfs, cfg) => LFS_ERR_INVAL; + } + + lfs_t lfs2 = lfs; + struct lfs_config cfg2 = *cfg; + cfg2.block_count = AFTER_BLOCK_COUNT; + lfs2.cfg = &cfg2; + lfs_mount(&lfs2, &cfg2) => 0; + for (int i = 0; i < FILES_COUNT + 1; i++) { + lfs_file_t file; + char path[1024]; + sprintf(path, "file_%03d", i); + lfs_file_open(&lfs2, &file, path, + LFS_O_RDONLY ) => 0; + lfs_size_t size = BLOCK_SIZE - 0x40; + char wbuffer[size]; + char wbuffer_ref[size]; + // Ensure one block is taken per file, but that files are not inlined. + memset(wbuffer_ref, 'b', size); + sprintf(wbuffer_ref, "Hi %03d", i); + lfs_file_read(&lfs2, &file, wbuffer, BLOCK_SIZE) => size; + lfs_file_close(&lfs2, &file) => 0; + for (lfs_size_t j = 0; j < size; j++) { + wbuffer[j] => wbuffer_ref[j]; + } + } + lfs_unmount(&lfs2); + } +''' diff --git a/tests/test_superblocks.toml b/tests/test_superblocks.toml index 5911c9b0..07961751 100644 --- a/tests/test_superblocks.toml +++ b/tests/test_superblocks.toml @@ -524,6 +524,112 @@ code = ''' lfs_unmount(&lfs) => 0; ''' + +# mount and grow the filesystem +[cases.test_superblocks_shrink] +defines.BLOCK_COUNT = 'ERASE_COUNT' +defines.BLOCK_COUNT_2 = ['ERASE_COUNT/2', 'ERASE_COUNT/4', '2'] +defines.KNOWN_BLOCK_COUNT = [true, false] +code = ''' + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + if (KNOWN_BLOCK_COUNT) { + cfg->block_count = BLOCK_COUNT; + } else { + cfg->block_count = 0; + } + + // mount with block_size < erase_size + lfs_mount(&lfs, cfg) => 0; + struct lfs_fsinfo fsinfo; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT); + lfs_unmount(&lfs) => 0; + + // same size is a noop + lfs_mount(&lfs, cfg) => 0; + lfs_fs_shrink(&lfs, BLOCK_COUNT) => 0; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT); + lfs_unmount(&lfs) => 0; + + lfs_mount(&lfs, cfg) => 0; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT); + lfs_unmount(&lfs) => 0; + + // grow to new size + lfs_mount(&lfs, cfg) => 0; + lfs_fs_shrink(&lfs, BLOCK_COUNT_2) => 0; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT_2); + lfs_unmount(&lfs) => 0; + + if (KNOWN_BLOCK_COUNT) { + cfg->block_count = BLOCK_COUNT_2; + } else { + cfg->block_count = 0; + } + + lfs_mount(&lfs, cfg) => 0; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT_2); + lfs_unmount(&lfs) => 0; + + // mounting with the previous size should fail + cfg->block_count = BLOCK_COUNT; + lfs_mount(&lfs, cfg) => LFS_ERR_INVAL; + + if (KNOWN_BLOCK_COUNT) { + cfg->block_count = BLOCK_COUNT_2; + } else { + cfg->block_count = 0; + } + + // same size is a noop + lfs_mount(&lfs, cfg) => 0; + lfs_fs_shrink(&lfs, BLOCK_COUNT_2) => 0; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT_2); + lfs_unmount(&lfs) => 0; + + lfs_mount(&lfs, cfg) => 0; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT_2); + lfs_unmount(&lfs) => 0; + + // do some work + lfs_mount(&lfs, cfg) => 0; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT_2); + lfs_file_t file; + lfs_file_open(&lfs, &file, "test", + LFS_O_CREAT | LFS_O_EXCL | LFS_O_WRONLY) => 0; + lfs_file_write(&lfs, &file, "hello!", 6) => 6; + lfs_file_close(&lfs, &file) => 0; + lfs_unmount(&lfs) => 0; + + lfs_mount(&lfs, cfg) => 0; + lfs_fs_stat(&lfs, &fsinfo) => 0; + assert(fsinfo.block_size == BLOCK_SIZE); + assert(fsinfo.block_count == BLOCK_COUNT_2); + lfs_file_open(&lfs, &file, "test", LFS_O_RDONLY) => 0; + uint8_t buffer[256]; + lfs_file_read(&lfs, &file, buffer, sizeof(buffer)) => 6; + lfs_file_close(&lfs, &file) => 0; + assert(memcmp(buffer, "hello!", 6) == 0); + lfs_unmount(&lfs) => 0; +''' + # test that metadata_max does not cause problems for superblock compaction [cases.test_superblocks_metadata_max] defines.METADATA_MAX = [ From 0634d13e07d831a2bda7efeb38287dcaf06ffac2 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 3 May 2025 17:15:26 -0500 Subject: [PATCH 05/15] tests: Added non-reentrant variants of orphan/relocation tests These are the same as the related reentrant variants, but by opting out of powerloss testing, we can test a much larger number of states without having to worry about the impact on powerloss testing runtime. Bumped CYCLES from 20 -> 2000. This reveals an orphan remove bug found by Hugh-Baoa. --- tests/test_orphans.toml | 69 ++++++++++++++- tests/test_relocations.toml | 168 ++++++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 1 deletion(-) diff --git a/tests/test_orphans.toml b/tests/test_orphans.toml index d7040ed0..76882800 100644 --- a/tests/test_orphans.toml +++ b/tests/test_orphans.toml @@ -207,7 +207,8 @@ code = ''' [cases.test_orphans_reentrant] reentrant = true # TODO fix this case, caused by non-DAG trees -if = '!(DEPTH == 3 && CACHE_SIZE != 64)' +# NOTE the second condition is required +if = '!(DEPTH == 3 && CACHE_SIZE != 64) && 2*FILES < BLOCK_COUNT' defines = [ {FILES=6, DEPTH=1, CYCLES=20}, {FILES=26, DEPTH=1, CYCLES=20}, @@ -271,3 +272,69 @@ code = ''' lfs_unmount(&lfs) => 0; ''' +# non-reentrant testing for orphans, this is the same as reentrant +# testing, but we test way more states than we could under powerloss +[cases.test_orphans_nonreentrant] +# TODO fix this case, caused by non-DAG trees +# NOTE the second condition is required +if = '!(DEPTH == 3 && CACHE_SIZE != 64) && 2*FILES < BLOCK_COUNT' +defines = [ + {FILES=6, DEPTH=1, CYCLES=2000}, + {FILES=26, DEPTH=1, CYCLES=2000}, + {FILES=3, DEPTH=3, CYCLES=2000}, +] +code = ''' + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + lfs_mount(&lfs, cfg) => 0; + + uint32_t prng = 1; + const char alpha[] = "abcdefghijklmnopqrstuvwxyz"; + for (unsigned i = 0; i < CYCLES; i++) { + // create random path + char full_path[256]; + for (unsigned d = 0; d < DEPTH; d++) { + sprintf(&full_path[2*d], "/%c", alpha[TEST_PRNG(&prng) % FILES]); + } + + // if it does not exist, we create it, else we destroy + struct lfs_info info; + int res = lfs_stat(&lfs, full_path, &info); + if (res == LFS_ERR_NOENT) { + // create each directory in turn, ignore if dir already exists + for (unsigned d = 0; d < DEPTH; d++) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + int err = lfs_mkdir(&lfs, path); + assert(!err || err == LFS_ERR_EXIST); + } + + for (unsigned d = 0; d < DEPTH; d++) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + lfs_stat(&lfs, path, &info) => 0; + assert(strcmp(info.name, &path[2*d+1]) == 0); + assert(info.type == LFS_TYPE_DIR); + } + } else { + // is valid dir? + assert(strcmp(info.name, &full_path[2*(DEPTH-1)+1]) == 0); + assert(info.type == LFS_TYPE_DIR); + + // try to delete path in reverse order, ignore if dir is not empty + for (int d = DEPTH-1; d >= 0; d--) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + int err = lfs_remove(&lfs, path); + assert(!err || err == LFS_ERR_NOTEMPTY); + } + + lfs_stat(&lfs, full_path, &info) => LFS_ERR_NOENT; + } + } + lfs_unmount(&lfs) => 0; +''' + diff --git a/tests/test_relocations.toml b/tests/test_relocations.toml index d20cb8cf..060e865b 100644 --- a/tests/test_relocations.toml +++ b/tests/test_relocations.toml @@ -341,3 +341,171 @@ code = ''' } lfs_unmount(&lfs) => 0; ''' + +# non-reentrant testing for orphans, this is the same as reentrant +# testing, but we test way more states than we could under powerloss +[cases.test_relocations_nonreentrant] +# TODO fix this case, caused by non-DAG trees +# NOTE the second condition is required +if = '!(DEPTH == 3 && CACHE_SIZE != 64) && 2*FILES < BLOCK_COUNT' +defines = [ + {FILES=6, DEPTH=1, CYCLES=2000, BLOCK_CYCLES=1}, + {FILES=26, DEPTH=1, CYCLES=2000, BLOCK_CYCLES=1}, + {FILES=3, DEPTH=3, CYCLES=2000, BLOCK_CYCLES=1}, +] +code = ''' + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + lfs_mount(&lfs, cfg) => 0; + + uint32_t prng = 1; + const char alpha[] = "abcdefghijklmnopqrstuvwxyz"; + for (unsigned i = 0; i < CYCLES; i++) { + // create random path + char full_path[256]; + for (unsigned d = 0; d < DEPTH; d++) { + sprintf(&full_path[2*d], "/%c", alpha[TEST_PRNG(&prng) % FILES]); + } + + // if it does not exist, we create it, else we destroy + struct lfs_info info; + int res = lfs_stat(&lfs, full_path, &info); + if (res == LFS_ERR_NOENT) { + // create each directory in turn, ignore if dir already exists + for (unsigned d = 0; d < DEPTH; d++) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + int err = lfs_mkdir(&lfs, path); + assert(!err || err == LFS_ERR_EXIST); + } + + for (unsigned d = 0; d < DEPTH; d++) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + lfs_stat(&lfs, path, &info) => 0; + assert(strcmp(info.name, &path[2*d+1]) == 0); + assert(info.type == LFS_TYPE_DIR); + } + } else { + // is valid dir? + assert(strcmp(info.name, &full_path[2*(DEPTH-1)+1]) == 0); + assert(info.type == LFS_TYPE_DIR); + + // try to delete path in reverse order, ignore if dir is not empty + for (unsigned d = DEPTH-1; d+1 > 0; d--) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + int err = lfs_remove(&lfs, path); + assert(!err || err == LFS_ERR_NOTEMPTY); + } + + lfs_stat(&lfs, full_path, &info) => LFS_ERR_NOENT; + } + } + lfs_unmount(&lfs) => 0; +''' + +# non-reentrant testing for relocations, but now with random renames! +[cases.test_relocations_nonreentrant_renames] +# TODO fix this case, caused by non-DAG trees +# NOTE the second condition is required +if = '!(DEPTH == 3 && CACHE_SIZE != 64) && 2*FILES < BLOCK_COUNT' +defines = [ + {FILES=6, DEPTH=1, CYCLES=2000, BLOCK_CYCLES=1}, + {FILES=26, DEPTH=1, CYCLES=2000, BLOCK_CYCLES=1}, + {FILES=3, DEPTH=3, CYCLES=2000, BLOCK_CYCLES=1}, +] +code = ''' + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + lfs_mount(&lfs, cfg) => 0; + + uint32_t prng = 1; + const char alpha[] = "abcdefghijklmnopqrstuvwxyz"; + for (unsigned i = 0; i < CYCLES; i++) { + // create random path + char full_path[256]; + for (unsigned d = 0; d < DEPTH; d++) { + sprintf(&full_path[2*d], "/%c", alpha[TEST_PRNG(&prng) % FILES]); + } + + // if it does not exist, we create it, else we destroy + struct lfs_info info; + int res = lfs_stat(&lfs, full_path, &info); + assert(!res || res == LFS_ERR_NOENT); + if (res == LFS_ERR_NOENT) { + // create each directory in turn, ignore if dir already exists + for (unsigned d = 0; d < DEPTH; d++) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + int err = lfs_mkdir(&lfs, path); + assert(!err || err == LFS_ERR_EXIST); + } + + for (unsigned d = 0; d < DEPTH; d++) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + lfs_stat(&lfs, path, &info) => 0; + assert(strcmp(info.name, &path[2*d+1]) == 0); + assert(info.type == LFS_TYPE_DIR); + } + } else { + assert(strcmp(info.name, &full_path[2*(DEPTH-1)+1]) == 0); + assert(info.type == LFS_TYPE_DIR); + + // create new random path + char new_path[256]; + for (unsigned d = 0; d < DEPTH; d++) { + sprintf(&new_path[2*d], "/%c", alpha[TEST_PRNG(&prng) % FILES]); + } + + // if new path does not exist, rename, otherwise destroy + res = lfs_stat(&lfs, new_path, &info); + assert(!res || res == LFS_ERR_NOENT); + if (res == LFS_ERR_NOENT) { + // stop once some dir is renamed + for (unsigned d = 0; d < DEPTH; d++) { + char path[1024]; + strcpy(&path[2*d], &full_path[2*d]); + path[2*d+2] = '\0'; + strcpy(&path[128+2*d], &new_path[2*d]); + path[128+2*d+2] = '\0'; + int err = lfs_rename(&lfs, path, path+128); + assert(!err || err == LFS_ERR_NOTEMPTY); + if (!err) { + strcpy(path, path+128); + } + } + + for (unsigned d = 0; d < DEPTH; d++) { + char path[1024]; + strcpy(path, new_path); + path[2*d+2] = '\0'; + lfs_stat(&lfs, path, &info) => 0; + assert(strcmp(info.name, &path[2*d+1]) == 0); + assert(info.type == LFS_TYPE_DIR); + } + + lfs_stat(&lfs, full_path, &info) => LFS_ERR_NOENT; + } else { + // try to delete path in reverse order, + // ignore if dir is not empty + for (unsigned d = DEPTH-1; d+1 > 0; d--) { + char path[1024]; + strcpy(path, full_path); + path[2*d+2] = '\0'; + int err = lfs_remove(&lfs, path); + assert(!err || err == LFS_ERR_NOTEMPTY); + } + + lfs_stat(&lfs, full_path, &info) => LFS_ERR_NOENT; + } + } + } + lfs_unmount(&lfs) => 0; +''' From a3d6bec5f0e90a5a5b96cccab21fd1bf8fb97f17 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 3 May 2025 17:20:02 -0500 Subject: [PATCH 06/15] Fixed a double deorphan caused by relocation mid dir remove Long story short: There is a specific case where removing a directory can trigger a deorphan pass, but lfs_remove did not check for this, would try to clean up the (already cleaned) directory orphan, and trigger an assert: lfs.c:4890:assert: assert failed with false, expected eq true LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) > 0x000 || orphans >= 0); The specific case being a remove commit that triggers a relocation that creates an orphan. This is also possible in lfs_rename, but only if you're renaming a directory that implies a remove, which is a pretty rare operation. --- This was probably an oversight introduced in the non-recursive commit logic rework. Fortunately the fix is to just check if we even have an orphan before trying to remove it. We can rely on this instead of the file type, so this fix shouldn't even increase the code size. Found and root-caused by Hugh-Baoa --- lfs.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lfs.c b/lfs.c index 28a632b6..05b1ca10 100644 --- a/lfs.c +++ b/lfs.c @@ -3932,7 +3932,9 @@ static int lfs_remove_(lfs_t *lfs, const char *path) { } lfs->mlist = dir.next; - if (lfs_tag_type3(tag) == LFS_TYPE_DIR) { + if (lfs_gstate_hasorphans(&lfs->gstate)) { + LFS_ASSERT(lfs_tag_type3(tag) == LFS_TYPE_DIR); + // fix orphan err = lfs_fs_preporphans(lfs, -1); if (err) { @@ -4076,8 +4078,10 @@ static int lfs_rename_(lfs_t *lfs, const char *oldpath, const char *newpath) { } lfs->mlist = prevdir.next; - if (prevtag != LFS_ERR_NOENT - && lfs_tag_type3(prevtag) == LFS_TYPE_DIR) { + if (lfs_gstate_hasorphans(&lfs->gstate)) { + LFS_ASSERT(prevtag != LFS_ERR_NOENT + && lfs_tag_type3(prevtag) == LFS_TYPE_DIR); + // fix orphan err = lfs_fs_preporphans(lfs, -1); if (err) { From 9b8f802b4357e7cb8d8d6e6dc5c768ad2178c1c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Mon, 5 May 2025 11:37:39 +0200 Subject: [PATCH 07/15] fixup! Add support for shrinking a filesystem --- lfs.c | 84 ++++++++++++++----------------------- lfs.h | 15 ++----- tests/test_shrink.toml | 4 +- tests/test_superblocks.toml | 6 +-- 4 files changed, 40 insertions(+), 69 deletions(-) diff --git a/lfs.c b/lfs.c index 874715be..3d128ff7 100644 --- a/lfs.c +++ b/lfs.c @@ -5233,12 +5233,41 @@ static int lfs_fs_gc_(lfs_t *lfs) { #endif #ifndef LFS_READONLY -static int lfs_fs_rewrite_block_count(lfs_t *lfs, lfs_size_t block_count) { +#ifdef LFS_SHRINKIFCHEAP +static int lfs_shrink_checkblock(void * data, lfs_block_t block) { + lfs_size_t threshold = *((lfs_size_t *) data); + if (block >= threshold) { + return LFS_ERR_NOTEMPTY; + } + return 0; +} +#endif + +static int lfs_fs_grow_(lfs_t *lfs, lfs_size_t block_count) { + int err; + + if (block_count == lfs->block_count) { + return 0; + } + + +#ifndef LFS_SHRINKIFCHEAP + // shrinking is not supported + LFS_ASSERT(block_count >= lfs->block_count); +#endif +#ifdef LFS_SHRINKIFCHEAP + lfs_block_t threshold = block_count; + err = lfs_fs_traverse_(lfs, lfs_shrink_checkblock, &threshold, true); + if (err) { + return err; + } +#endif + lfs->block_count = block_count; // fetch the root lfs_mdir_t root; - int err = lfs_dir_fetch(lfs, &root, lfs->root); + err = lfs_dir_fetch(lfs, &root, lfs->root); if (err) { return err; } @@ -5263,41 +5292,6 @@ static int lfs_fs_rewrite_block_count(lfs_t *lfs, lfs_size_t block_count) { } return 0; } - -static int lfs_fs_grow_(lfs_t *lfs, lfs_size_t block_count) { - // shrinking is not supported - LFS_ASSERT(block_count >= lfs->block_count); - - if (block_count > lfs->block_count) { - return lfs_fs_rewrite_block_count(lfs, block_count); - } - - return 0; -} - -static int lfs_shrink_check_block(void * data, lfs_block_t block) { - lfs_size_t threshold = *((lfs_size_t *) data); - if (block >= threshold) { - return LFS_ERR_NOTEMPTY; - } - return 0; -} - -static int lfs_fs_shrink_(lfs_t *lfs, lfs_size_t block_count) { - if (block_count != lfs->block_count) { - - lfs_block_t threshold = block_count; - - int err = lfs_fs_traverse_(lfs, lfs_shrink_check_block, &threshold, true); - if (err) { - return err; - } - - return lfs_fs_rewrite_block_count(lfs, block_count); - } - - return 0; -} #endif #ifdef LFS_MIGRATE @@ -6514,22 +6508,6 @@ int lfs_fs_grow(lfs_t *lfs, lfs_size_t block_count) { } #endif -#ifndef LFS_READONLY -int lfs_fs_shrink(lfs_t *lfs, lfs_size_t block_count) { - int err = LFS_LOCK(lfs->cfg); - if (err) { - return err; - } - LFS_TRACE("lfs_fs_shrink(%p, %"PRIu32")", (void*)lfs, block_count); - - err = lfs_fs_shrink_(lfs, block_count); - - LFS_TRACE("lfs_fs_shrink -> %d", err); - LFS_UNLOCK(lfs->cfg); - return err; -} -#endif - #ifdef LFS_MIGRATE int lfs_migrate(lfs_t *lfs, const struct lfs_config *cfg) { int err = LFS_LOCK(cfg); diff --git a/lfs.h b/lfs.h index 072b290b..be98b54b 100644 --- a/lfs.h +++ b/lfs.h @@ -766,23 +766,16 @@ int lfs_fs_gc(lfs_t *lfs); // Grows the filesystem to a new size, updating the superblock with the new // block count. // +// if LFS_SHRINKIFCHEAP is defined, this function will also accept +// block_counts smaller than the current configuration, after checking +// that none of the blocks that are being removed are in use. +// // Note: This is irreversible. // // Returns a negative error code on failure. int lfs_fs_grow(lfs_t *lfs, lfs_size_t block_count); #endif -#ifndef LFS_READONLY -// Shrinks the filesystem to a new size, updating the superblock with the new -// block count. -// -// Note: This first checks that none of the blocks that are being removed are in use -// and will fail if it is the case -// -// Returns a negative error code on failure. -int lfs_fs_shrink(lfs_t *lfs, lfs_size_t block_count); -#endif - #ifndef LFS_READONLY #ifdef LFS_MIGRATE // Attempts to migrate a previous version of littlefs diff --git a/tests/test_shrink.toml b/tests/test_shrink.toml index 0c317d66..10c00ae8 100644 --- a/tests/test_shrink.toml +++ b/tests/test_shrink.toml @@ -7,7 +7,7 @@ code = ''' lfs_t lfs; lfs_format(&lfs, cfg) => 0; lfs_mount(&lfs, cfg) => 0; - lfs_fs_shrink(&lfs, AFTER_BLOCK_COUNT) => 0; + lfs_fs_grow(&lfs, AFTER_BLOCK_COUNT) => 0; lfs_unmount(&lfs); if (BLOCK_COUNT != AFTER_BLOCK_COUNT) { lfs_mount(&lfs, cfg) => LFS_ERR_INVAL; @@ -46,7 +46,7 @@ code = ''' lfs_file_close(&lfs, &file) => 0; } - int err = lfs_fs_shrink(&lfs, AFTER_BLOCK_COUNT); + int err = lfs_fs_grow(&lfs, AFTER_BLOCK_COUNT); if (err == 0) { for (int i = 0; i < FILES_COUNT + 1; i++) { lfs_file_t file; diff --git a/tests/test_superblocks.toml b/tests/test_superblocks.toml index 07961751..4daf764b 100644 --- a/tests/test_superblocks.toml +++ b/tests/test_superblocks.toml @@ -550,7 +550,7 @@ code = ''' // same size is a noop lfs_mount(&lfs, cfg) => 0; - lfs_fs_shrink(&lfs, BLOCK_COUNT) => 0; + lfs_fs_grow(&lfs, BLOCK_COUNT) => 0; lfs_fs_stat(&lfs, &fsinfo) => 0; assert(fsinfo.block_size == BLOCK_SIZE); assert(fsinfo.block_count == BLOCK_COUNT); @@ -564,7 +564,7 @@ code = ''' // grow to new size lfs_mount(&lfs, cfg) => 0; - lfs_fs_shrink(&lfs, BLOCK_COUNT_2) => 0; + lfs_fs_grow(&lfs, BLOCK_COUNT_2) => 0; lfs_fs_stat(&lfs, &fsinfo) => 0; assert(fsinfo.block_size == BLOCK_SIZE); assert(fsinfo.block_count == BLOCK_COUNT_2); @@ -594,7 +594,7 @@ code = ''' // same size is a noop lfs_mount(&lfs, cfg) => 0; - lfs_fs_shrink(&lfs, BLOCK_COUNT_2) => 0; + lfs_fs_grow(&lfs, BLOCK_COUNT_2) => 0; lfs_fs_stat(&lfs, &fsinfo) => 0; assert(fsinfo.block_size == BLOCK_SIZE); assert(fsinfo.block_count == BLOCK_COUNT_2); From f4a1bb328ae1b85ce1b9b72985742073a1c89765 Mon Sep 17 00:00:00 2001 From: "selim.keles" Date: Mon, 5 May 2025 13:38:05 +0300 Subject: [PATCH 08/15] fix: added uint32_t cast to the bitshift places In 16 bit and 8 bit architectures, overflow and underflow issues were occuring while using functions lfs_frombe32 and lfs_fromle32 --- lfs_util.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lfs_util.h b/lfs_util.h index 0aec4885..0c3004bb 100644 --- a/lfs_util.h +++ b/lfs_util.h @@ -195,10 +195,10 @@ static inline uint32_t lfs_fromle32(uint32_t a) { (defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__)) return __builtin_bswap32(a); #else - return (((uint8_t*)&a)[0] << 0) | - (((uint8_t*)&a)[1] << 8) | - (((uint8_t*)&a)[2] << 16) | - (((uint8_t*)&a)[3] << 24); + return ((uint32_t)((uint8_t*)&a)[0] << 0) | + ((uint32_t)((uint8_t*)&a)[1] << 8) | + ((uint32_t)((uint8_t*)&a)[2] << 16) | + ((uint32_t)((uint8_t*)&a)[3] << 24); #endif } @@ -218,10 +218,10 @@ static inline uint32_t lfs_frombe32(uint32_t a) { (defined(__BYTE_ORDER__) && defined(__ORDER_BIG_ENDIAN__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) return a; #else - return (((uint8_t*)&a)[0] << 24) | - (((uint8_t*)&a)[1] << 16) | - (((uint8_t*)&a)[2] << 8) | - (((uint8_t*)&a)[3] << 0); + return ((uint32_t)((uint8_t*)&a)[0] << 24) | + ((uint32_t)((uint8_t*)&a)[1] << 16) | + ((uint32_t)((uint8_t*)&a)[2] << 8) | + ((uint32_t)((uint8_t*)&a)[3] << 0); #endif } From 7782d3dfa306b3dad917afb3a646cc1e72bba4a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Tue, 6 May 2025 10:59:32 +0200 Subject: [PATCH 09/15] Mention that shrinking is unlikely to work --- .github/workflows/test.yml | 16 ++++++++++++++++ lfs.h | 6 +++--- tests/test_shrink.toml | 5 +++++ tests/test_superblocks.toml | 2 ++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f3100bee..0815867c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -374,6 +374,22 @@ jobs: run: | CFLAGS="$CFLAGS -DLFS_NO_INTRINSICS" make test + test-shrink: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: install + run: | + # need a few things + sudo apt-get update -qq + sudo apt-get install -qq gcc python3 python3-pip + pip3 install toml + gcc --version + python3 --version + - name: test-no-intrinsics + run: | + CFLAGS="$CFLAGS -DLFS_SHRINKIFCHEAP" make test + # run with all trace options enabled to at least make sure these # all compile test-yes-trace: diff --git a/lfs.h b/lfs.h index be98b54b..a8e35eed 100644 --- a/lfs.h +++ b/lfs.h @@ -766,11 +766,11 @@ int lfs_fs_gc(lfs_t *lfs); // Grows the filesystem to a new size, updating the superblock with the new // block count. // -// if LFS_SHRINKIFCHEAP is defined, this function will also accept +// If LFS_SHRINKIFCHEAP is defined, this function will also accept // block_counts smaller than the current configuration, after checking // that none of the blocks that are being removed are in use. -// -// Note: This is irreversible. +// Note that littlefs's pseudorandom block allocation means that +// this is very unlikely to work in the general case. // // Returns a negative error code on failure. int lfs_fs_grow(lfs_t *lfs, lfs_size_t block_count); diff --git a/tests/test_shrink.toml b/tests/test_shrink.toml index 10c00ae8..47610485 100644 --- a/tests/test_shrink.toml +++ b/tests/test_shrink.toml @@ -2,8 +2,10 @@ [cases.test_shrink_simple] defines.BLOCK_COUNT = [10, 15, 20] defines.AFTER_BLOCK_COUNT = [5, 10, 15, 19] + if = "AFTER_BLOCK_COUNT <= BLOCK_COUNT" code = ''' +#ifdef LFS_SHRINKIFCHEAP lfs_t lfs; lfs_format(&lfs, cfg) => 0; lfs_mount(&lfs, cfg) => 0; @@ -18,6 +20,7 @@ code = ''' lfs2.cfg = &cfg2; lfs_mount(&lfs2, &cfg2) => 0; lfs_unmount(&lfs2) => 0; +#endif ''' # shrinking full @@ -27,6 +30,7 @@ defines.AFTER_BLOCK_COUNT = [5, 7, 10, 12, 15, 17, 20] defines.FILES_COUNT = [7, 8, 9, 10] if = "AFTER_BLOCK_COUNT <= BLOCK_COUNT && FILES_COUNT + 2 < BLOCK_COUNT" code = ''' +#ifdef LFS_SHRINKIFCHEAP lfs_t lfs; lfs_format(&lfs, cfg) => 0; // create FILES_COUNT files of BLOCK_SIZE - 50 bytes (to avoid inlining) @@ -101,4 +105,5 @@ code = ''' } lfs_unmount(&lfs2); } +#endif ''' diff --git a/tests/test_superblocks.toml b/tests/test_superblocks.toml index 4daf764b..188e4339 100644 --- a/tests/test_superblocks.toml +++ b/tests/test_superblocks.toml @@ -531,6 +531,7 @@ defines.BLOCK_COUNT = 'ERASE_COUNT' defines.BLOCK_COUNT_2 = ['ERASE_COUNT/2', 'ERASE_COUNT/4', '2'] defines.KNOWN_BLOCK_COUNT = [true, false] code = ''' +#ifdef LFS_SHRINKIFCHEAP lfs_t lfs; lfs_format(&lfs, cfg) => 0; @@ -628,6 +629,7 @@ code = ''' lfs_file_close(&lfs, &file) => 0; assert(memcmp(buffer, "hello!", 6) == 0); lfs_unmount(&lfs) => 0; +#endif ''' # test that metadata_max does not cause problems for superblock compaction From 7d79423972049b195ba26822e58e34d2d4cf5f97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 7 May 2025 10:32:03 +0200 Subject: [PATCH 10/15] Rename SHRINKIFCHEAP to SHRINKNONRELOCATING --- .github/workflows/test.yml | 2 +- lfs.c | 6 +++--- lfs.h | 2 +- tests/test_shrink.toml | 4 ++-- tests/test_superblocks.toml | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0815867c..a1af9049 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -388,7 +388,7 @@ jobs: python3 --version - name: test-no-intrinsics run: | - CFLAGS="$CFLAGS -DLFS_SHRINKIFCHEAP" make test + CFLAGS="$CFLAGS -DLFS_SHRINKNONRELOCATING" make test # run with all trace options enabled to at least make sure these # all compile diff --git a/lfs.c b/lfs.c index 3d128ff7..14b44001 100644 --- a/lfs.c +++ b/lfs.c @@ -5233,7 +5233,7 @@ static int lfs_fs_gc_(lfs_t *lfs) { #endif #ifndef LFS_READONLY -#ifdef LFS_SHRINKIFCHEAP +#ifdef LFS_SHRINKNONRELOCATING static int lfs_shrink_checkblock(void * data, lfs_block_t block) { lfs_size_t threshold = *((lfs_size_t *) data); if (block >= threshold) { @@ -5251,11 +5251,11 @@ static int lfs_fs_grow_(lfs_t *lfs, lfs_size_t block_count) { } -#ifndef LFS_SHRINKIFCHEAP +#ifndef LFS_SHRINKNONRELOCATING // shrinking is not supported LFS_ASSERT(block_count >= lfs->block_count); #endif -#ifdef LFS_SHRINKIFCHEAP +#ifdef LFS_SHRINKNONRELOCATING lfs_block_t threshold = block_count; err = lfs_fs_traverse_(lfs, lfs_shrink_checkblock, &threshold, true); if (err) { diff --git a/lfs.h b/lfs.h index a8e35eed..3968b5d7 100644 --- a/lfs.h +++ b/lfs.h @@ -766,7 +766,7 @@ int lfs_fs_gc(lfs_t *lfs); // Grows the filesystem to a new size, updating the superblock with the new // block count. // -// If LFS_SHRINKIFCHEAP is defined, this function will also accept +// If LFS_SHRINKNONRELOCATING is defined, this function will also accept // block_counts smaller than the current configuration, after checking // that none of the blocks that are being removed are in use. // Note that littlefs's pseudorandom block allocation means that diff --git a/tests/test_shrink.toml b/tests/test_shrink.toml index 47610485..6efa012c 100644 --- a/tests/test_shrink.toml +++ b/tests/test_shrink.toml @@ -5,7 +5,7 @@ defines.AFTER_BLOCK_COUNT = [5, 10, 15, 19] if = "AFTER_BLOCK_COUNT <= BLOCK_COUNT" code = ''' -#ifdef LFS_SHRINKIFCHEAP +#ifdef LFS_SHRINKNONRELOCATING lfs_t lfs; lfs_format(&lfs, cfg) => 0; lfs_mount(&lfs, cfg) => 0; @@ -30,7 +30,7 @@ defines.AFTER_BLOCK_COUNT = [5, 7, 10, 12, 15, 17, 20] defines.FILES_COUNT = [7, 8, 9, 10] if = "AFTER_BLOCK_COUNT <= BLOCK_COUNT && FILES_COUNT + 2 < BLOCK_COUNT" code = ''' -#ifdef LFS_SHRINKIFCHEAP +#ifdef LFS_SHRINKNONRELOCATING lfs_t lfs; lfs_format(&lfs, cfg) => 0; // create FILES_COUNT files of BLOCK_SIZE - 50 bytes (to avoid inlining) diff --git a/tests/test_superblocks.toml b/tests/test_superblocks.toml index 188e4339..78050f13 100644 --- a/tests/test_superblocks.toml +++ b/tests/test_superblocks.toml @@ -531,7 +531,7 @@ defines.BLOCK_COUNT = 'ERASE_COUNT' defines.BLOCK_COUNT_2 = ['ERASE_COUNT/2', 'ERASE_COUNT/4', '2'] defines.KNOWN_BLOCK_COUNT = [true, false] code = ''' -#ifdef LFS_SHRINKIFCHEAP +#ifdef LFS_SHRINKNONRELOCATING lfs_t lfs; lfs_format(&lfs, cfg) => 0; From edaaaf88ead8d0828fa79f45b9c0d55662504da1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sosth=C3=A8ne=20Gu=C3=A9don?= Date: Wed, 7 May 2025 10:38:43 +0200 Subject: [PATCH 11/15] Apply review comments --- lfs.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lfs.c b/lfs.c index 14b44001..fcb20c77 100644 --- a/lfs.c +++ b/lfs.c @@ -5234,8 +5234,8 @@ static int lfs_fs_gc_(lfs_t *lfs) { #ifndef LFS_READONLY #ifdef LFS_SHRINKNONRELOCATING -static int lfs_shrink_checkblock(void * data, lfs_block_t block) { - lfs_size_t threshold = *((lfs_size_t *) data); +static int lfs_shrink_checkblock(void *data, lfs_block_t block) { + lfs_size_t threshold = *((lfs_size_t*)data); if (block >= threshold) { return LFS_ERR_NOTEMPTY; } @@ -5256,10 +5256,11 @@ static int lfs_fs_grow_(lfs_t *lfs, lfs_size_t block_count) { LFS_ASSERT(block_count >= lfs->block_count); #endif #ifdef LFS_SHRINKNONRELOCATING - lfs_block_t threshold = block_count; - err = lfs_fs_traverse_(lfs, lfs_shrink_checkblock, &threshold, true); - if (err) { - return err; + if (block_count < lfs->block_count) { + err = lfs_fs_traverse_(lfs, lfs_shrink_checkblock, &block_count, true); + if (err) { + return err; + } } #endif From bff4dfd1b1f6f2ee53bd4aae6de5d3b66c172251 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 7 May 2025 22:46:31 -0500 Subject: [PATCH 12/15] Added NO_GCC to allow users to explicitly disable GCC-specific flags This is the same as the implicit Clang => NO_GCC behavior introduced by yamt, but with an explicit variable that can be assigned by users using other, non-gcc, compilers: $ NO_GCC=1 make Note, stack measurements are currently GCC specific: $ NO_GCC=1 make stack ... snip ... FileNotFoundError: [Errno 2] No such file or directory: 'lfs.ci' make: *** [Makefile:494: lfs.stack.csv] Error 1 --- Makefile | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 1fd856f9..588e095d 100644 --- a/Makefile +++ b/Makefile @@ -18,6 +18,12 @@ VALGRIND ?= valgrind GDB ?= gdb PERF ?= perf +# guess clang or gcc (clang sometimes masquerades as gcc because of +# course it does) +ifneq ($(shell $(CC) --version | grep clang),) +NO_GCC = 1 +endif + SRC ?= $(filter-out $(wildcard *.t.* *.b.*),$(wildcard *.c)) OBJ := $(SRC:%.c=$(BUILDDIR)/%.o) DEP := $(SRC:%.c=$(BUILDDIR)/%.d) @@ -63,9 +69,9 @@ CFLAGS += -g3 CFLAGS += -I. CFLAGS += -std=c99 -Wall -Wextra -pedantic CFLAGS += -Wmissing-prototypes -ifeq ($(shell $(CC) --version | grep clang),) -CFLAGS += -ftrack-macro-expansion=0 +ifndef NO_GCC CFLAGS += -fcallgraph-info=su +CFLAGS += -ftrack-macro-expansion=0 endif ifdef DEBUG From 0115cf6b74cc27e288678aeb7f8c977be0792be0 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 7 May 2025 23:45:29 -0500 Subject: [PATCH 13/15] gha: Dropped explicit CFLAGS from clang testing in CI Thanks to yamt, GCC-specific flags should now be disabled if compiling with clang. Dropping the explicit flags also doubles as a test that the NO_GCC inference works. --- .github/workflows/test.yml | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f3100bee..f18c23d4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -454,8 +454,7 @@ jobs: TESTFLAGS="$TESTFLAGS --valgrind --context=1024 -Gdefault -Pnone" \ make test - # test that compilation is warning free under clang - # run with Clang, mostly to check for Clang-specific warnings + # compile/run with Clang, mostly to check for Clang-specific warnings test-clang: runs-on: ubuntu-latest steps: @@ -469,12 +468,8 @@ jobs: python3 --version - name: test-clang run: | - # override CFLAGS since Clang does not support -fcallgraph-info - # and -ftrack-macro-expansions - make \ - CC=clang \ - CFLAGS="$CFLAGS -MMD -g3 -I. -std=c99 -Wall -Wextra -pedantic" \ - test + CC=clang \ + make test # run benchmarks # From ba250a30751d5568ff1d690987d6c77e9eec60d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20de=20Giessen?= Date: Tue, 13 May 2025 13:12:42 +0200 Subject: [PATCH 14/15] use shutil.move instead of os.rename to move file This prevents a "OSError: [Errno 18] Invalid cross-device link" if the temporary file was created on different filesystem (such as a tmpfs mount). --- scripts/changeprefix.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/changeprefix.py b/scripts/changeprefix.py index 381a4568..1ecc2e43 100755 --- a/scripts/changeprefix.py +++ b/scripts/changeprefix.py @@ -73,7 +73,7 @@ def changefile(from_prefix, to_prefix, from_path, to_path, *, shutil.copystat(from_path, to_path) if to_path_temp: - os.rename(to_path, from_path) + shutil.move(to_path, from_path) elif from_path != '-': os.remove(from_path) From 8434536f0abe7ac6dae491f9a781d98491f77b71 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 13 May 2025 13:18:31 -0500 Subject: [PATCH 15/15] Bumped minor version to v2.11 --- lfs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lfs.h b/lfs.h index 3968b5d7..215309c5 100644 --- a/lfs.h +++ b/lfs.h @@ -21,7 +21,7 @@ extern "C" // Software library version // Major (top-nibble), incremented on backwards incompatible changes // Minor (bottom-nibble), incremented on feature additions -#define LFS_VERSION 0x0002000a +#define LFS_VERSION 0x0002000b #define LFS_VERSION_MAJOR (0xffff & (LFS_VERSION >> 16)) #define LFS_VERSION_MINOR (0xffff & (LFS_VERSION >> 0))