From 2588948d70d05c3bc4cc54529b80fd99ccfb7842 Mon Sep 17 00:00:00 2001 From: Haneef Mubarak Date: Thu, 11 Jul 2019 15:46:17 -0700 Subject: [PATCH 01/18] removed preventing compile on some archs --- emubd/lfs_emubd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/emubd/lfs_emubd.c b/emubd/lfs_emubd.c index 3f31bfa..10158c6 100644 --- a/emubd/lfs_emubd.c +++ b/emubd/lfs_emubd.c @@ -11,7 +11,7 @@ #include #include #include -#include +//#include #include #include #include From 2e92f7a49b430c3aba0891e17b2bc00d92781953 Mon Sep 17 00:00:00 2001 From: Haneef Mubarak Date: Fri, 12 Jul 2019 11:46:18 -0700 Subject: [PATCH 02/18] actually removed --- emubd/lfs_emubd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/emubd/lfs_emubd.c b/emubd/lfs_emubd.c index 10158c6..712fcba 100644 --- a/emubd/lfs_emubd.c +++ b/emubd/lfs_emubd.c @@ -11,7 +11,6 @@ #include #include #include -//#include #include #include #include From 53a6e0471215b680426f97d9afce241d739ee49d Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 17 Jul 2019 17:05:20 -0500 Subject: [PATCH 03/18] Changed block_cycles disable from 0 to -1 As it is now, block_cycles = 0 disables wear leveling. This was a mistake as 0 is the "default" value for several other config options. It's even worse when migrating from v1 as it's easy to miss the addition of block_cycles and end up with a filesystem that is not actually wear-leveling. Clearly, block_cycles = 0 should do anything but disable wear-leveling. Here, I've changed block_cycles = 0 to assert. Forcing users to set a value for block_cycles (500 is suggested). block_cycles can be set to -1 to explicitly disable wear leveling if desired. --- lfs.c | 8 +++++--- lfs.h | 9 ++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lfs.c b/lfs.c index c554135..0417202 100644 --- a/lfs.c +++ b/lfs.c @@ -1453,7 +1453,7 @@ static int lfs_dir_compact(lfs_t *lfs, // increment revision count dir->rev += 1; - if (lfs->cfg->block_cycles && + if (lfs->cfg->block_cycles > 0 && (dir->rev % (lfs->cfg->block_cycles+1) == 0)) { if (lfs_pair_cmp(dir->pair, (const lfs_block_t[2]){0, 1}) == 0) { // oh no! we're writing too much to the superblock, @@ -3192,8 +3192,10 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) { LFS_ASSERT(4*lfs_npw2(0xffffffff / (lfs->cfg->block_size-2*4)) <= lfs->cfg->block_size); - // we don't support some corner cases - LFS_ASSERT(lfs->cfg->block_cycles < 0xffffffff); + // block_cycles = 0 is no longer supported, must either set a number + // of erase cycles before moving logs to another block (~500 suggested), + // or explicitly disable wear-leveling with -1. + LFS_ASSERT(lfs->cfg->block_cycles != 0); // setup read cache if (lfs->cfg->read_buffer) { diff --git a/lfs.h b/lfs.h index c78b3d6..276b5a5 100644 --- a/lfs.h +++ b/lfs.h @@ -190,9 +190,12 @@ struct lfs_config { // Number of erasable blocks on the device. lfs_size_t block_count; - // Number of erase cycles before we should move data to another block. - // May be zero, in which case no block-level wear-leveling is performed. - uint32_t block_cycles; + // Number of erase cycles before we should move logs 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. + // + // Set to -1 to disable block-level wear-leveling. + int32_t block_cycles; // Size of block caches. Each cache buffers a portion of a block in RAM. // The littlefs needs a read cache, a program cache, and one additional From df2e676562aec4db2f979da1ecc8b9e3bcc2ea44 Mon Sep 17 00:00:00 2001 From: Ar2rL Date: Sun, 21 Jul 2019 11:34:14 +0200 Subject: [PATCH 04/18] Add necessary flag to mark file as being opened. --- lfs.h | 1 + 1 file changed, 1 insertion(+) diff --git a/lfs.h b/lfs.h index c78b3d6..9577a17 100644 --- a/lfs.h +++ b/lfs.h @@ -136,6 +136,7 @@ enum lfs_open_flags { LFS_F_READING = 0x040000, // File has been read since last flush LFS_F_ERRED = 0x080000, // An error occured during write LFS_F_INLINE = 0x100000, // Currently inlined in directory entry + LFS_F_OPENED = 0x200000, // File has been opened }; // File seek flags From 72a3758958d7773e90ffed75d2e25df9365d2a9c Mon Sep 17 00:00:00 2001 From: Ar2rL Date: Sun, 21 Jul 2019 11:34:53 +0200 Subject: [PATCH 05/18] Use LFS_F_OPENED flag to protect against use of not opened or closed file. --- lfs.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/lfs.c b/lfs.c index c554135..a3cb85b 100644 --- a/lfs.c +++ b/lfs.c @@ -2248,6 +2248,9 @@ static int lfs_ctz_traverse(lfs_t *lfs, int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, const char *path, int flags, const struct lfs_file_config *cfg) { + // do not allow open for already opened file + LFS_ASSERT(0 == (file->flags & LFS_F_OPENED)); + // deorphan if we haven't yet, needed at most once after poweron if ((flags & 3) != LFS_O_RDONLY) { int err = lfs_fs_forceconsistency(lfs); @@ -2381,6 +2384,8 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, } } + file->flags |= LFS_F_OPENED; + return 0; cleanup: @@ -2397,6 +2402,8 @@ int lfs_file_open(lfs_t *lfs, lfs_file_t *file, } int lfs_file_close(lfs_t *lfs, lfs_file_t *file) { + LFS_ASSERT(file->flags & LFS_F_OPENED); + int err = lfs_file_sync(lfs, file); // remove from list of mdirs @@ -2412,10 +2419,14 @@ int lfs_file_close(lfs_t *lfs, lfs_file_t *file) { lfs_free(file->cache.buffer); } + file->flags &= ~LFS_F_OPENED; + return err; } static int lfs_file_relocate(lfs_t *lfs, lfs_file_t *file) { + LFS_ASSERT(file->flags & LFS_F_OPENED); + while (true) { // just relocate what exists into new block lfs_block_t nblock; @@ -2486,6 +2497,8 @@ relocate: } static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file) { + LFS_ASSERT(file->flags & LFS_F_OPENED); + if (file->flags & LFS_F_READING) { if (!(file->flags & LFS_F_INLINE)) { lfs_cache_drop(lfs, &file->cache); @@ -2564,6 +2577,8 @@ relocate: } int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) { + LFS_ASSERT(file->flags & LFS_F_OPENED); + while (true) { int err = lfs_file_flush(lfs, file); if (err) { @@ -2628,6 +2643,8 @@ lfs_ssize_t lfs_file_read(lfs_t *lfs, lfs_file_t *file, uint8_t *data = buffer; lfs_size_t nsize = size; + LFS_ASSERT(file->flags & LFS_F_OPENED); + if ((file->flags & 3) == LFS_O_WRONLY) { return LFS_ERR_BADF; } @@ -2701,6 +2718,8 @@ lfs_ssize_t lfs_file_write(lfs_t *lfs, lfs_file_t *file, const uint8_t *data = buffer; lfs_size_t nsize = size; + LFS_ASSERT(file->flags & LFS_F_OPENED); + if ((file->flags & 3) == LFS_O_RDONLY) { return LFS_ERR_BADF; } @@ -2821,6 +2840,8 @@ relocate: lfs_soff_t lfs_file_seek(lfs_t *lfs, lfs_file_t *file, lfs_soff_t off, int whence) { + LFS_ASSERT(file->flags & LFS_F_OPENED); + // write out everything beforehand, may be noop if rdonly int err = lfs_file_flush(lfs, file); if (err) { @@ -2848,6 +2869,8 @@ lfs_soff_t lfs_file_seek(lfs_t *lfs, lfs_file_t *file, } int lfs_file_truncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) { + LFS_ASSERT(file->flags & LFS_F_OPENED); + if ((file->flags & 3) == LFS_O_RDONLY) { return LFS_ERR_BADF; } @@ -2906,6 +2929,7 @@ int lfs_file_truncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) { lfs_soff_t lfs_file_tell(lfs_t *lfs, lfs_file_t *file) { (void)lfs; + LFS_ASSERT(file->flags & LFS_F_OPENED); return file->pos; } @@ -2920,6 +2944,7 @@ int lfs_file_rewind(lfs_t *lfs, lfs_file_t *file) { lfs_soff_t lfs_file_size(lfs_t *lfs, lfs_file_t *file) { (void)lfs; + LFS_ASSERT(file->flags & LFS_F_OPENED); if (file->flags & LFS_F_WRITING) { return lfs_max(file->pos, file->ctz.size); } else { @@ -3324,7 +3349,7 @@ int lfs_format(lfs_t *lfs, const struct lfs_config *cfg) { }; lfs_superblock_tole32(&superblock); - err = lfs_dir_commit(lfs, &root, LFS_MKATTRS( + err = lfs_dir_commit(lfs, &root, LFS_MKATTRS( {LFS_MKTAG(LFS_TYPE_CREATE, 0, 0), NULL}, {LFS_MKTAG(LFS_TYPE_SUPERBLOCK, 0, 8), "littlefs"}, {LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), @@ -4311,7 +4336,7 @@ int lfs_migrate(lfs_t *lfs, const struct lfs_config *cfg) { entry1.d.type &= ~0x80; } - + // also fetch name char name[LFS_NAME_MAX+1]; memset(name, 0, sizeof(name)); From 7e1bad3eeeb41edd19a8c0cd70a4d1f36742ffbf Mon Sep 17 00:00:00 2001 From: Ar2rL Date: Sun, 21 Jul 2019 14:36:40 +0200 Subject: [PATCH 06/18] Set LFS_F_OPENED flag at places required by lfs internal logic. --- lfs.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lfs.c b/lfs.c index a3cb85b..bccc4ac 100644 --- a/lfs.c +++ b/lfs.c @@ -2262,7 +2262,7 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, // setup simple file details int err; file->cfg = cfg; - file->flags = flags; + file->flags = flags | LFS_F_OPENED; file->pos = 0; file->cache.buffer = NULL; @@ -2384,8 +2384,6 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, } } - file->flags |= LFS_F_OPENED; - return 0; cleanup: @@ -2514,7 +2512,7 @@ static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file) { lfs_file_t orig = { .ctz.head = file->ctz.head, .ctz.size = file->ctz.size, - .flags = LFS_O_RDONLY, + .flags = LFS_O_RDONLY | LFS_F_OPENED, .pos = file->pos, .cache = lfs->rcache, }; From eb013e6dd6e1cb26e430c632c762436a81a11522 Mon Sep 17 00:00:00 2001 From: "Peter A. Bigot" Date: Tue, 23 Jul 2019 11:05:04 -0500 Subject: [PATCH 07/18] lfs: correct documentation on lookahead-related values The size of the lookahead buffer is required to be a multiple of 8 bytes in anticipation of a future improvement. The buffer itself need only be aligned to support access through a uint32_t pointer. Signed-off-by: Peter A. Bigot --- lfs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lfs.h b/lfs.h index c78b3d6..d09124b 100644 --- a/lfs.h +++ b/lfs.h @@ -204,7 +204,7 @@ struct lfs_config { // Size of the lookahead buffer in bytes. A larger lookahead buffer // increases the number of blocks found during an allocation pass. The // lookahead buffer is stored as a compact bitmap, so each byte of RAM - // can track 8 blocks. Must be a multiple of 4. + // can track 8 blocks. Must be a multiple of 8. lfs_size_t lookahead_size; // Optional statically allocated read buffer. Must be cache_size. @@ -216,7 +216,7 @@ struct lfs_config { void *prog_buffer; // Optional statically allocated lookahead buffer. Must be lookahead_size - // and aligned to a 64-bit boundary. By default lfs_malloc is used to + // and aligned to a 32-bit boundary. By default lfs_malloc is used to // allocate this buffer. void *lookahead_buffer; From 649640c605a41fb1a8cb4cbaa4a620e9edbedbd6 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 31 May 2019 06:42:15 -0500 Subject: [PATCH 08/18] Fixed workaround for erase sizes >1024 B Introduced in 0b76635, the workaround for erases sizes >1024 is to commit with an unaligned CRC tag. Upon reading an unaligned CRC, littlefs should treat the metadata pair as "requires erased". While necessary for portability, this also lets us workaround the lack of handling of erases sizes >1024. Unfortunately, this workaround wasn't implemented correctly (by me) in the case that the metadata-pair does not immediately compact. This is solved here by added the erase check to lfs_dir_commit. Note this is still only a part of a workaround which should be replaced. One potential solution is to pad the commit with multiple smaller CRC tags until we reach the next prog_size boundary. found by kazink --- lfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lfs.c b/lfs.c index c554135..af3a5d7 100644 --- a/lfs.c +++ b/lfs.c @@ -1247,7 +1247,7 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // build crc tag bool reset = ~lfs_frombe32(tag) >> 31; tag = LFS_MKTAG(LFS_TYPE_CRC + reset, 0x3ff, - off - (commit->off+sizeof(lfs_tag_t))); + lfs_min(off - (commit->off+sizeof(lfs_tag_t)), 0x3fe)); // write out crc uint32_t footer[2]; @@ -1760,6 +1760,8 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, // successful commit, update dir dir->off = commit.off; dir->etag = commit.ptag; + // workaround to handle prog_size >1024 + dir->erased = (dir->off % lfs->cfg->prog_size == 0); // note we able to have already handled move here if (lfs_gstate_hasmovehere(&lfs->gpending, dir->pair)) { From ef1c926940f176b20078e56ae4dddd268d9c0497 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 24 Jul 2019 14:19:39 -0500 Subject: [PATCH 09/18] Increased testing to include geometries that can't be fully tested This is primarily to get better test coverage over devices with very large erase/prog/read sizes. The unfortunate state of the tests is that most of them rely on a specific block device size, so that ENOSPC and ECORRUPT errors occur in specific situations. This should be improved in the future, but at least for now we can open up some of the simpler tests to run on these different configurations. Also added testing over both 0x00 and 0xff erase values in emubd. Also added a number of small file tests that expose issues prevalent on NAND devices. --- .travis.yml | 12 ++++++++ emubd/lfs_emubd.c | 2 +- emubd/lfs_emubd.h | 16 ++-------- tests/template.fmt | 2 +- tests/test_files.sh | 71 +++++++++++++++++++++++++++++++++++++++++---- 5 files changed, 81 insertions(+), 22 deletions(-) diff --git a/.travis.yml b/.travis.yml index dcfedf6..04f8720 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,8 +23,20 @@ script: - make test QUIET=1 CFLAGS+="-DLFS_BLOCK_COUNT=1023 -DLFS_LOOKAHEAD_SIZE=256" - make clean test QUIET=1 CFLAGS+="-DLFS_INLINE_MAX=0" + - make clean test QUIET=1 CFLAGS+="-DLFS_EMUBD_ERASE_VALUE=0xff" - make clean test QUIET=1 CFLAGS+="-DLFS_NO_INTRINSICS" + # additional configurations that don't support all tests (this should be + # fixed but at the moment it is what it is) + - make test_files QUIET=1 + CFLAGS+="-DLFS_READ_SIZE=1 -DLFS_BLOCK_SIZE=4096" + - make test_files QUIET=1 + CFLAGS+="-DLFS_READ_SIZE=\(2*1024\) -DLFS_BLOCK_SIZE=\(64*1024\)" + - make test_files QUIET=1 + CFLAGS+="-DLFS_READ_SIZE=\(8*1024\) -DLFS_BLOCK_SIZE=\(64*1024\)" + - make test_files QUIET=1 + CFLAGS+="-DLFS_READ_SIZE=11 -DLFS_BLOCK_SIZE=704" + # compile and find the code size with the smallest configuration - make clean size OBJ="$(ls lfs*.o | tr '\n' ' ')" diff --git a/emubd/lfs_emubd.c b/emubd/lfs_emubd.c index 3f31bfa..3c77945 100644 --- a/emubd/lfs_emubd.c +++ b/emubd/lfs_emubd.c @@ -83,7 +83,7 @@ int lfs_emubd_create(const struct lfs_config *cfg, const char *path) { snprintf(emu->child, LFS_NAME_MAX, ".stats"); FILE *f = fopen(emu->path, "r"); if (!f) { - memset(&emu->stats, 0, sizeof(emu->stats)); + memset(&emu->stats, LFS_EMUBD_ERASE_VALUE, sizeof(emu->stats)); } else { size_t res = fread(&emu->stats, sizeof(emu->stats), 1, f); lfs_emubd_fromle32(emu); diff --git a/emubd/lfs_emubd.h b/emubd/lfs_emubd.h index 64afa3e..0fd78c1 100644 --- a/emubd/lfs_emubd.h +++ b/emubd/lfs_emubd.h @@ -17,20 +17,8 @@ extern "C" // Config options -#ifndef LFS_EMUBD_READ_SIZE -#define LFS_EMUBD_READ_SIZE 1 -#endif - -#ifndef LFS_EMUBD_PROG_SIZE -#define LFS_EMUBD_PROG_SIZE 1 -#endif - -#ifndef LFS_EMUBD_ERASE_SIZE -#define LFS_EMUBD_ERASE_SIZE 512 -#endif - -#ifndef LFS_EMUBD_TOTAL_SIZE -#define LFS_EMUBD_TOTAL_SIZE 524288 +#ifndef LFS_EMUBD_ERASE_VALUE +#define LFS_EMUBD_ERASE_VALUE 0x00 #endif diff --git a/tests/template.fmt b/tests/template.fmt index 7fdec7c..b52d907 100644 --- a/tests/template.fmt +++ b/tests/template.fmt @@ -82,7 +82,7 @@ uintmax_t test; #endif #ifndef LFS_CACHE_SIZE -#define LFS_CACHE_SIZE 64 +#define LFS_CACHE_SIZE (64 % LFS_PROG_SIZE == 0 ? 64 : LFS_PROG_SIZE) #endif #ifndef LFS_LOOKAHEAD_SIZE diff --git a/tests/test_files.sh b/tests/test_files.sh index 5251c61..950636a 100755 --- a/tests/test_files.sh +++ b/tests/test_files.sh @@ -135,20 +135,79 @@ tests/test.py << TEST lfs_unmount(&lfs) => 0; TEST -echo "--- Many file test ---" +echo "--- Many files test ---" tests/test.py << TEST lfs_format(&lfs, &cfg) => 0; TEST tests/test.py << TEST - // Create 300 files of 6 bytes + // Create 300 files of 7 bytes lfs_mount(&lfs, &cfg) => 0; - lfs_mkdir(&lfs, "directory") => 0; for (unsigned i = 0; i < 300; i++) { snprintf((char*)buffer, sizeof(buffer), "file_%03d", i); - lfs_file_open(&lfs, &file[0], (char*)buffer, LFS_O_WRONLY | LFS_O_CREAT) => 0; - size = 6; - memcpy(wbuffer, "Hello", size); + lfs_file_open(&lfs, &file[0], (char*)buffer, + LFS_O_RDWR | LFS_O_CREAT | LFS_O_EXCL) => 0; + size = 7; + snprintf((char*)wbuffer, size, "Hi %03d", i); lfs_file_write(&lfs, &file[0], wbuffer, size) => size; + lfs_file_rewind(&lfs, &file[0]) => 0; + lfs_file_read(&lfs, &file[0], rbuffer, size) => size; + memcmp(wbuffer, rbuffer, size) => 0; + lfs_file_close(&lfs, &file[0]) => 0; + } + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Many files with flush test ---" +tests/test.py << TEST + lfs_format(&lfs, &cfg) => 0; +TEST +tests/test.py << TEST + // Create 300 files of 7 bytes + lfs_mount(&lfs, &cfg) => 0; + for (unsigned i = 0; i < 300; i++) { + snprintf((char*)buffer, sizeof(buffer), "file_%03d", i); + lfs_file_open(&lfs, &file[0], (char*)buffer, + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + size = 7; + snprintf((char*)wbuffer, size, "Hi %03d", i); + lfs_file_write(&lfs, &file[0], wbuffer, size) => size; + lfs_file_close(&lfs, &file[0]) => 0; + + snprintf((char*)buffer, sizeof(buffer), "file_%03d", i); + lfs_file_open(&lfs, &file[0], (char*)buffer, LFS_O_RDONLY) => 0; + size = 7; + snprintf((char*)wbuffer, size, "Hi %03d", i); + lfs_file_read(&lfs, &file[0], rbuffer, size) => size; + memcmp(wbuffer, rbuffer, size) => 0; + lfs_file_close(&lfs, &file[0]) => 0; + } + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Many files with power cycle test ---" +tests/test.py << TEST + lfs_format(&lfs, &cfg) => 0; +TEST +tests/test.py << TEST + // Create 300 files of 7 bytes + lfs_mount(&lfs, &cfg) => 0; + for (unsigned i = 0; i < 300; i++) { + snprintf((char*)buffer, sizeof(buffer), "file_%03d", i); + lfs_file_open(&lfs, &file[0], (char*)buffer, + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + size = 7; + snprintf((char*)wbuffer, size, "Hi %03d", i); + lfs_file_write(&lfs, &file[0], wbuffer, size) => size; + lfs_file_close(&lfs, &file[0]) => 0; + lfs_unmount(&lfs) => 0; + + lfs_mount(&lfs, &cfg) => 0; + snprintf((char*)buffer, sizeof(buffer), "file_%03d", i); + lfs_file_open(&lfs, &file[0], (char*)buffer, LFS_O_RDONLY) => 0; + size = 7; + snprintf((char*)wbuffer, size, "Hi %03d", i); + lfs_file_read(&lfs, &file[0], rbuffer, size) => size; + memcmp(wbuffer, rbuffer, size) => 0; lfs_file_close(&lfs, &file[0]) => 0; } lfs_unmount(&lfs) => 0; From 312326c4e47ed9bc19f0b120faf9d126a15c0bd1 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 24 Jul 2019 14:24:29 -0500 Subject: [PATCH 10/18] Added a better solution for large prog sizes A current limitation of the lfs tag is the 10-bit (1024) length field. This field is used to indicate padding for commits and effectively limits the size of commits to 1KiB. Because commits must be prog size aligned, this is a problem on devices with prog size > 1024. [---- 6KiB erase block ----] [-- 2KiB prog size --|-- 2KiB prog size --|-- 2KiB prog size --] [ 1KiB commit | ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ] This can be increased to 12-bit (4096), but for NAND devices this is still to small to completely solve the issue. The previous workaround was to just create unaligned commits. This can occur naturally if littlefs is used on portable media as the prog size does not have to be consistent on different drivers. If littlefs sees an unaligned commit, it treats the dir as unerased and must compact the dir if it creates any new commits. Unfortunately this isn't great. It effectively means that every small commit forced an erase on devices with prog size > 1024. This is pretty terrible. [---- 6KiB erase block ----] [-- 2KiB prog size --|-- 2KiB prog size --|-- 2KiB prog size --] [ 1KiB commit |------------------- wasted ---------------------] A different solution, implemented here, is to use multiple crc tags to pad the commit until the remaining space fits in the padding. This effectively looks like multiple empty commits and has a small runtime cost to parse these tags, but otherwise does no harm. [---- 6KiB erase block ----] [-- 2KiB prog size --|-- 2KiB prog size --|-- 2KiB prog size --] [ 1KiB commit | noop | 1KiB commit | noop | 1KiB commit | noop ] It was a bit tricky to implement, but now we can effectively support unlimited prog sizes since there's no limit to the number of commits in a block. found by kazink and joicetm --- lfs.c | 113 ++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 66 insertions(+), 47 deletions(-) diff --git a/lfs.c b/lfs.c index af3a5d7..a8724bb 100644 --- a/lfs.c +++ b/lfs.c @@ -1232,65 +1232,85 @@ static int lfs_dir_commitattr(lfs_t *lfs, struct lfs_commit *commit, static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // align to program units - lfs_off_t off = lfs_alignup(commit->off + 2*sizeof(uint32_t), + const lfs_off_t off1 = commit->off + sizeof(lfs_tag_t); + const lfs_off_t end = lfs_alignup(off1 + sizeof(uint32_t), lfs->cfg->prog_size); - // read erased state from next program unit - lfs_tag_t tag; - int err = lfs_bd_read(lfs, - NULL, &lfs->rcache, sizeof(tag), - commit->block, off, &tag, sizeof(tag)); - if (err && err != LFS_ERR_CORRUPT) { - return err; - } + // create crc tags to fill up remainder of commit, note that + // padding is not crcd, which lets fetches skip padding but + // makes committing a bit more complicated + while (commit->off < end) { + lfs_off_t off = commit->off + sizeof(lfs_tag_t); + lfs_off_t noff = lfs_min(end - off, 0x3fe) + off; + if (noff < end) { + noff = lfs_min(noff, end - 2*sizeof(uint32_t)); + } - // build crc tag - bool reset = ~lfs_frombe32(tag) >> 31; - tag = LFS_MKTAG(LFS_TYPE_CRC + reset, 0x3ff, - lfs_min(off - (commit->off+sizeof(lfs_tag_t)), 0x3fe)); + // read erased state from next program unit + lfs_tag_t tag = 0xffffffff; + int err = lfs_bd_read(lfs, + NULL, &lfs->rcache, sizeof(tag), + commit->block, noff, &tag, sizeof(tag)); + if (err && err != LFS_ERR_CORRUPT) { + return err; + } - // write out crc - uint32_t footer[2]; - footer[0] = lfs_tobe32(tag ^ commit->ptag); - commit->crc = lfs_crc(commit->crc, &footer[0], sizeof(footer[0])); - footer[1] = lfs_tole32(commit->crc); - err = lfs_bd_prog(lfs, - &lfs->pcache, &lfs->rcache, false, - commit->block, commit->off, &footer, sizeof(footer)); - if (err) { - return err; - } - commit->off += sizeof(tag)+lfs_tag_size(tag); - commit->ptag = tag ^ (reset << 31); + // build crc tag + bool reset = ~lfs_frombe32(tag) >> 31; + tag = LFS_MKTAG(LFS_TYPE_CRC + reset, 0x3ff, noff - off); - // flush buffers - err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false); - if (err) { - return err; - } - - // successful commit, check checksum to make sure - uint32_t crc = 0xffffffff; - lfs_size_t size = commit->off - lfs_tag_size(tag) - commit->begin; - for (lfs_off_t i = 0; i < size; i++) { - // leave it up to caching to make this efficient - uint8_t dat; - err = lfs_bd_read(lfs, - NULL, &lfs->rcache, size-i, - commit->block, commit->begin+i, &dat, 1); + // write out crc + uint32_t footer[2]; + footer[0] = lfs_tobe32(tag ^ commit->ptag); + commit->crc = lfs_crc(commit->crc, &footer[0], sizeof(footer[0])); + footer[1] = lfs_tole32(commit->crc); + err = lfs_bd_prog(lfs, + &lfs->pcache, &lfs->rcache, false, + commit->block, commit->off, &footer, sizeof(footer)); if (err) { return err; } - crc = lfs_crc(crc, &dat, 1); + commit->off += sizeof(tag)+lfs_tag_size(tag); + commit->ptag = tag ^ (reset << 31); + commit->crc = 0xffffffff; // reset crc for next "commit" } + // flush buffers + int err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false); if (err) { return err; } - if (crc != commit->crc) { - return LFS_ERR_CORRUPT; + // successful commit, check checksums to make sure + lfs_off_t off = commit->begin; + lfs_off_t noff = off1; + while (off < end) { + uint32_t crc = 0xffffffff; + for (lfs_off_t i = off; i < noff+sizeof(uint32_t); i++) { + // leave it up to caching to make this efficient + uint8_t dat; + err = lfs_bd_read(lfs, + NULL, &lfs->rcache, noff+sizeof(uint32_t)-i, + commit->block, i, &dat, 1); + if (err) { + return err; + } + + crc = lfs_crc(crc, &dat, 1); + } + + // detected write error? + if (crc != 0) { + return LFS_ERR_CORRUPT; + } + + // skip padding + off = lfs_min(end - noff, 0x3fe) + noff; + if (off < end) { + off = lfs_min(off, end - 2*sizeof(uint32_t)); + } + noff = off + sizeof(uint32_t); } return 0; @@ -1583,11 +1603,11 @@ static int lfs_dir_compact(lfs_t *lfs, } // successful compaction, swap dir pair to indicate most recent + LFS_ASSERT(commit.off % lfs->cfg->prog_size == 0); lfs_pair_swap(dir->pair); dir->count = end - begin; dir->off = commit.off; dir->etag = commit.ptag; - dir->erased = (dir->off % lfs->cfg->prog_size == 0); // note we able to have already handled move here if (lfs_gstate_hasmovehere(&lfs->gpending, dir->pair)) { lfs_gstate_xormove(&lfs->gpending, @@ -1758,10 +1778,9 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, } // successful commit, update dir + LFS_ASSERT(commit.off % lfs->cfg->prog_size == 0); dir->off = commit.off; dir->etag = commit.ptag; - // workaround to handle prog_size >1024 - dir->erased = (dir->off % lfs->cfg->prog_size == 0); // note we able to have already handled move here if (lfs_gstate_hasmovehere(&lfs->gpending, dir->pair)) { From 19838371fb6e24ddbcec196b2269472f5eafef7f Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 26 Jul 2019 12:29:37 -0500 Subject: [PATCH 11/18] Fixed issue where sed buffering (QUIET=1) caused Travis timeout --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 185d8e5..640dd63 100644 --- a/Makefile +++ b/Makefile @@ -46,7 +46,7 @@ test: test_format test_dirs test_files test_seek test_truncate \ test_%: tests/test_%.sh ifdef QUIET - @./$< | sed -n '/^[-=]/p' + @./$< | sed -nu '/^[-=]/p' else ./$< endif From 38a2a8d2a3d64e5219ec0babaf233ae9e8da577c Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 28 Jul 2019 20:42:13 -0500 Subject: [PATCH 12/18] Minor improvement to documentation over block_cycles Suggested by haneefmubarak --- lfs.c | 10 +++++++--- lfs.h | 7 ++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lfs.c b/lfs.c index 0417202..3fe6389 100644 --- a/lfs.c +++ b/lfs.c @@ -3192,11 +3192,15 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) { LFS_ASSERT(4*lfs_npw2(0xffffffff / (lfs->cfg->block_size-2*4)) <= lfs->cfg->block_size); - // block_cycles = 0 is no longer supported, must either set a number - // of erase cycles before moving logs to another block (~500 suggested), - // or explicitly disable wear-leveling with -1. + // block_cycles = 0 is no longer supported. + // + // block_cycles is the number of erase cycles before littlefs evicts + // metadata logs as a part of wear leveling. Suggested values are in the + // range of 100-1000, or set block_cycles to -1 to disable block-level + // wear-leveling. LFS_ASSERT(lfs->cfg->block_cycles != 0); + // setup read cache if (lfs->cfg->read_buffer) { lfs->rcache.buffer = lfs->cfg->read_buffer; diff --git a/lfs.h b/lfs.h index 276b5a5..6abfcdb 100644 --- a/lfs.h +++ b/lfs.h @@ -190,9 +190,10 @@ struct lfs_config { // Number of erasable blocks on the device. lfs_size_t block_count; - // Number of erase cycles before we should move logs 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. + // 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. // // Set to -1 to disable block-level wear-leveling. int32_t block_cycles; From e8c023aab055a8892b8abc262d82f78a11108dc5 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 28 Jul 2019 20:43:12 -0500 Subject: [PATCH 13/18] Changed FUSE branch to v2 (previously v2-alpha) --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index dcfedf6..ab69041 100644 --- a/.travis.yml +++ b/.travis.yml @@ -111,7 +111,7 @@ jobs: if: branch !~ -prefix$ install: - sudo apt-get install libfuse-dev - - git clone --depth 1 https://github.com/geky/littlefs-fuse -b v2-alpha + - git clone --depth 1 https://github.com/geky/littlefs-fuse -b v2 - fusermount -V - gcc --version before_script: @@ -146,7 +146,7 @@ jobs: if: branch !~ -prefix$ install: - sudo apt-get install libfuse-dev - - git clone --depth 1 https://github.com/geky/littlefs-fuse -b v2-alpha v2 + - git clone --depth 1 https://github.com/geky/littlefs-fuse -b v2 v2 - git clone --depth 1 https://github.com/geky/littlefs-fuse -b v1 v1 - fusermount -V - gcc --version From de5972699a3761d5602d8467df3f5f792bfb1b51 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 18 Jul 2019 19:43:49 -0500 Subject: [PATCH 14/18] Fixed license header in lfs.c Found by pabigot --- lfs.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/lfs.c b/lfs.c index c554135..d968cc9 100644 --- a/lfs.c +++ b/lfs.c @@ -1,19 +1,8 @@ /* * The little filesystem * - * Copyright (c) 2017 ARM Limited - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright (c) 2017, Arm Limited. All rights reserved. + * SPDX-License-Identifier: BSD-3-Clause */ #include "lfs.h" #include "lfs_util.h" From 3806d8828504d303ba6a8e07656535d1e82808f9 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 3 Jul 2019 15:14:59 -0500 Subject: [PATCH 15/18] Fixed seek-related typos in lfs.h - lfs_file_rewind == lfs_file_seek(lfs, file, 0, LFS_SEEK_SET) - lfs_file_seek returns the _new_ position of the file --- lfs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lfs.h b/lfs.h index d09124b..504932d 100644 --- a/lfs.h +++ b/lfs.h @@ -528,7 +528,7 @@ lfs_ssize_t lfs_file_write(lfs_t *lfs, lfs_file_t *file, // Change the position of the file // // The change in position is determined by the offset and whence flag. -// Returns the old position of the file, or a negative error code on failure. +// Returns the new position of the file, or a negative error code on failure. lfs_soff_t lfs_file_seek(lfs_t *lfs, lfs_file_t *file, lfs_soff_t off, int whence); @@ -545,7 +545,7 @@ lfs_soff_t lfs_file_tell(lfs_t *lfs, lfs_file_t *file); // Change the position of the file to the beginning of the file // -// Equivalent to lfs_file_seek(lfs, file, 0, LFS_SEEK_CUR) +// Equivalent to lfs_file_seek(lfs, file, 0, LFS_SEEK_SET) // Returns a negative error code on failure. int lfs_file_rewind(lfs_t *lfs, lfs_file_t *file); From 4ec442527247b5b534e302e3936491ad18337dbb Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 16 Jul 2019 15:23:42 -0500 Subject: [PATCH 16/18] Fixed overlapping memcpy in emubd Found by DanielLyubin --- emubd/lfs_emubd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/emubd/lfs_emubd.c b/emubd/lfs_emubd.c index 712fcba..8c2c30b 100644 --- a/emubd/lfs_emubd.c +++ b/emubd/lfs_emubd.c @@ -215,7 +215,7 @@ int lfs_emubd_prog(const struct lfs_config *cfg, lfs_block_t block, // update history and stats if (block != emu->history.blocks[0]) { - memcpy(&emu->history.blocks[1], &emu->history.blocks[0], + memmove(&emu->history.blocks[1], &emu->history.blocks[0], sizeof(emu->history) - sizeof(emu->history.blocks[0])); emu->history.blocks[0] = block; } From 4850e01e14373d9808b78f3fb7833e26583744c1 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 24 Jul 2019 14:59:48 -0500 Subject: [PATCH 17/18] Changed rdonly/wronly mistakes to assert Previously these returned LFS_ERR_BADF. But attempting to modify a file opened read-only, or reading a write-only flie, is a user error and should not occur in normal use. Changing this to an assert allows the logic to be omitted if the user disables asserts to reduce the code footprint (not suggested unless the user really really knows what they're doing). --- lfs.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/lfs.c b/lfs.c index a535295..dd63677 100644 --- a/lfs.c +++ b/lfs.c @@ -2631,10 +2631,7 @@ lfs_ssize_t lfs_file_read(lfs_t *lfs, lfs_file_t *file, lfs_size_t nsize = size; LFS_ASSERT(file->flags & LFS_F_OPENED); - - if ((file->flags & 3) == LFS_O_WRONLY) { - return LFS_ERR_BADF; - } + LFS_ASSERT((file->flags & 3) != LFS_O_WRONLY); if (file->flags & LFS_F_WRITING) { // flush out any writes @@ -2706,10 +2703,7 @@ lfs_ssize_t lfs_file_write(lfs_t *lfs, lfs_file_t *file, lfs_size_t nsize = size; LFS_ASSERT(file->flags & LFS_F_OPENED); - - if ((file->flags & 3) == LFS_O_RDONLY) { - return LFS_ERR_BADF; - } + LFS_ASSERT((file->flags & 3) != LFS_O_RDONLY); if (file->flags & LFS_F_READING) { // drop any reads @@ -2857,10 +2851,7 @@ lfs_soff_t lfs_file_seek(lfs_t *lfs, lfs_file_t *file, int lfs_file_truncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) { LFS_ASSERT(file->flags & LFS_F_OPENED); - - if ((file->flags & 3) == LFS_O_RDONLY) { - return LFS_ERR_BADF; - } + LFS_ASSERT((file->flags & 3) != LFS_O_RDONLY); if (size > LFS_FILE_MAX) { return LFS_ERR_INVAL; From 0d4c0b105cabc7dd76a9aaa3f82f655a7b35aa79 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 9 Jul 2019 17:51:15 -0500 Subject: [PATCH 18/18] Fixed issue where inline files were not cleaned up Due to the logging nature of metadata pairs, switching from inline files (type3 = 0x201) to CTZ skip-lists (type3 = 0x202) does not explicitly erase inline files, but instead leaves them up to compaction to omit. To save code size, this is handled by the same logic that deduplicates tags. Unfortunately, this wasn't working. Due to a relatively late change in v2 the struct's type field was changed to no longer be a part of determining a tag's "uniqueness". A part of this should have been the modification of directory traversal filtering to respect type-dependent uniqueness, but I missed this. The fix is to add in correct type-dependent filtering. Also there was some clean up necessary around removing delete tags during compaction and outlining files. Note that while this appears to conflict with the possibility of combining inline + ctz files, we still have the device-side-only LFS_TYPE_FROM tag that can be repurposed for 256 additional inline "chunks". Found by Johnxjj --- lfs.c | 40 +++++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/lfs.c b/lfs.c index dd63677..70f54f3 100644 --- a/lfs.c +++ b/lfs.c @@ -403,7 +403,7 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, static int lfs_dir_compact(lfs_t *lfs, lfs_mdir_t *dir, const struct lfs_mattr *attrs, int attrcount, lfs_mdir_t *source, uint16_t begin, uint16_t end); -static int lfs_file_relocate(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 void lfs_fs_prepmove(lfs_t *lfs, @@ -619,11 +619,17 @@ static int lfs_dir_traverse_filter(void *p, lfs_tag_t *filtertag = p; (void)buffer; + // which mask depends on unique bit in tag structure + uint32_t mask = (tag & LFS_MKTAG(0x100, 0, 0)) + ? LFS_MKTAG(0x7ff, 0x3ff, 0) + : LFS_MKTAG(0x700, 0x3ff, 0); + // check for redundancy - uint32_t mask = LFS_MKTAG(0x7ff, 0x3ff, 0); if ((mask & tag) == (mask & *filtertag) || - (mask & tag) == (LFS_MKTAG(LFS_TYPE_DELETE, 0, 0) | - (LFS_MKTAG(0, 0x3ff, 0) & *filtertag))) { + lfs_tag_isdelete(*filtertag) || + (LFS_MKTAG(0x7ff, 0x3ff, 0) & tag) == ( + LFS_MKTAG(LFS_TYPE_DELETE, 0, 0) | + (LFS_MKTAG(0, 0x3ff, 0) & *filtertag))) { return true; } @@ -1632,11 +1638,7 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, if (dir != &f->m && lfs_pair_cmp(f->m.pair, dir->pair) == 0 && f->type == LFS_TYPE_REG && (f->flags & LFS_F_INLINE) && f->ctz.size > lfs->cfg->cache_size) { - f->flags &= ~LFS_F_READING; - f->off = 0; - - lfs_alloc_ack(lfs); - int err = lfs_file_relocate(lfs, f); + int err = lfs_file_outline(lfs, f); if (err) { return err; } @@ -2471,7 +2473,6 @@ static int lfs_file_relocate(lfs_t *lfs, lfs_file_t *file) { lfs_cache_zero(lfs, &lfs->pcache); file->block = nblock; - file->flags &= ~LFS_F_INLINE; file->flags |= LFS_F_WRITING; return 0; @@ -2483,6 +2484,18 @@ relocate: } } +static int lfs_file_outline(lfs_t *lfs, lfs_file_t *file) { + file->off = file->pos; + lfs_alloc_ack(lfs); + int err = lfs_file_relocate(lfs, file); + if (err) { + return err; + } + + file->flags &= ~LFS_F_INLINE; + return 0; +} + static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file) { LFS_ASSERT(file->flags & LFS_F_OPENED); @@ -2616,8 +2629,7 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) { relocate: // inline file doesn't fit anymore - file->off = file->pos; - err = lfs_file_relocate(lfs, file); + err = lfs_file_outline(lfs, file); if (err) { file->flags |= LFS_F_ERRED; return err; @@ -2740,9 +2752,7 @@ lfs_ssize_t lfs_file_write(lfs_t *lfs, lfs_file_t *file, lfs_min(0x3fe, lfs_min( lfs->cfg->cache_size, lfs->cfg->block_size/8))) { // inline file doesn't fit anymore - file->off = file->pos; - lfs_alloc_ack(lfs); - int err = lfs_file_relocate(lfs, file); + int err = lfs_file_outline(lfs, file); if (err) { file->flags |= LFS_F_ERRED; return err;