From 6b16dafb4db903fc1f8509d4170af56ae70ee56e Mon Sep 17 00:00:00 2001 From: Will Date: Tue, 15 Dec 2020 12:59:32 +1000 Subject: [PATCH 01/11] Add metadata_max and inline_file_max to config We have seen poor read performance on NAND flashes with 128kB blocks. The root cause is inline files having to traverse many sets of metadata pairs inside the current block before being fully reconstructed. Simply disabling inline files is not enough, as the metadata will still fill up the block and eventually need to be compacted. By allowing configuration of how much size metadata takes up, along with limiting (or disabling) inline file size, we achieve read performance improvements on an order of magnitude. --- lfs.c | 24 +++++++++++++++++++----- lfs.h | 18 +++++++++++++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/lfs.c b/lfs.c index d7439fe3..5585b40b 100644 --- a/lfs.c +++ b/lfs.c @@ -1589,7 +1589,7 @@ static int lfs_dir_compact(lfs_t *lfs, // for metadata updates. if (end - begin < 0xff && size <= lfs_min(lfs->cfg->block_size - 36, - lfs_alignup(lfs->cfg->block_size/2, + lfs_alignup(lfs->metadata_max/2, lfs->cfg->prog_size))) { break; } @@ -1674,7 +1674,7 @@ static int lfs_dir_compact(lfs_t *lfs, .crc = 0xffffffff, .begin = 0, - .end = lfs->cfg->block_size - 8, + .end = lfs->metadata_max - 8, }; // erase block to write to @@ -1884,7 +1884,7 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, .crc = 0xffffffff, .begin = dir->off, - .end = lfs->cfg->block_size - 8, + .end = lfs->metadata_max - 8, }; // traverse attrs that need to be written out @@ -2966,7 +2966,7 @@ static lfs_ssize_t lfs_file_rawwrite(lfs_t *lfs, lfs_file_t *file, if ((file->flags & LFS_F_INLINE) && lfs_max(file->pos+nsize, file->ctz.size) > lfs_min(0x3fe, lfs_min( - lfs->cfg->cache_size, lfs->cfg->block_size/8))) { + lfs->cfg->cache_size, lfs->inline_file_max))) { // inline file doesn't fit anymore int err = lfs_file_outline(lfs, file); if (err) { @@ -3536,6 +3536,20 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) { lfs->attr_max = LFS_ATTR_MAX; } + LFS_ASSERT(lfs->cfg->metadata_max <= lfs->cfg->block_size); + lfs->metadata_max = lfs->cfg->metadata_max; + if (!lfs->metadata_max) { + lfs->metadata_max = lfs->cfg->block_size; + } + + LFS_ASSERT(lfs->cfg->inline_file_max <= LFS_FILE_MAX); + lfs->inline_file_max = lfs->cfg->inline_file_max; + if (!lfs->inline_file_max) { + lfs->inline_file_max = lfs->cfg->block_size / 8; + } else if(lfs->inline_file_max == -1) { + lfs->inline_file_max = 0; + } + // setup default state lfs->root[0] = LFS_BLOCK_NULL; lfs->root[1] = LFS_BLOCK_NULL; @@ -3829,7 +3843,7 @@ int lfs_fs_rawtraverse(lfs_t *lfs, if (err) { return err; } - } else if (includeorphans && + } else if (includeorphans && lfs_tag_type3(tag) == LFS_TYPE_DIRSTRUCT) { for (int i = 0; i < 2; i++) { err = cb(data, (&ctz.head)[i]); diff --git a/lfs.h b/lfs.h index 3b02b6a7..8a4b1d0c 100644 --- a/lfs.h +++ b/lfs.h @@ -207,7 +207,7 @@ struct lfs_config { // Number of erasable blocks on the device. lfs_size_t block_count; - // Number of erase cycles before littlefs evicts metadata logs and moves + // Number of erase cycles before littlefs evicts metadata logs and moves // the metadata to another block. Suggested values are in the // range 100-1000, with large values having better performance at the cost // of less consistent wear distribution. @@ -256,6 +256,20 @@ struct lfs_config { // larger attributes size but must be <= LFS_ATTR_MAX. Defaults to // LFS_ATTR_MAX when zero. lfs_size_t attr_max; + + // Optional upper limit on total space given to metadata pairs in bytes. On + // devices with large blocks (e.g. 128kB) setting this to a low size (2-8kB) + // can help bound the metadata compaction time. Must be <= block_size. + // Defaults to block_size when zero. + lfs_size_t metadata_max; + + // Optional upper limit on inline files in bytes. On devices with large + // blocks (e.g. 128kB) setting this to a low size or disabling inline files + // can help bound file read overhead. Must be <= LFS_FILE_MAX. Defaults to + // block_size/8 when zero. + // + // Set to -1 to disable inline files. + lfs_ssize_t inline_file_max; }; // File info structure @@ -406,6 +420,8 @@ typedef struct lfs { lfs_size_t name_max; lfs_size_t file_max; lfs_size_t attr_max; + lfs_size_t metadata_max; + lfs_ssize_t inline_file_max; #ifdef LFS_MIGRATE struct lfs1 *lfs1; From 37f4de297672ee3ec05103d60e09bb7748c5e5b3 Mon Sep 17 00:00:00 2001 From: Will Date: Fri, 18 Dec 2020 13:05:20 +1000 Subject: [PATCH 02/11] Remove inline_files_max and lfs_t entry for metadata_max --- lfs.c | 25 +++++++++---------------- lfs.h | 10 ---------- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/lfs.c b/lfs.c index 5585b40b..50fb9a47 100644 --- a/lfs.c +++ b/lfs.c @@ -1589,7 +1589,8 @@ static int lfs_dir_compact(lfs_t *lfs, // for metadata updates. if (end - begin < 0xff && size <= lfs_min(lfs->cfg->block_size - 36, - lfs_alignup(lfs->metadata_max/2, + lfs_alignup((lfs->cfg->metadata_max ? + lfs->cfg->metadata_max : lfs->cfg->block_size)/2, lfs->cfg->prog_size))) { break; } @@ -1674,7 +1675,8 @@ static int lfs_dir_compact(lfs_t *lfs, .crc = 0xffffffff, .begin = 0, - .end = lfs->metadata_max - 8, + .end = (lfs->cfg->metadata_max ? + lfs->cfg->metadata_max : lfs->cfg->block_size) - 8, }; // erase block to write to @@ -1884,7 +1886,8 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, .crc = 0xffffffff, .begin = dir->off, - .end = lfs->metadata_max - 8, + .end = (lfs->cfg->metadata_max ? + lfs->cfg->metadata_max : lfs->cfg->block_size) - 8, }; // traverse attrs that need to be written out @@ -2966,7 +2969,9 @@ static lfs_ssize_t lfs_file_rawwrite(lfs_t *lfs, lfs_file_t *file, if ((file->flags & LFS_F_INLINE) && lfs_max(file->pos+nsize, file->ctz.size) > lfs_min(0x3fe, lfs_min( - lfs->cfg->cache_size, lfs->inline_file_max))) { + lfs->cfg->cache_size, + (lfs->cfg->metadata_max ? + lfs->cfg->metadata_max : lfs->cfg->block_size) / 8))) { // inline file doesn't fit anymore int err = lfs_file_outline(lfs, file); if (err) { @@ -3537,18 +3542,6 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) { } LFS_ASSERT(lfs->cfg->metadata_max <= lfs->cfg->block_size); - lfs->metadata_max = lfs->cfg->metadata_max; - if (!lfs->metadata_max) { - lfs->metadata_max = lfs->cfg->block_size; - } - - LFS_ASSERT(lfs->cfg->inline_file_max <= LFS_FILE_MAX); - lfs->inline_file_max = lfs->cfg->inline_file_max; - if (!lfs->inline_file_max) { - lfs->inline_file_max = lfs->cfg->block_size / 8; - } else if(lfs->inline_file_max == -1) { - lfs->inline_file_max = 0; - } // setup default state lfs->root[0] = LFS_BLOCK_NULL; diff --git a/lfs.h b/lfs.h index 8a4b1d0c..c7ec6d3e 100644 --- a/lfs.h +++ b/lfs.h @@ -262,14 +262,6 @@ struct lfs_config { // can help bound the metadata compaction time. Must be <= block_size. // Defaults to block_size when zero. lfs_size_t metadata_max; - - // Optional upper limit on inline files in bytes. On devices with large - // blocks (e.g. 128kB) setting this to a low size or disabling inline files - // can help bound file read overhead. Must be <= LFS_FILE_MAX. Defaults to - // block_size/8 when zero. - // - // Set to -1 to disable inline files. - lfs_ssize_t inline_file_max; }; // File info structure @@ -420,8 +412,6 @@ typedef struct lfs { lfs_size_t name_max; lfs_size_t file_max; lfs_size_t attr_max; - lfs_size_t metadata_max; - lfs_ssize_t inline_file_max; #ifdef LFS_MIGRATE struct lfs1 *lfs1; From 2b804537b08dcb052a23d28fa4cb0805dca34ffc Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 22 Dec 2020 00:06:51 -0600 Subject: [PATCH 03/11] Moved sanity check in lfs_format after compaction After a bit of tweaking in 9dde5c7 to write out all superblocks during lfs_format, additional writes were added after the sanity checking normally done at the end. This turned out to be a problem when porting littlefs, as it makes it easy for addressing issues to not get caught during lfs_format. Found by marekr, tristanclare94, and mjs513 --- lfs.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lfs.c b/lfs.c index d7439fe3..96c2d6ee 100644 --- a/lfs.c +++ b/lfs.c @@ -3616,12 +3616,6 @@ static int lfs_rawformat(lfs_t *lfs, const struct lfs_config *cfg) { goto cleanup; } - // sanity check that fetch works - err = lfs_dir_fetch(lfs, &root, (const lfs_block_t[2]){0, 1}); - if (err) { - goto cleanup; - } - // force compaction to prevent accidentally mounting any // older version of littlefs that may live on disk root.erased = false; @@ -3629,6 +3623,12 @@ static int lfs_rawformat(lfs_t *lfs, const struct lfs_config *cfg) { if (err) { goto cleanup; } + + // sanity check that fetch works + err = lfs_dir_fetch(lfs, &root, (const lfs_block_t[2]){0, 1}); + if (err) { + goto cleanup; + } } cleanup: From 6bb404315405ba6f5217c061c0e3fb2a0d19099b Mon Sep 17 00:00:00 2001 From: Themba Dube Date: Thu, 24 Dec 2020 14:05:46 -0500 Subject: [PATCH 04/11] Skip flushing file if lfs_file_rawseek() doesn't change position --- lfs.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lfs.c b/lfs.c index d7439fe3..c59d3d27 100644 --- a/lfs.c +++ b/lfs.c @@ -3048,14 +3048,6 @@ relocate: static lfs_soff_t lfs_file_rawseek(lfs_t *lfs, lfs_file_t *file, lfs_soff_t off, int whence) { -#ifndef LFS_READONLY - // write out everything beforehand, may be noop if rdonly - int err = lfs_file_flush(lfs, file); - if (err) { - return err; - } -#endif - // find new pos lfs_off_t npos = file->pos; if (whence == LFS_SEEK_SET) { @@ -3071,6 +3063,19 @@ static lfs_soff_t lfs_file_rawseek(lfs_t *lfs, lfs_file_t *file, return LFS_ERR_INVAL; } + if (file->pos == npos) { + // noop - position has not changed + return npos; + } + +#ifndef LFS_READONLY + // write out everything beforehand, may be noop if rdonly + int err = lfs_file_flush(lfs, file); + if (err) { + return err; + } +#endif + // update pos file->pos = npos; return npos; From 3216b07c3bce220115ea8c5c8b3eb1e452bf6de0 Mon Sep 17 00:00:00 2001 From: Themba Dube Date: Wed, 6 Jan 2021 11:20:41 -0500 Subject: [PATCH 05/11] Use lfs_file_rawsize to calculate LFS_SEEK_END position --- lfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lfs.c b/lfs.c index c59d3d27..281f1386 100644 --- a/lfs.c +++ b/lfs.c @@ -3055,7 +3055,7 @@ static lfs_soff_t lfs_file_rawseek(lfs_t *lfs, lfs_file_t *file, } else if (whence == LFS_SEEK_CUR) { npos = file->pos + off; } else if (whence == LFS_SEEK_END) { - npos = file->ctz.size + off; + npos = lfs_file_rawsize(lfs, file) + off; } if (npos > lfs->file_max) { From 745d98cde08850cd0322daed86e5584a891095eb Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 11 Jan 2021 00:01:05 -0600 Subject: [PATCH 06/11] Fixed lfs_file_truncate issue where internal state may not be flushed This was caused by the new lfs_file_rawseek optimization that can skip flushing when calculated file->pos is unchanged combined with an implicit expectation in lfs_file_truncate that lfs_file_rawseek unconditionally sets file->pos. Because of this assumption, lfs_file_truncate could leave file->pos in an outdated state while changing the internal file metadata. Humorously, this was always gauranteed to trigger the skip in lfs_file_rawseek when we try to restore the file->pos, leaving the file->cache used to do the CTZ skip-list lookup in a potentially bad state. The easiest fix is to just update file->pos correctly. Note we don't want to explicitly flush since we can leverage the same noop optimization if we truncate to the file position. Which I've added a test for. --- lfs.c | 3 +++ tests/test_truncate.toml | 45 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/lfs.c b/lfs.c index 281f1386..f54aa049 100644 --- a/lfs.c +++ b/lfs.c @@ -3106,6 +3106,9 @@ static int lfs_file_rawtruncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) { return err; } + // need to set pos/block/off consistently so seeking back to + // the old position does not get confused + file->pos = size; file->ctz.head = file->block; file->ctz.size = size; file->flags |= LFS_F_DIRTY | LFS_F_READING; diff --git a/tests/test_truncate.toml b/tests/test_truncate.toml index c11285b7..850d7aae 100644 --- a/tests/test_truncate.toml +++ b/tests/test_truncate.toml @@ -392,3 +392,48 @@ code = ''' lfs_unmount(&lfs) => 0; ''' + +[[case]] # noop truncate +define.MEDIUMSIZE = [32, 2048] +code = ''' + lfs_format(&lfs, &cfg) => 0; + lfs_mount(&lfs, &cfg) => 0; + lfs_file_open(&lfs, &file, "baldynoop", + LFS_O_RDWR | LFS_O_CREAT) => 0; + + strcpy((char*)buffer, "hair"); + size = strlen((char*)buffer); + for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) { + lfs_file_write(&lfs, &file, buffer, size) => size; + + // this truncate should do nothing + lfs_file_truncate(&lfs, &file, j+size) => 0; + } + lfs_file_size(&lfs, &file) => MEDIUMSIZE; + + lfs_file_seek(&lfs, &file, 0, LFS_SEEK_SET) => 0; + // should do nothing again + lfs_file_truncate(&lfs, &file, MEDIUMSIZE) => 0; + lfs_file_size(&lfs, &file) => MEDIUMSIZE; + + for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) { + lfs_file_read(&lfs, &file, buffer, size) => size; + memcmp(buffer, "hair", size) => 0; + } + lfs_file_read(&lfs, &file, buffer, size) => 0; + + lfs_file_close(&lfs, &file) => 0; + lfs_unmount(&lfs) => 0; + + // still there after reboot? + lfs_mount(&lfs, &cfg) => 0; + lfs_file_open(&lfs, &file, "baldynoop", LFS_O_RDWR) => 0; + lfs_file_size(&lfs, &file) => MEDIUMSIZE; + for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) { + lfs_file_read(&lfs, &file, buffer, size) => size; + memcmp(buffer, "hair", size) => 0; + } + lfs_file_read(&lfs, &file, buffer, size) => 0; + lfs_file_close(&lfs, &file) => 0; + lfs_unmount(&lfs) => 0; +''' From 47d6b2fcf34c231c491ae4a81dda2b3787f1156b Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 11 Jan 2021 00:13:18 -0600 Subject: [PATCH 07/11] Removed unnecessary truncate condition thanks to new seek optimization --- lfs.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lfs.c b/lfs.c index f54aa049..7b7d5b6c 100644 --- a/lfs.c +++ b/lfs.c @@ -3114,16 +3114,14 @@ static int lfs_file_rawtruncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) { file->flags |= LFS_F_DIRTY | LFS_F_READING; } else if (size > oldsize) { // flush+seek if not already at end - if (file->pos != oldsize) { - lfs_soff_t res = lfs_file_rawseek(lfs, file, 0, LFS_SEEK_END); - if (res < 0) { - return (int)res; - } + lfs_soff_t res = lfs_file_rawseek(lfs, file, 0, LFS_SEEK_END); + if (res < 0) { + return (int)res; } // fill with zeros while (file->pos < size) { - lfs_ssize_t res = lfs_file_rawwrite(lfs, file, &(uint8_t){0}, 1); + res = lfs_file_rawwrite(lfs, file, &(uint8_t){0}, 1); if (res < 0) { return (int)res; } From 10a08833c6ee949829b8c3a85fe98782fc8b066e Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 8 Dec 2020 20:24:58 -0600 Subject: [PATCH 08/11] Moved lfs_mdir_isopen behind LFS_NO_ASSERT lfs_mdir_isopen goes unused if asserts are disabled, and this caused an "unused function" warning on Clang (curiously not on GCC since the function was static inline, commonly used for header-only functions). Also removed "inline" from the lfs_mdir_* functions as these involve linked-list operations and really shouldn't be inlined. And since they are static, inlining should occur automatically if there is a benefit. Found by dpgeorge --- lfs.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lfs.c b/lfs.c index d7439fe3..be2bbd71 100644 --- a/lfs.c +++ b/lfs.c @@ -425,7 +425,8 @@ static inline void lfs_superblock_tole32(lfs_superblock_t *superblock) { superblock->attr_max = lfs_tole32(superblock->attr_max); } -static inline bool lfs_mlist_isopen(struct lfs_mlist *head, +#ifndef LFS_NO_ASSERT +static bool lfs_mlist_isopen(struct lfs_mlist *head, struct lfs_mlist *node) { for (struct lfs_mlist **p = &head; *p; p = &(*p)->next) { if (*p == (struct lfs_mlist*)node) { @@ -435,8 +436,9 @@ static inline bool lfs_mlist_isopen(struct lfs_mlist *head, return false; } +#endif -static inline void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) { +static void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) { for (struct lfs_mlist **p = &lfs->mlist; *p; p = &(*p)->next) { if (*p == mlist) { *p = (*p)->next; @@ -445,7 +447,7 @@ static inline void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) { } } -static inline void lfs_mlist_append(lfs_t *lfs, struct lfs_mlist *mlist) { +static void lfs_mlist_append(lfs_t *lfs, struct lfs_mlist *mlist) { mlist->next = lfs->mlist; lfs->mlist = mlist; } From 21488d9e067ea8b5b5da4e6c9c911c8751c68217 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 13 Dec 2020 10:26:46 -0600 Subject: [PATCH 09/11] Fixed incorrect documentation in test.py The argparse documented an outdated format, and was off by 1. Found by sender6 --- scripts/test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/test.py b/scripts/test.py index e5869c20..9d3d2b5f 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -746,9 +746,9 @@ if __name__ == "__main__": parser.add_argument('testpaths', nargs='*', default=[TESTDIR], help="Description of test(s) to run. By default, this is all tests \ found in the \"{0}\" directory. Here, you can specify a different \ - directory of tests, a specific file, a suite by name, and even a \ - specific test case by adding brackets. For example \ - \"test_dirs[0]\" or \"{0}/test_dirs.toml[0]\".".format(TESTDIR)) + directory of tests, a specific file, a suite by name, and even \ + specific test cases and permutations. For example \ + \"test_dirs#1\" or \"{0}/test_dirs.toml#1#1\".".format(TESTDIR)) parser.add_argument('-D', action='append', default=[], help="Overriding parameter definitions.") parser.add_argument('-v', '--verbose', action='store_true', From e7e4b352bd22d2b7b01ce6cec153a6718f66bd27 Mon Sep 17 00:00:00 2001 From: Will Date: Mon, 4 Jan 2021 11:37:28 +1000 Subject: [PATCH 10/11] lfs_fs_preporphans ret int for graceful LFS_ASSERT --- lfs.c | 53 +++++++++++++++++++++++++++++++++++++++-------------- lfs.h | 2 +- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/lfs.c b/lfs.c index d7439fe3..7622c038 100644 --- a/lfs.c +++ b/lfs.c @@ -465,7 +465,7 @@ static int lfs_file_rawsync(lfs_t *lfs, lfs_file_t *file); static int lfs_file_outline(lfs_t *lfs, lfs_file_t *file); static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file); -static void lfs_fs_preporphans(lfs_t *lfs, int8_t orphans); +static int lfs_fs_preporphans(lfs_t *lfs, int8_t orphans); static void lfs_fs_prepmove(lfs_t *lfs, uint16_t id, const lfs_block_t pair[2]); static int lfs_fs_pred(lfs_t *lfs, const lfs_block_t dir[2], @@ -2061,7 +2061,10 @@ static int lfs_rawmkdir(lfs_t *lfs, const char *path) { // current block end of list? if (cwd.m.split) { // update tails, this creates a desync - lfs_fs_preporphans(lfs, +1); + err = lfs_fs_preporphans(lfs, +1); + if (err) { + return err; + } // it's possible our predecessor has to be relocated, and if // our parent is our predecessor's predecessor, this could have @@ -2081,7 +2084,10 @@ static int lfs_rawmkdir(lfs_t *lfs, const char *path) { } lfs->mlist = cwd.next; - lfs_fs_preporphans(lfs, -1); + err = lfs_fs_preporphans(lfs, -1); + if (err) { + return err; + } } // now insert into our parent block @@ -3206,7 +3212,10 @@ static int lfs_rawremove(lfs_t *lfs, const char *path) { } // mark fs as orphaned - lfs_fs_preporphans(lfs, +1); + err = lfs_fs_preporphans(lfs, +1); + if (err) { + return err; + } // I know it's crazy but yes, dir can be changed by our parent's // commit (if predecessor is child) @@ -3226,7 +3235,10 @@ static int lfs_rawremove(lfs_t *lfs, const char *path) { lfs->mlist = dir.next; if (lfs_tag_type3(tag) == LFS_TYPE_DIR) { // fix orphan - lfs_fs_preporphans(lfs, -1); + err = lfs_fs_preporphans(lfs, -1); + if (err) { + return err; + } err = lfs_fs_pred(lfs, dir.m.pair, &cwd); if (err) { @@ -3312,7 +3324,10 @@ static int lfs_rawrename(lfs_t *lfs, const char *oldpath, const char *newpath) { } // mark fs as orphaned - lfs_fs_preporphans(lfs, +1); + err = lfs_fs_preporphans(lfs, +1); + if (err) { + return err; + } // I know it's crazy but yes, dir can be changed by our parent's // commit (if predecessor is child) @@ -3355,7 +3370,10 @@ static int lfs_rawrename(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) { // fix orphan - lfs_fs_preporphans(lfs, -1); + err = lfs_fs_preporphans(lfs, -1); + if (err) { + return err; + } err = lfs_fs_pred(lfs, prevdir.m.pair, &newcwd); if (err) { @@ -3829,7 +3847,7 @@ int lfs_fs_rawtraverse(lfs_t *lfs, if (err) { return err; } - } else if (includeorphans && + } else if (includeorphans && lfs_tag_type3(tag) == LFS_TYPE_DIRSTRUCT) { for (int i = 0; i < 2; i++) { err = cb(data, (&ctz.head)[i]); @@ -3986,7 +4004,10 @@ static int lfs_fs_relocate(lfs_t *lfs, if (tag != LFS_ERR_NOENT) { // update disk, this creates a desync - lfs_fs_preporphans(lfs, +1); + int err = lfs_fs_preporphans(lfs, +1); + if (err) { + return err; + } // fix pending move in this pair? this looks like an optimization but // is in fact _required_ since relocating may outdate the move. @@ -4003,7 +4024,7 @@ static int lfs_fs_relocate(lfs_t *lfs, } lfs_pair_tole32(newpair); - int err = lfs_dir_commit(lfs, &parent, LFS_MKATTRS( + err = lfs_dir_commit(lfs, &parent, LFS_MKATTRS( {LFS_MKTAG_IF(moveid != 0x3ff, LFS_TYPE_DELETE, moveid, 0), NULL}, {tag, newpair})); @@ -4013,7 +4034,10 @@ static int lfs_fs_relocate(lfs_t *lfs, } // next step, clean up orphans - lfs_fs_preporphans(lfs, -1); + err = lfs_fs_preporphans(lfs, -1); + if (err) { + return err; + } } // find pred @@ -4052,11 +4076,13 @@ static int lfs_fs_relocate(lfs_t *lfs, #endif #ifndef LFS_READONLY -static void lfs_fs_preporphans(lfs_t *lfs, int8_t orphans) { +static int lfs_fs_preporphans(lfs_t *lfs, int8_t orphans) { LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) > 0 || orphans >= 0); lfs->gstate.tag += orphans; lfs->gstate.tag = ((lfs->gstate.tag & ~LFS_MKTAG(0x800, 0, 0)) | ((uint32_t)lfs_gstate_hasorphans(&lfs->gstate) << 31)); + + return 0; } #endif @@ -4173,8 +4199,7 @@ static int lfs_fs_deorphan(lfs_t *lfs) { } // mark orphans as fixed - lfs_fs_preporphans(lfs, -lfs_gstate_getorphans(&lfs->gstate)); - return 0; + return lfs_fs_preporphans(lfs, -lfs_gstate_getorphans(&lfs->gstate)); } #endif diff --git a/lfs.h b/lfs.h index 3b02b6a7..92fb1e8a 100644 --- a/lfs.h +++ b/lfs.h @@ -207,7 +207,7 @@ struct lfs_config { // Number of erasable blocks on the device. lfs_size_t block_count; - // Number of erase cycles before littlefs evicts metadata logs and moves + // Number of erase cycles before littlefs evicts metadata logs and moves // the metadata to another block. Suggested values are in the // range 100-1000, with large values having better performance at the cost // of less consistent wear distribution. From c9eed1f181a0db90c6dcb5126350e3ce6e0b1641 Mon Sep 17 00:00:00 2001 From: Will Date: Thu, 7 Jan 2021 17:22:43 +1000 Subject: [PATCH 11/11] Add test to ensure asserts can return --- .travis.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.travis.yml b/.travis.yml index 4b59af8c..943fd6a2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -224,6 +224,22 @@ jobs: # report-size will compile littlefs and report the size script: [*report-size] + # test compilation with asserts that return -1 + - stage: test + env: + - NAME=littlefs-assert-return + - CC="arm-linux-gnueabi-gcc --static -mthumb" + - CFLAGS="-Werror -D'LFS_ASSERT(test)=do {if(!(test)) {return -1;}} while(0)'" + if: branch !~ -prefix$ + install: + - *install-common + - sudo apt-get install + gcc-arm-linux-gnueabi + libc6-dev-armel-cross + - arm-linux-gnueabi-gcc --version + # report-size will compile littlefs and report the size + script: [*report-size] + # test compilation in thread-safe mode - stage: test env: