From 77e3078b9f28ff689e5a623250eae5c85ae8b3aa Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 29 Jan 2020 17:50:38 -0600 Subject: [PATCH] Added/fixed tests for noop writes (where bd error can't be trusted) It's interesting how many ways block devices can show failed writes: 1. prog can error 2. erase can error 3. read can error after writing (ECC failure) 4. prog doesn't error but doesn't write the data correctly 5. erase doesn't error but doesn't erase correctly Can read fail without an error? Yes, though this appears the same as prog and erase failing. These weren't all simulated by testbd since I unintentionally assumed the block device could always error. Fixed by added additional bad-black behaviors to testbd. Note: This also includes a small fix where we can miss bad writes if the underlying block device contains a valid commit with the exact same size in the exact same offset. --- bd/lfs_testbd.c | 31 ++++--- bd/lfs_testbd.h | 20 +++-- lfs.c | 6 +- scripts/test.py | 6 +- tests/test_badblocks.toml | 171 +++++++------------------------------ tests/test_exhaustion.toml | 34 +++----- 6 files changed, 80 insertions(+), 188 deletions(-) diff --git a/bd/lfs_testbd.c b/bd/lfs_testbd.c index 62cba01..8de7dcb 100644 --- a/bd/lfs_testbd.c +++ b/bd/lfs_testbd.c @@ -47,7 +47,7 @@ int lfs_testbd_createcfg(const struct lfs_config *cfg, const char *path, memset(bd->wear, 0, sizeof(lfs_testbd_wear_t) * cfg->block_count); } - + // create underlying block device if (bd->persist) { bd->u.file.cfg = (struct lfs_filebd_config){ @@ -155,9 +155,8 @@ int lfs_testbd_read(const struct lfs_config *cfg, lfs_block_t block, LFS_ASSERT(block < cfg->block_count); // block bad? - if (bd->cfg->erase_cycles && - bd->cfg->badblock_behavior == LFS_TESTBD_BADBLOCK_NOREAD && - bd->wear[block] >= bd->cfg->erase_cycles) { + if (bd->cfg->erase_cycles && bd->wear[block] >= bd->cfg->erase_cycles && + bd->cfg->badblock_behavior == LFS_TESTBD_BADBLOCK_READERROR) { LFS_TRACE("lfs_testbd_read -> %d", LFS_ERR_CORRUPT); return LFS_ERR_CORRUPT; } @@ -180,11 +179,18 @@ int lfs_testbd_prog(const struct lfs_config *cfg, lfs_block_t block, LFS_ASSERT(block < cfg->block_count); // block bad? - if (bd->cfg->erase_cycles && - bd->cfg->badblock_behavior == LFS_TESTBD_BADBLOCK_NOPROG && - bd->wear[block] >= bd->cfg->erase_cycles) { - LFS_TRACE("lfs_testbd_prog -> %d", LFS_ERR_CORRUPT); - return LFS_ERR_CORRUPT; + if (bd->cfg->erase_cycles && bd->wear[block] >= bd->cfg->erase_cycles) { + if (bd->cfg->badblock_behavior == + LFS_TESTBD_BADBLOCK_PROGERROR) { + LFS_TRACE("lfs_testbd_prog -> %d", LFS_ERR_CORRUPT); + return LFS_ERR_CORRUPT; + } else if (bd->cfg->badblock_behavior == + LFS_TESTBD_BADBLOCK_PROGNOOP || + bd->cfg->badblock_behavior == + LFS_TESTBD_BADBLOCK_ERASENOOP) { + LFS_TRACE("lfs_testbd_prog -> %d", 0); + return 0; + } } // prog @@ -219,9 +225,14 @@ int lfs_testbd_erase(const struct lfs_config *cfg, lfs_block_t block) { // block bad? if (bd->cfg->erase_cycles) { if (bd->wear[block] >= bd->cfg->erase_cycles) { - if (bd->cfg->badblock_behavior == LFS_TESTBD_BADBLOCK_NOERASE) { + if (bd->cfg->badblock_behavior == + LFS_TESTBD_BADBLOCK_ERASEERROR) { LFS_TRACE("lfs_testbd_erase -> %d", LFS_ERR_CORRUPT); return LFS_ERR_CORRUPT; + } else if (bd->cfg->badblock_behavior == + LFS_TESTBD_BADBLOCK_ERASENOOP) { + LFS_TRACE("lfs_testbd_erase -> %d", 0); + return 0; } } else { // mark wear diff --git a/bd/lfs_testbd.h b/bd/lfs_testbd.h index 78580d4..68180ac 100644 --- a/bd/lfs_testbd.h +++ b/bd/lfs_testbd.h @@ -19,14 +19,18 @@ extern "C" #endif -// Mode determining how "bad blocks" behave during testing. This -// simulates some real-world circumstances such as writes not -// going through (noprog), erases not sticking (noerase), and ECC -// failures (noread). +// Mode determining how "bad blocks" behave during testing. This simulates +// some real-world circumstances such as progs not sticking (prog-noop), +// a readonly disk (erase-noop), and ECC failures (read-error). +// +// Not that read-noop is not allowed. Read _must_ return a consistent (but +// may be arbitrary) value on every read. enum lfs_testbd_badblock_behavior { - LFS_TESTBD_BADBLOCK_NOPROG = 0, - LFS_TESTBD_BADBLOCK_NOERASE = 1, - LFS_TESTBD_BADBLOCK_NOREAD = 2, + LFS_TESTBD_BADBLOCK_PROGERROR, + LFS_TESTBD_BADBLOCK_ERASEERROR, + LFS_TESTBD_BADBLOCK_READERROR, + LFS_TESTBD_BADBLOCK_PROGNOOP, + LFS_TESTBD_BADBLOCK_ERASENOOP, }; // Type for measuring wear @@ -82,7 +86,7 @@ typedef struct lfs_testbd { /// Block device API /// // Create a test block device using the geometry in lfs_config -// +// // Note that filebd is used if a path is provided, if path is NULL // testbd will use rambd which can be much faster. int lfs_testbd_create(const struct lfs_config *cfg, const char *path); diff --git a/lfs.c b/lfs.c index 0a27e9b..9785092 100644 --- a/lfs.c +++ b/lfs.c @@ -1300,9 +1300,9 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // check against written crc to detect if block is readonly // (we may pick up old commits) // TODO rm me? -// if (i == noff && crc != ncrc) { -// return LFS_ERR_CORRUPT; -// } + if (i == noff && crc != ncrc) { + return LFS_ERR_CORRUPT; + } crc = lfs_crc(crc, &dat, 1); } diff --git a/scripts/test.py b/scripts/test.py index 3c3d692..cbc8838 100755 --- a/scripts/test.py +++ b/scripts/test.py @@ -52,7 +52,7 @@ DEFINES = { 'LFS_LOOKAHEAD_SIZE': 16, 'LFS_ERASE_VALUE': 0xff, 'LFS_ERASE_CYCLES': 0, - 'LFS_BADBLOCK_BEHAVIOR': 'LFS_TESTBD_BADBLOCK_NOPROG', + 'LFS_BADBLOCK_BEHAVIOR': 'LFS_TESTBD_BADBLOCK_PROGERROR', } PROLOGUE = """ // prologue @@ -183,12 +183,10 @@ class TestCase: return False elif self.if_ is not None: if_ = self.if_ - print(if_) while True: for k, v in self.defines.items(): if k in if_: if_ = if_.replace(k, '(%s)' % v) - print(if_) break else: break @@ -196,8 +194,6 @@ class TestCase: re.sub('(\&\&|\?)', ' and ', re.sub('(\|\||:)', ' or ', re.sub('!(?!=)', ' not ', if_)))) - print(if_) - print('---', eval(if_), '---') return eval(if_) else: return True diff --git a/tests/test_badblocks.toml b/tests/test_badblocks.toml index 7969d43..44c3e09 100644 --- a/tests/test_badblocks.toml +++ b/tests/test_badblocks.toml @@ -1,6 +1,14 @@ [[case]] # single bad blocks +define.LFS_BLOCK_COUNT = 256 # small bd so test runs faster define.LFS_ERASE_CYCLES = 0xffffffff -define.LFS_BADBLOCK_BEHAVIOR = 'LFS_TESTBD_BADBLOCK_NOPROG' +define.LFS_ERASE_VALUE = [0x00, 0xff, -1] +define.LFS_BADBLOCK_BEHAVIOR = [ + 'LFS_TESTBD_BADBLOCK_PROGERROR', + 'LFS_TESTBD_BADBLOCK_ERASEERROR', + 'LFS_TESTBD_BADBLOCK_READERROR', + 'LFS_TESTBD_BADBLOCK_PROGNOOP', + 'LFS_TESTBD_BADBLOCK_ERASENOOP', +] define.NAMEMULT = 64 define.FILEMULT = 1 code = ''' @@ -64,144 +72,16 @@ code = ''' } ''' -[[case]] # single persistent blocks (can't erase) -define.LFS_ERASE_CYCLES = 0xffffffff -define.LFS_BADBLOCK_BEHAVIOR = 'LFS_TESTBD_BADBLOCK_NOERASE' -define.NAMEMULT = 64 -define.FILEMULT = 1 -code = ''' - for (lfs_block_t badblock = 2; badblock < LFS_BLOCK_COUNT; badblock++) { - lfs_testbd_setwear(&cfg, badblock-1, 0) => 0; - lfs_testbd_setwear(&cfg, badblock, 0xffffffff) => 0; - - lfs_format(&lfs, &cfg) => 0; - - lfs_mount(&lfs, &cfg) => 0; - for (int i = 1; i < 10; i++) { - for (int j = 0; j < NAMEMULT; j++) { - buffer[j] = '0'+i; - } - buffer[NAMEMULT] = '\0'; - lfs_mkdir(&lfs, (char*)buffer) => 0; - - buffer[NAMEMULT] = '/'; - for (int j = 0; j < NAMEMULT; j++) { - buffer[j+NAMEMULT+1] = '0'+i; - } - buffer[2*NAMEMULT+1] = '\0'; - lfs_file_open(&lfs, &file, (char*)buffer, - LFS_O_WRONLY | LFS_O_CREAT) => 0; - - size = NAMEMULT; - for (int j = 0; j < i*FILEMULT; j++) { - lfs_file_write(&lfs, &file, buffer, size) => size; - } - - lfs_file_close(&lfs, &file) => 0; - } - lfs_unmount(&lfs) => 0; - - lfs_mount(&lfs, &cfg) => 0; - for (int i = 1; i < 10; i++) { - for (int j = 0; j < NAMEMULT; j++) { - buffer[j] = '0'+i; - } - buffer[NAMEMULT] = '\0'; - lfs_stat(&lfs, (char*)buffer, &info) => 0; - info.type => LFS_TYPE_DIR; - - buffer[NAMEMULT] = '/'; - for (int j = 0; j < NAMEMULT; j++) { - buffer[j+NAMEMULT+1] = '0'+i; - } - buffer[2*NAMEMULT+1] = '\0'; - lfs_file_open(&lfs, &file, (char*)buffer, LFS_O_RDONLY) => 0; - - size = NAMEMULT; - for (int j = 0; j < i*FILEMULT; j++) { - uint8_t rbuffer[1024]; - lfs_file_read(&lfs, &file, rbuffer, size) => size; - memcmp(buffer, rbuffer, size) => 0; - } - - lfs_file_close(&lfs, &file) => 0; - } - lfs_unmount(&lfs) => 0; - } -''' - -[[case]] # single unreadable blocks (can't read) -define.LFS_ERASE_CYCLES = 0xffffffff -define.LFS_BADBLOCK_BEHAVIOR = 'LFS_TESTBD_BADBLOCK_NOREAD' -define.NAMEMULT = 64 -define.FILEMULT = 1 -code = ''' - for (lfs_block_t badblock = 2; badblock < LFS_BLOCK_COUNT/2; badblock++) { - lfs_testbd_setwear(&cfg, badblock-1, 0) => 0; - lfs_testbd_setwear(&cfg, badblock, 0xffffffff) => 0; - - lfs_format(&lfs, &cfg) => 0; - - lfs_mount(&lfs, &cfg) => 0; - for (int i = 1; i < 10; i++) { - for (int j = 0; j < NAMEMULT; j++) { - buffer[j] = '0'+i; - } - buffer[NAMEMULT] = '\0'; - lfs_mkdir(&lfs, (char*)buffer) => 0; - - buffer[NAMEMULT] = '/'; - for (int j = 0; j < NAMEMULT; j++) { - buffer[j+NAMEMULT+1] = '0'+i; - } - buffer[2*NAMEMULT+1] = '\0'; - lfs_file_open(&lfs, &file, (char*)buffer, - LFS_O_WRONLY | LFS_O_CREAT) => 0; - - size = NAMEMULT; - for (int j = 0; j < i*FILEMULT; j++) { - lfs_file_write(&lfs, &file, buffer, size) => size; - } - - lfs_file_close(&lfs, &file) => 0; - } - lfs_unmount(&lfs) => 0; - - lfs_mount(&lfs, &cfg) => 0; - for (int i = 1; i < 10; i++) { - for (int j = 0; j < NAMEMULT; j++) { - buffer[j] = '0'+i; - } - buffer[NAMEMULT] = '\0'; - lfs_stat(&lfs, (char*)buffer, &info) => 0; - info.type => LFS_TYPE_DIR; - - buffer[NAMEMULT] = '/'; - for (int j = 0; j < NAMEMULT; j++) { - buffer[j+NAMEMULT+1] = '0'+i; - } - buffer[2*NAMEMULT+1] = '\0'; - lfs_file_open(&lfs, &file, (char*)buffer, LFS_O_RDONLY) => 0; - - size = NAMEMULT; - for (int j = 0; j < i*FILEMULT; j++) { - uint8_t rbuffer[1024]; - lfs_file_read(&lfs, &file, rbuffer, size) => size; - memcmp(buffer, rbuffer, size) => 0; - } - - lfs_file_close(&lfs, &file) => 0; - } - lfs_unmount(&lfs) => 0; - } -''' - [[case]] # region corruption (causes cascading failures) +define.LFS_BLOCK_COUNT = 256 # small bd so test runs faster define.LFS_ERASE_CYCLES = 0xffffffff +define.LFS_ERASE_VALUE = [0x00, 0xff, -1] define.LFS_BADBLOCK_BEHAVIOR = [ - 'LFS_TESTBD_BADBLOCK_NOPROG', - 'LFS_TESTBD_BADBLOCK_NOERASE', - 'LFS_TESTBD_BADBLOCK_NOREAD', + 'LFS_TESTBD_BADBLOCK_PROGERROR', + 'LFS_TESTBD_BADBLOCK_ERASEERROR', + 'LFS_TESTBD_BADBLOCK_READERROR', + 'LFS_TESTBD_BADBLOCK_PROGNOOP', + 'LFS_TESTBD_BADBLOCK_ERASENOOP', ] define.NAMEMULT = 64 define.FILEMULT = 1 @@ -266,11 +146,15 @@ code = ''' ''' [[case]] # alternating corruption (causes cascading failures) +define.LFS_BLOCK_COUNT = 256 # small bd so test runs faster define.LFS_ERASE_CYCLES = 0xffffffff +define.LFS_ERASE_VALUE = [0x00, 0xff, -1] define.LFS_BADBLOCK_BEHAVIOR = [ - 'LFS_TESTBD_BADBLOCK_NOPROG', - 'LFS_TESTBD_BADBLOCK_NOERASE', - 'LFS_TESTBD_BADBLOCK_NOREAD', + 'LFS_TESTBD_BADBLOCK_PROGERROR', + 'LFS_TESTBD_BADBLOCK_ERASEERROR', + 'LFS_TESTBD_BADBLOCK_READERROR', + 'LFS_TESTBD_BADBLOCK_PROGNOOP', + 'LFS_TESTBD_BADBLOCK_ERASENOOP', ] define.NAMEMULT = 64 define.FILEMULT = 1 @@ -337,10 +221,13 @@ code = ''' # other corner cases [[case]] # bad superblocks (corrupt 1 or 0) define.LFS_ERASE_CYCLES = 0xffffffff +define.LFS_ERASE_VALUE = [0x00, 0xff, -1] define.LFS_BADBLOCK_BEHAVIOR = [ - 'LFS_TESTBD_BADBLOCK_NOPROG', - 'LFS_TESTBD_BADBLOCK_NOERASE', - 'LFS_TESTBD_BADBLOCK_NOREAD', + 'LFS_TESTBD_BADBLOCK_PROGERROR', + 'LFS_TESTBD_BADBLOCK_ERASEERROR', + 'LFS_TESTBD_BADBLOCK_READERROR', + 'LFS_TESTBD_BADBLOCK_PROGNOOP', + 'LFS_TESTBD_BADBLOCK_ERASENOOP', ] code = ''' lfs_testbd_setwear(&cfg, 0, 0xffffffff) => 0; diff --git a/tests/test_exhaustion.toml b/tests/test_exhaustion.toml index 7faa176..86bf15c 100644 --- a/tests/test_exhaustion.toml +++ b/tests/test_exhaustion.toml @@ -1,11 +1,13 @@ [[case]] # test running a filesystem to exhaustion define.LFS_ERASE_CYCLES = 10 -define.LFS_BLOCK_COUNT = 256 # small bd so it runs faster +define.LFS_BLOCK_COUNT = 256 # small bd so test runs faster define.LFS_BLOCK_CYCLES = 'LFS_ERASE_CYCLES / 2' define.LFS_BADBLOCK_BEHAVIOR = [ - 'LFS_TESTBD_BADBLOCK_NOPROG', - 'LFS_TESTBD_BADBLOCK_NOERASE', - 'LFS_TESTBD_BADBLOCK_NOREAD', + 'LFS_TESTBD_BADBLOCK_PROGERROR', + 'LFS_TESTBD_BADBLOCK_ERASEERROR', + 'LFS_TESTBD_BADBLOCK_READERROR', + 'LFS_TESTBD_BADBLOCK_PROGNOOP', + 'LFS_TESTBD_BADBLOCK_ERASENOOP', ] define.FILES = 10 code = ''' @@ -79,12 +81,14 @@ exhausted: [[case]] # test running a filesystem to exhaustion # which also requires expanding superblocks define.LFS_ERASE_CYCLES = 10 -define.LFS_BLOCK_COUNT = 256 # small bd so it runs faster +define.LFS_BLOCK_COUNT = 256 # small bd so test runs faster define.LFS_BLOCK_CYCLES = 'LFS_ERASE_CYCLES / 2' define.LFS_BADBLOCK_BEHAVIOR = [ - 'LFS_TESTBD_BADBLOCK_NOPROG', - 'LFS_TESTBD_BADBLOCK_NOERASE', - 'LFS_TESTBD_BADBLOCK_NOREAD', + 'LFS_TESTBD_BADBLOCK_PROGERROR', + 'LFS_TESTBD_BADBLOCK_ERASEERROR', + 'LFS_TESTBD_BADBLOCK_READERROR', + 'LFS_TESTBD_BADBLOCK_PROGNOOP', + 'LFS_TESTBD_BADBLOCK_ERASENOOP', ] define.FILES = 10 code = ''' @@ -159,13 +163,8 @@ exhausted: [[case]] # wear-level test running a filesystem to exhaustion define.LFS_ERASE_CYCLES = 20 -define.LFS_BLOCK_COUNT = 256 # small bd so it runs faster +define.LFS_BLOCK_COUNT = 256 # small bd so test runs faster define.LFS_BLOCK_CYCLES = 'LFS_ERASE_CYCLES / 2' -define.LFS_BADBLOCK_BEHAVIOR = [ - 'LFS_TESTBD_BADBLOCK_NOPROG', - 'LFS_TESTBD_BADBLOCK_NOERASE', - 'LFS_TESTBD_BADBLOCK_NOREAD', -] define.FILES = 10 code = ''' uint32_t run_cycles[2]; @@ -252,13 +251,8 @@ exhausted: [[case]] # wear-level test + expanding superblock define.LFS_ERASE_CYCLES = 20 -define.LFS_BLOCK_COUNT = 256 # small bd so it runs faster +define.LFS_BLOCK_COUNT = 256 # small bd so test runs faster define.LFS_BLOCK_CYCLES = 'LFS_ERASE_CYCLES / 2' -define.LFS_BADBLOCK_BEHAVIOR = [ - 'LFS_TESTBD_BADBLOCK_NOPROG', - 'LFS_TESTBD_BADBLOCK_NOERASE', - 'LFS_TESTBD_BADBLOCK_NOREAD', -] define.FILES = 10 code = ''' uint32_t run_cycles[2];