diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 460ac604..a1a1a436 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -96,7 +96,7 @@ jobs: | capture("Code size is (?[0-9]+)").result' \ prev-results.json || echo 0)" ./scripts/code.py -u results/code-thumb-readonly.csv -s | awk ' - NR==2 {printf "Code size (readonly),%d B",$2} + NR==2 {printf "Code size
(readonly),%d B",$2} NR==2 && ENVIRON["PREV"]+0 != 0 { printf " (%+.1f%%)",100*($2-ENVIRON["PREV"])/ENVIRON["PREV"]} NR==2 {printf "\n"}' \ @@ -107,7 +107,7 @@ jobs: | capture("Code size is (?[0-9]+)").result' \ prev-results.json || echo 0)" ./scripts/code.py -u results/code-thumb-threadsafe.csv -s | awk ' - NR==2 {printf "Code size (threadsafe),%d B",$2} + NR==2 {printf "Code size
(threadsafe),%d B",$2} NR==2 && ENVIRON["PREV"]+0 != 0 { printf " (%+.1f%%)",100*($2-ENVIRON["PREV"])/ENVIRON["PREV"]} NR==2 {printf "\n"}' \ @@ -118,7 +118,18 @@ jobs: | capture("Code size is (?[0-9]+)").result' \ prev-results.json || echo 0)" ./scripts/code.py -u results/code-thumb-migrate.csv -s | awk ' - NR==2 {printf "Code size (migrate),%d B",$2} + NR==2 {printf "Code size
(migrate),%d B",$2} + NR==2 && ENVIRON["PREV"]+0 != 0 { + printf " (%+.1f%%)",100*($2-ENVIRON["PREV"])/ENVIRON["PREV"]} + NR==2 {printf "\n"}' \ + >> results.csv) + [ -e results/code-thumb-error-asserts.csv ] && ( \ + export PREV="$(jq -re ' + select(.context == "results / code (error-asserts)").description + | capture("Code size is (?[0-9]+)").result' \ + prev-results.json || echo 0)" + ./scripts/code.py -u results/code-thumb-error-asserts.csv -s | awk ' + NR==2 {printf "Code size
(error-asserts),%d B",$2} NR==2 && ENVIRON["PREV"]+0 != 0 { printf " (%+.1f%%)",100*($2-ENVIRON["PREV"])/ENVIRON["PREV"]} NR==2 {printf "\n"}' \ diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 907224c6..6d633f8e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -195,6 +195,17 @@ jobs: -DLFS_NO_ERROR \ -DLFS_MIGRATE" \ CODEFLAGS+="-o results/code-${{matrix.arch}}-migrate.csv" + - name: results-code-error-asserts + run: | + mkdir -p results + make clean + make code \ + CFLAGS+=" \ + -DLFS_NO_DEBUG \ + -DLFS_NO_WARN \ + -DLFS_NO_ERROR \ + -D'LFS_ASSERT(test)=do {if(!(test)) {return -1;}} while(0)'" \ + CODEFLAGS+="-o results/code-${{matrix.arch}}-error-asserts.csv" - name: upload-results uses: actions/upload-artifact@v2 with: diff --git a/lfs.c b/lfs.c index a6bfbf81..d976389d 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; } @@ -465,7 +467,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], @@ -1589,7 +1591,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->cfg->block_size/2, + lfs_alignup((lfs->cfg->metadata_max ? + lfs->cfg->metadata_max : lfs->cfg->block_size)/2, lfs->cfg->prog_size))) { break; } @@ -1674,7 +1677,8 @@ static int lfs_dir_compact(lfs_t *lfs, .crc = 0xffffffff, .begin = 0, - .end = lfs->cfg->block_size - 8, + .end = (lfs->cfg->metadata_max ? + lfs->cfg->metadata_max : lfs->cfg->block_size) - 8, }; // erase block to write to @@ -1884,7 +1888,8 @@ 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->cfg->metadata_max ? + lfs->cfg->metadata_max : lfs->cfg->block_size) - 8, }; // traverse attrs that need to be written out @@ -2061,7 +2066,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 +2089,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 @@ -2966,7 +2977,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->cfg->block_size/8))) { + 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) { @@ -3048,14 +3061,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) { @@ -3063,7 +3068,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) { @@ -3071,6 +3076,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; @@ -3101,21 +3119,22 @@ 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; } 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; } @@ -3206,7 +3225,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 +3248,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 +3337,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 +3383,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) { @@ -3536,6 +3567,8 @@ 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); + // setup default state lfs->root[0] = LFS_BLOCK_NULL; lfs->root[1] = LFS_BLOCK_NULL; @@ -3616,12 +3649,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 +3656,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: @@ -3829,7 +3862,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 +4019,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 +4039,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 +4049,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 +4091,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 +4214,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..c7ec6d3e 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,12 @@ 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; }; // File info structure diff --git a/scripts/test.py b/scripts/test.py index 0ffcb7f1..4e205f73 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -812,9 +812,9 @@ if __name__ == "__main__": parser.add_argument('test_paths', nargs='*', default=[TEST_PATHS], 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(TEST_PATHS)) + 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(TEST_PATHS)) parser.add_argument('-D', action='append', default=[], help="Overriding parameter definitions.") parser.add_argument('-v', '--verbose', action='store_true', 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; +'''