Skip to content

Commit

Permalink
Added/fixed tests for noop writes (where bd error can't be trusted)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@geky
geky committed Feb 9, 2020
1 parent 517d341 commit 77e3078
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 188 deletions.
31 changes: 21 additions & 10 deletions bd/lfs_testbd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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){
Expand Down Expand Up @@ -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;
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
20 changes: 12 additions & 8 deletions bd/lfs_testbd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions lfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
6 changes: 1 addition & 5 deletions scripts/test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
'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
Expand Down Expand Up @@ -183,21 +183,17 @@ def shouldtest(self, **args):
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
if_ = (
re.sub('(\&\&|\?)', ' and ',
re.sub('(\|\||:)', ' or ',
re.sub('!(?!=)', ' not ', if_))))
print(if_)
print('---', eval(if_), '---')
return eval(if_)
else:
return True
Expand Down
171 changes: 29 additions & 142 deletions tests/test_badblocks.toml
View file
Original file line number Diff line number Diff line change
@@ -1,72 +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.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 persistent blocks (can't erase)
define.LFS_ERASE_CYCLES = 0xffffffff
define.LFS_BADBLOCK_BEHAVIOR = 'LFS_TESTBD_BADBLOCK_NOERASE'
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 = '''
Expand Down Expand Up @@ -130,78 +72,16 @@ code = '''
}
'''

[[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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 77e3078

Please sign in to comment.