From d498b9fb31504af2d2e580d3ec0a5031cdd5d941 Mon Sep 17 00:00:00 2001 From: Derek Thrasher Date: Tue, 24 Mar 2020 09:22:05 -0400 Subject: [PATCH 1/4] (bugfix) adding line function to clear out all the global 'free' information so that we can reset it after a failed traversal --- lfs.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lfs.c b/lfs.c index 93a124d..4caa363 100644 --- a/lfs.c +++ b/lfs.c @@ -10,6 +10,8 @@ #define LFS_BLOCK_NULL ((lfs_block_t)-1) #define LFS_BLOCK_INLINE ((lfs_block_t)-2) +static void lfs_alloc_ack(lfs_t *lfs); + /// Caching block device operations /// static inline void lfs_cache_drop(lfs_t *lfs, lfs_cache_t *rcache) { // do not zero, cheaper if cache is readonly or only going to be @@ -24,6 +26,15 @@ static inline void lfs_cache_zero(lfs_t *lfs, lfs_cache_t *pcache) { pcache->block = LFS_BLOCK_NULL; } +/// Invalidate the lookahead buffer. This is done during mounting and failed traversals /// +static inline void lfs_setup_invalid_lookahead_buffer(lfs_t *lfs) +{ + lfs->free.off = lfs->seed % lfs->cfg->block_size; + lfs->free.size = 0; + lfs->free.i = 0; + lfs_alloc_ack(lfs); +} + static int lfs_bd_read(lfs_t *lfs, const lfs_cache_t *pcache, lfs_cache_t *rcache, lfs_size_t hint, lfs_block_t block, lfs_off_t off, @@ -477,6 +488,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { memset(lfs->free.buffer, 0, lfs->cfg->lookahead_size); int err = lfs_fs_traverseraw(lfs, lfs_alloc_lookahead, lfs, true); if (err) { + lfs_setup_invalid_lookahead_buffer(lfs); return err; } } @@ -3772,10 +3784,7 @@ int lfs_mount(lfs_t *lfs, const struct lfs_config *cfg) { lfs->gdisk = lfs->gstate; // setup free lookahead - lfs->free.off = lfs->seed % lfs->cfg->block_size; - lfs->free.size = 0; - lfs->free.i = 0; - lfs_alloc_ack(lfs); + lfs_setup_invalid_lookahead_buffer(lfs); LFS_TRACE("lfs_mount -> %d", 0); return 0; @@ -4594,6 +4603,7 @@ static int lfs1_mount(lfs_t *lfs, struct lfs1 *lfs1, lfs->lfs1->root[1] = LFS_BLOCK_NULL; // setup free lookahead + // TODO should this also call lfs_setup_invalid_lookahead_buffer(lfs); the free.off is different in the current version of lfs lfs->free.off = 0; lfs->free.size = 0; lfs->free.i = 0; From 5e5b5d85724607b79efb426c02da3e2009e607b1 Mon Sep 17 00:00:00 2001 From: Derek Thrasher Date: Thu, 26 Mar 2020 08:47:12 -0400 Subject: [PATCH 2/4] (chore) updates from PR, we decided not to move forward with changing v1 code since it can be risky. Let's improve the future! Also renamed and moved around a the lookahead free / reset function --- lfs.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/lfs.c b/lfs.c index 4caa363..72b8220 100644 --- a/lfs.c +++ b/lfs.c @@ -26,15 +26,6 @@ static inline void lfs_cache_zero(lfs_t *lfs, lfs_cache_t *pcache) { pcache->block = LFS_BLOCK_NULL; } -/// Invalidate the lookahead buffer. This is done during mounting and failed traversals /// -static inline void lfs_setup_invalid_lookahead_buffer(lfs_t *lfs) -{ - lfs->free.off = lfs->seed % lfs->cfg->block_size; - lfs->free.size = 0; - lfs->free.i = 0; - lfs_alloc_ack(lfs); -} - static int lfs_bd_read(lfs_t *lfs, const lfs_cache_t *pcache, lfs_cache_t *rcache, lfs_size_t hint, lfs_block_t block, lfs_off_t off, @@ -448,6 +439,19 @@ static int lfs_alloc_lookahead(void *p, lfs_block_t block) { return 0; } +static void lfs_alloc_ack(lfs_t *lfs) { + lfs->free.ack = lfs->cfg->block_count; +} + +/// Invalidate the lookahead buffer. This is done during mounting and failed traversals /// +static void lfs_alloc_reset(lfs_t *lfs) +{ + lfs->free.off = lfs->seed % lfs->cfg->block_size; + lfs->free.size = 0; + lfs->free.i = 0; + lfs_alloc_ack(lfs); +} + static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { while (true) { while (lfs->free.i != lfs->free.size) { @@ -488,17 +492,12 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { memset(lfs->free.buffer, 0, lfs->cfg->lookahead_size); int err = lfs_fs_traverseraw(lfs, lfs_alloc_lookahead, lfs, true); if (err) { - lfs_setup_invalid_lookahead_buffer(lfs); + lfs_alloc_reset(lfs); return err; } } } -static void lfs_alloc_ack(lfs_t *lfs) { - lfs->free.ack = lfs->cfg->block_count; -} - - /// Metadata pair and directory operations /// static lfs_stag_t lfs_dir_getslice(lfs_t *lfs, const lfs_mdir_t *dir, lfs_tag_t gmask, lfs_tag_t gtag, @@ -3784,7 +3783,7 @@ int lfs_mount(lfs_t *lfs, const struct lfs_config *cfg) { lfs->gdisk = lfs->gstate; // setup free lookahead - lfs_setup_invalid_lookahead_buffer(lfs); + lfs_alloc_reset(lfs); LFS_TRACE("lfs_mount -> %d", 0); return 0; @@ -4603,7 +4602,6 @@ static int lfs1_mount(lfs_t *lfs, struct lfs1 *lfs1, lfs->lfs1->root[1] = LFS_BLOCK_NULL; // setup free lookahead - // TODO should this also call lfs_setup_invalid_lookahead_buffer(lfs); the free.off is different in the current version of lfs lfs->free.off = 0; lfs->free.size = 0; lfs->free.i = 0; From f17d3d7ebae42fc97ac740d7cc1a89d80524585d Mon Sep 17 00:00:00 2001 From: Derek Thrasher Date: Thu, 26 Mar 2020 08:48:52 -0400 Subject: [PATCH 3/4] Minor cleanup - Removed the declaration of lfs_alloc_ack - Consistent brackets --- lfs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lfs.c b/lfs.c index 72b8220..d7b4a90 100644 --- a/lfs.c +++ b/lfs.c @@ -10,8 +10,6 @@ #define LFS_BLOCK_NULL ((lfs_block_t)-1) #define LFS_BLOCK_INLINE ((lfs_block_t)-2) -static void lfs_alloc_ack(lfs_t *lfs); - /// Caching block device operations /// static inline void lfs_cache_drop(lfs_t *lfs, lfs_cache_t *rcache) { // do not zero, cheaper if cache is readonly or only going to be @@ -443,9 +441,9 @@ static void lfs_alloc_ack(lfs_t *lfs) { lfs->free.ack = lfs->cfg->block_count; } -/// Invalidate the lookahead buffer. This is done during mounting and failed traversals /// -static void lfs_alloc_reset(lfs_t *lfs) -{ +// Invalidate the lookahead buffer. This is done during mounting and +// failed traversals +static void lfs_alloc_reset(lfs_t *lfs) { lfs->free.off = lfs->seed % lfs->cfg->block_size; lfs->free.size = 0; lfs->free.i = 0; From f9dbec3d92b9b71c81accbcfbf7e8da8f6ab70ea Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 24 Mar 2020 17:30:25 -0500 Subject: [PATCH 4/4] Added test case catching issues with errors during a lookahead scan Original issue found by thrasher8390 --- tests/test_alloc.toml | 84 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/tests/test_alloc.toml b/tests/test_alloc.toml index c05f001..fa92da5 100644 --- a/tests/test_alloc.toml +++ b/tests/test_alloc.toml @@ -323,6 +323,90 @@ code = ''' lfs_unmount(&lfs) => 0; ''' +[[case]] # what if we have a bad block during an allocation scan? +in = "lfs.c" +define.LFS_ERASE_CYCLES = 0xffffffff +define.LFS_BADBLOCK_BEHAVIOR = 'LFS_TESTBD_BADBLOCK_READERROR' +code = ''' + lfs_format(&lfs, &cfg) => 0; + lfs_mount(&lfs, &cfg) => 0; + // first fill to exhaustion to find available space + lfs_file_open(&lfs, &file, "pacman", LFS_O_WRONLY | LFS_O_CREAT) => 0; + strcpy((char*)buffer, "waka"); + size = strlen("waka"); + lfs_size_t filesize = 0; + while (true) { + lfs_ssize_t res = lfs_file_write(&lfs, &file, buffer, size); + assert(res == (lfs_ssize_t)size || res == LFS_ERR_NOSPC); + if (res == LFS_ERR_NOSPC) { + break; + } + filesize += size; + } + lfs_file_close(&lfs, &file) => 0; + // now fill all but a couple of blocks of the filesystem with data + filesize -= 3*LFS_BLOCK_SIZE; + lfs_file_open(&lfs, &file, "pacman", LFS_O_WRONLY | LFS_O_CREAT) => 0; + strcpy((char*)buffer, "waka"); + size = strlen("waka"); + for (lfs_size_t i = 0; i < filesize/size; i++) { + lfs_file_write(&lfs, &file, buffer, size) => size; + } + lfs_file_close(&lfs, &file) => 0; + // also save head of file so we can error during lookahead scan + lfs_block_t fileblock = file.ctz.head; + lfs_unmount(&lfs) => 0; + + // remount to force an alloc scan + lfs_mount(&lfs, &cfg) => 0; + + // but mark the head of our file as a "bad block", this is force our + // scan to bail early + lfs_testbd_setwear(&cfg, fileblock, 0xffffffff) => 0; + lfs_file_open(&lfs, &file, "ghost", LFS_O_WRONLY | LFS_O_CREAT) => 0; + strcpy((char*)buffer, "chomp"); + size = strlen("chomp"); + while (true) { + lfs_ssize_t res = lfs_file_write(&lfs, &file, buffer, size); + assert(res == (lfs_ssize_t)size || res == LFS_ERR_CORRUPT); + if (res == LFS_ERR_CORRUPT) { + break; + } + } + lfs_file_close(&lfs, &file) => 0; + + // now reverse the "bad block" and try to write the file again until we + // run out of space + lfs_testbd_setwear(&cfg, fileblock, 0) => 0; + lfs_file_open(&lfs, &file, "ghost", LFS_O_WRONLY | LFS_O_CREAT) => 0; + strcpy((char*)buffer, "chomp"); + size = strlen("chomp"); + while (true) { + lfs_ssize_t res = lfs_file_write(&lfs, &file, buffer, size); + assert(res == (lfs_ssize_t)size || res == LFS_ERR_NOSPC); + if (res == LFS_ERR_NOSPC) { + break; + } + } + lfs_file_close(&lfs, &file) => 0; + + lfs_unmount(&lfs) => 0; + + // check that the disk isn't hurt + lfs_mount(&lfs, &cfg) => 0; + lfs_file_open(&lfs, &file, "pacman", LFS_O_RDONLY) => 0; + strcpy((char*)buffer, "waka"); + size = strlen("waka"); + for (lfs_size_t i = 0; i < filesize/size; i++) { + uint8_t rbuffer[4]; + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + lfs_file_close(&lfs, &file) => 0; + lfs_unmount(&lfs) => 0; +''' + + # Below, I don't like these tests. They're fragile and depend _heavily_ # on the geometry of the block device. But they are valuable. Eventually they # should be removed and replaced with generalized tests.