From b6373792106f04d8de6b959be61062b89d5b793a Mon Sep 17 00:00:00 2001 From: ondrap Date: Wed, 2 Aug 2023 11:51:52 +0200 Subject: [PATCH 1/5] Update lfs_find_free_blocks to match the latest changes. --- lfs.c | 27 +++++++++++++++++---------- lfs.h | 4 ++++ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/lfs.c b/lfs.c index fbd0ca60..250e7346 100644 --- a/lfs.c +++ b/lfs.c @@ -654,20 +654,27 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { return LFS_ERR_NOSPC; } - lfs->free.off = (lfs->free.off + lfs->free.size) - % lfs->block_count; - lfs->free.size = lfs_min(8*lfs->cfg->lookahead_size, lfs->free.ack); - lfs->free.i = 0; - - // find mask of free blocks from tree - memset(lfs->free.buffer, 0, lfs->cfg->lookahead_size); - int err = lfs_fs_rawtraverse(lfs, lfs_alloc_lookahead, lfs, true); - if (err) { - lfs_alloc_drop(lfs); + int err = lfs_find_free_blocks(lfs); + if(err) { return err; } } } + +int lfs_find_free_blocks(lfs_t *lfs){ + lfs->free.off = (lfs->free.off + lfs->free.size) + % lfs->block_count; + lfs->free.size = lfs_min(8*lfs->cfg->lookahead_size, lfs->free.ack); + lfs->free.i = 0; + + // find mask of free blocks from tree + memset(lfs->free.buffer, 0, lfs->cfg->lookahead_size); + int const err = lfs_fs_rawtraverse(lfs, lfs_alloc_lookahead, lfs, true); + if (err) { + lfs_alloc_drop(lfs); + } + return err; +} #endif /// Metadata pair and directory operations /// diff --git a/lfs.h b/lfs.h index 291dbb51..f574b317 100644 --- a/lfs.h +++ b/lfs.h @@ -712,6 +712,10 @@ lfs_ssize_t lfs_fs_size(lfs_t *lfs); // Returns a negative error code on failure. int lfs_fs_traverse(lfs_t *lfs, int (*cb)(void*, lfs_block_t), void *data); +// Use Traverse function and try to find free blocks. LittleFS free blocks search is unpredictable. +// Search is costly operation which may delay write. In realtime write scenarios can be better to find them before a write. +int lfs_find_free_blocks(lfs_t *lfs); + #ifndef LFS_READONLY // Attempt to make the filesystem consistent and ready for writing // From d85a0fe2e2f1a2c48d80b05a29c3c60a28c7e3b0 Mon Sep 17 00:00:00 2001 From: ondrap Date: Tue, 29 Aug 2023 12:50:16 +0200 Subject: [PATCH 2/5] Move lookahead buffer offset at the first free block if such block doesn't exist move it for whole lookahead size. --- lfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lfs.c b/lfs.c index 250e7346..3664e801 100644 --- a/lfs.c +++ b/lfs.c @@ -662,7 +662,9 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { } int lfs_find_free_blocks(lfs_t *lfs){ - lfs->free.off = (lfs->free.off + lfs->free.size) + // Move free offset at the first unused block (lfs->free.i) + // lfs->free.i is equal lfs->free.size when all blocks are used + lfs->free.off = (lfs->free.off + lfs->free.i) % lfs->block_count; lfs->free.size = lfs_min(8*lfs->cfg->lookahead_size, lfs->free.ack); lfs->free.i = 0; From dbe4598c12a9b1e6cab2f8dc78158755b64a1e6d Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 11 Sep 2023 23:42:37 -0500 Subject: [PATCH 3/5] Added API boilerplate for lfs_fs_findfreeblocks and consistent style This adds the tracing and optional locking for the littlefs API. Also updated to match the code style, and added LFS_READONLY guards where necessary. --- lfs.c | 55 +++++++++++++++++++++++++++++++++++++------------------ lfs.h | 9 ++++++--- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/lfs.c b/lfs.c index 3664e801..dafe0805 100644 --- a/lfs.c +++ b/lfs.c @@ -622,6 +622,26 @@ static void lfs_alloc_drop(lfs_t *lfs) { lfs_alloc_ack(lfs); } +#ifndef LFS_READONLY +static int lfs_fs_rawfindfreeblocks(lfs_t *lfs) { + // Move free offset at the first unused block (lfs->free.i) + // lfs->free.i is equal lfs->free.size when all blocks are used + lfs->free.off = (lfs->free.off + lfs->free.i) % lfs->block_count; + lfs->free.size = lfs_min(8*lfs->cfg->lookahead_size, lfs->free.ack); + lfs->free.i = 0; + + // find mask of free blocks from tree + memset(lfs->free.buffer, 0, lfs->cfg->lookahead_size); + int err = lfs_fs_rawtraverse(lfs, lfs_alloc_lookahead, lfs, true); + if (err) { + lfs_alloc_drop(lfs); + return err; + } + + return 0; +} +#endif + #ifndef LFS_READONLY static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { while (true) { @@ -654,29 +674,12 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { return LFS_ERR_NOSPC; } - int err = lfs_find_free_blocks(lfs); + int err = lfs_fs_rawfindfreeblocks(lfs); if(err) { return err; } } } - -int lfs_find_free_blocks(lfs_t *lfs){ - // Move free offset at the first unused block (lfs->free.i) - // lfs->free.i is equal lfs->free.size when all blocks are used - lfs->free.off = (lfs->free.off + lfs->free.i) - % lfs->block_count; - lfs->free.size = lfs_min(8*lfs->cfg->lookahead_size, lfs->free.ack); - lfs->free.i = 0; - - // find mask of free blocks from tree - memset(lfs->free.buffer, 0, lfs->cfg->lookahead_size); - int const err = lfs_fs_rawtraverse(lfs, lfs_alloc_lookahead, lfs, true); - if (err) { - lfs_alloc_drop(lfs); - } - return err; -} #endif /// Metadata pair and directory operations /// @@ -6247,6 +6250,22 @@ int lfs_fs_traverse(lfs_t *lfs, int (*cb)(void *, lfs_block_t), void *data) { return err; } +#ifndef LFS_READONLY +int lfs_fs_findfreeblocks(lfs_t *lfs) { + int err = LFS_LOCK(lfs->cfg); + if (err) { + return err; + } + LFS_TRACE("lfs_fs_findfreeblocks(%p)", (void*)lfs); + + err = lfs_fs_rawfindfreeblocks(lfs); + + LFS_TRACE("lfs_fs_findfreeblocks -> %d", err); + LFS_UNLOCK(lfs->cfg); + return err; +} +#endif + #ifndef LFS_READONLY int lfs_fs_mkconsistent(lfs_t *lfs) { int err = LFS_LOCK(lfs->cfg); diff --git a/lfs.h b/lfs.h index f574b317..a0dce9d8 100644 --- a/lfs.h +++ b/lfs.h @@ -712,9 +712,12 @@ lfs_ssize_t lfs_fs_size(lfs_t *lfs); // Returns a negative error code on failure. int lfs_fs_traverse(lfs_t *lfs, int (*cb)(void*, lfs_block_t), void *data); -// Use Traverse function and try to find free blocks. LittleFS free blocks search is unpredictable. -// Search is costly operation which may delay write. In realtime write scenarios can be better to find them before a write. -int lfs_find_free_blocks(lfs_t *lfs); +// Use Traverse function and try to find free blocks. LittleFS free blocks +// search is unpredictable. +// +// Search is costly operation which may delay write. In realtime write +// scenarios can be better to find them before a write. +int lfs_fs_findfreeblocks(lfs_t *lfs); #ifndef LFS_READONLY // Attempt to make the filesystem consistent and ready for writing From 63e4408f2af1464df5b9c574661e923dc8be2386 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 11 Sep 2023 23:56:49 -0500 Subject: [PATCH 4/5] Extended alloc tests to test some properties of lfs_fs_findfreeblocks - 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. --- tests/test_alloc.toml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_alloc.toml b/tests/test_alloc.toml index 916fc2a5..50baa7d0 100644 --- a/tests/test_alloc.toml +++ b/tests/test_alloc.toml @@ -6,6 +6,7 @@ if = 'BLOCK_CYCLES == -1' [cases.test_alloc_parallel] defines.FILES = 3 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)' +defines.GC = [false, true] code = ''' const char *names[] = {"bacon", "eggs", "pancakes"}; lfs_file_t files[FILES]; @@ -24,6 +25,9 @@ code = ''' LFS_O_WRONLY | LFS_O_CREAT | LFS_O_APPEND) => 0; } for (int n = 0; n < FILES; n++) { + if (GC) { + lfs_fs_findfreeblocks(&lfs) => 0; + } size_t size = strlen(names[n]); for (lfs_size_t i = 0; i < SIZE; i += size) { lfs_file_write(&lfs, &files[n], names[n], size) => size; @@ -55,6 +59,7 @@ code = ''' [cases.test_alloc_serial] defines.FILES = 3 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)' +defines.GC = [false, true] code = ''' const char *names[] = {"bacon", "eggs", "pancakes"}; @@ -75,6 +80,9 @@ code = ''' uint8_t buffer[1024]; memcpy(buffer, names[n], size); for (int i = 0; i < SIZE; i += size) { + if (GC) { + lfs_fs_findfreeblocks(&lfs) => 0; + } lfs_file_write(&lfs, &file, buffer, size) => size; } lfs_file_close(&lfs, &file) => 0; @@ -247,6 +255,9 @@ code = ''' } res => LFS_ERR_NOSPC; + // note that lfs_fs_findfreeblocks should not error here + lfs_fs_findfreeblocks(&lfs) => 0; + lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0; @@ -298,6 +309,9 @@ code = ''' } res => LFS_ERR_NOSPC; + // note that lfs_fs_findfreeblocks should not error here + lfs_fs_findfreeblocks(&lfs) => 0; + lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0; @@ -337,6 +351,8 @@ code = ''' count += 1; } err => LFS_ERR_NOSPC; + // note that lfs_fs_findfreeblocks should not error here + lfs_fs_findfreeblocks(&lfs) => 0; lfs_file_close(&lfs, &file) => 0; lfs_remove(&lfs, "exhaustion") => 0; @@ -435,6 +451,8 @@ code = ''' break; } } + // note that lfs_fs_findfreeblocks should not error here + lfs_fs_findfreeblocks(&lfs) => 0; lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0; From 6b33ee5e34a29508e928dce2e6f32d3c6936131a Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 12 Sep 2023 00:06:04 -0500 Subject: [PATCH 5/5] Renamed lfs_fs_findfreeblocks -> lfs_fs_gc, tweaked documentation 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. --- lfs.c | 12 ++++++------ lfs.h | 15 ++++++++++----- tests/test_alloc.toml | 20 ++++++++++---------- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lfs.c b/lfs.c index dafe0805..0827331c 100644 --- a/lfs.c +++ b/lfs.c @@ -623,7 +623,7 @@ static void lfs_alloc_drop(lfs_t *lfs) { } #ifndef LFS_READONLY -static int lfs_fs_rawfindfreeblocks(lfs_t *lfs) { +static int lfs_fs_rawgc(lfs_t *lfs) { // Move free offset at the first unused block (lfs->free.i) // lfs->free.i is equal lfs->free.size when all blocks are used lfs->free.off = (lfs->free.off + lfs->free.i) % lfs->block_count; @@ -674,7 +674,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { return LFS_ERR_NOSPC; } - int err = lfs_fs_rawfindfreeblocks(lfs); + int err = lfs_fs_rawgc(lfs); if(err) { return err; } @@ -6251,16 +6251,16 @@ int lfs_fs_traverse(lfs_t *lfs, int (*cb)(void *, lfs_block_t), void *data) { } #ifndef LFS_READONLY -int lfs_fs_findfreeblocks(lfs_t *lfs) { +int lfs_fs_gc(lfs_t *lfs) { int err = LFS_LOCK(lfs->cfg); if (err) { return err; } - LFS_TRACE("lfs_fs_findfreeblocks(%p)", (void*)lfs); + LFS_TRACE("lfs_fs_gc(%p)", (void*)lfs); - err = lfs_fs_rawfindfreeblocks(lfs); + err = lfs_fs_rawgc(lfs); - LFS_TRACE("lfs_fs_findfreeblocks -> %d", err); + LFS_TRACE("lfs_fs_gc -> %d", err); LFS_UNLOCK(lfs->cfg); return err; } diff --git a/lfs.h b/lfs.h index a0dce9d8..6535eedb 100644 --- a/lfs.h +++ b/lfs.h @@ -712,12 +712,17 @@ lfs_ssize_t lfs_fs_size(lfs_t *lfs); // Returns a negative error code on failure. int lfs_fs_traverse(lfs_t *lfs, int (*cb)(void*, lfs_block_t), void *data); -// Use Traverse function and try to find free blocks. LittleFS free blocks -// search is unpredictable. +// Attempt to proactively find free blocks // -// Search is costly operation which may delay write. In realtime write -// scenarios can be better to find them before a write. -int lfs_fs_findfreeblocks(lfs_t *lfs); +// Calling this function is not required, but may allowing the offloading of +// the expensive block allocation scan to a less time-critical code path. +// +// Note: littlefs currently does not persist any found free blocks to disk. +// This may change in the future. +// +// Returns a negative error code on failure. Finding no free blocks is +// not an error. +int lfs_fs_gc(lfs_t *lfs); #ifndef LFS_READONLY // Attempt to make the filesystem consistent and ready for writing diff --git a/tests/test_alloc.toml b/tests/test_alloc.toml index 50baa7d0..e6fba975 100644 --- a/tests/test_alloc.toml +++ b/tests/test_alloc.toml @@ -26,7 +26,7 @@ code = ''' } for (int n = 0; n < FILES; n++) { if (GC) { - lfs_fs_findfreeblocks(&lfs) => 0; + lfs_fs_gc(&lfs) => 0; } size_t size = strlen(names[n]); for (lfs_size_t i = 0; i < SIZE; i += size) { @@ -81,7 +81,7 @@ code = ''' memcpy(buffer, names[n], size); for (int i = 0; i < SIZE; i += size) { if (GC) { - lfs_fs_findfreeblocks(&lfs) => 0; + lfs_fs_gc(&lfs) => 0; } lfs_file_write(&lfs, &file, buffer, size) => size; } @@ -255,8 +255,8 @@ code = ''' } res => LFS_ERR_NOSPC; - // note that lfs_fs_findfreeblocks should not error here - lfs_fs_findfreeblocks(&lfs) => 0; + // note that lfs_fs_gc should not error here + lfs_fs_gc(&lfs) => 0; lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0; @@ -309,8 +309,8 @@ code = ''' } res => LFS_ERR_NOSPC; - // note that lfs_fs_findfreeblocks should not error here - lfs_fs_findfreeblocks(&lfs) => 0; + // note that lfs_fs_gc should not error here + lfs_fs_gc(&lfs) => 0; lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0; @@ -351,8 +351,8 @@ code = ''' count += 1; } err => LFS_ERR_NOSPC; - // note that lfs_fs_findfreeblocks should not error here - lfs_fs_findfreeblocks(&lfs) => 0; + // note that lfs_fs_gc should not error here + lfs_fs_gc(&lfs) => 0; lfs_file_close(&lfs, &file) => 0; lfs_remove(&lfs, "exhaustion") => 0; @@ -451,8 +451,8 @@ code = ''' break; } } - // note that lfs_fs_findfreeblocks should not error here - lfs_fs_findfreeblocks(&lfs) => 0; + // note that lfs_fs_gc should not error here + lfs_fs_gc(&lfs) => 0; lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0;