From 55b072e7618b253a162bc942e3af5db405817e49 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 18 Jan 2023 14:28:40 -0600 Subject: [PATCH] Opened up rbyd testing for all geometries, and fixed related bugs - Caching is still presenting issues with the new requirements for rbyd trees, in this case the default bd, with 64 byte progs, revealed and issue where rcache could become outdated when reading from disk while ignoring what's in the pcache. It assumes the pcache will always override the rcache, but this is not true after pcache is flushed. This didn't happen before as the rcache and pcache don't interact while writing in the previous implementation. Because of these new requirements the caching system probably deserves a rework... - The quick tests for sublinear space utilization don't work when prog_size is > a byte, fortunately we should always have NOR-like geometry under test, so we can limit these asserts to NOR-like geometry. - Lots of problems fitting these tests into 512-byte block_size geometries, which is a bit concerning. This may be a larger change from the previous implementation than expected. This may deserve more scrutiny at small block sizes to see how things fit, since the sublinear space utilization doesn't really kick in at this scale... On the other hand it may just be that these tests are too aggressive for 512-byte block sizes, since they don't yet do compaction, which should help with padding/crc overhead... --- lfs.c | 71 +++++++++++++++++++++++----- lfs.h | 2 +- tests/test_rbyd.toml | 107 ++++++++++++++++++++++++++++++++++++------- 3 files changed, 150 insertions(+), 30 deletions(-) diff --git a/lfs.c b/lfs.c index 1a331b56..cfb0a7dd 100644 --- a/lfs.c +++ b/lfs.c @@ -97,6 +97,23 @@ static int lfs_bd_read(lfs_t *lfs, return err; } + // TODO this was a quick hack, the entire cache system probably + // requires a deeper look + // + // fix overlaps with our pcache + if (pcache + && block == pcache->block + && off < pcache->off + pcache->size + && off + diff > pcache->off) { + lfs_off_t off_ = lfs_max(off, pcache->off); + lfs_size_t diff_ = lfs_min( + diff - (off_-off), + pcache->size - (off_-pcache->off)); + memcpy(&data[off_-off], + &pcache->buffer[off_-pcache->off], + diff_); + } + data += diff; off += diff; size -= diff; @@ -119,6 +136,23 @@ static int lfs_bd_read(lfs_t *lfs, if (err) { return err; } + + // TODO this was a quick hack, the entire cache system probably + // requires a deeper look + // + // fix overlaps with our pcache + if (pcache + && rcache->block == pcache->block + && rcache->off < pcache->off + pcache->size + && rcache->off + rcache->size > pcache->off) { + lfs_off_t off_ = lfs_max(rcache->off, pcache->off); + lfs_size_t size_ = lfs_min( + rcache->size - (off_-rcache->off), + pcache->size - (off_-pcache->off)); + memcpy(&rcache->buffer[off_-rcache->off], + &pcache->buffer[off_-pcache->off], + size_); + } } return 0; @@ -259,8 +293,10 @@ static int lfs_bd_prog(lfs_t *lfs, && off < rcache->off + rcache->size && off + size > rcache->off) { lfs_off_t off_ = lfs_max(off, rcache->off); - lfs_size_t size_ = lfs_min(size, rcache->size - (off_-rcache->off)); - memcpy(&rcache->buffer[off_-rcache->off], data+(off_-off), size_); + lfs_size_t size_ = lfs_min( + size - (off_-off), + rcache->size - (off_-rcache->off)); + memcpy(&rcache->buffer[off_-rcache->off], &data[off_-off], size_); } while (size > 0) { @@ -428,6 +464,8 @@ enum lfsr_tag_type { LFSR_TAG_RMUATTR = 0x1001, LFSR_TAG_CRC = 0x0002, + LFSR_TAG_CRC0 = 0x0002, + LFSR_TAG_CRC1 = 0x0003, LFSR_TAG_FCRC = 0x0802, LFSR_TAG_ALT = 0x0004, @@ -1158,15 +1196,17 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd, // found a create? increase count of ids if (lfsr_tag_suptype(tag) == LFSR_TAG_MK) { - // TODO put this somewhere? we can't check this here because - // we may be reading invalid tags - //LFS_ASSERT(count < 0xffff); + // NOTE we can't check for overflow/underflow here because we + // may be overeagerly parsing an invalid commit, it's ok for + // this to overflow/underflow as long as we throw it out later + // on a bad crc count += 1; // found a delete? decrease count of ids } else if (lfsr_tag_suptype(tag) == LFSR_TAG_RM) { - // TODO put this somewhere? we can't check this here because - // we may be reading invalid tags - //LFS_ASSERT(count > 0); + // NOTE we can't check for overflow/underflow here because we + // may be overeagerly parsing an invalid commit, it's ok for + // this to overflow/underflow as long as we throw it out later + // on a bad crc count -= 1; // found an fcrc? save for later } else if (lfsr_tag_suptype(tag) == LFSR_TAG_FCRC) { @@ -1622,7 +1662,8 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd_, prune = true; // cut while following } else if (diverged - && lfsr_tag_follow(alt, lower_lower, lower_upper, lower_tag_) + && lfsr_tag_follow(alt, + lower_lower, lower_upper, lower_tag_) && (lower_tag_ < upper_tag_) == lfsr_tag_islt(alt)) { lfsr_tag_trim( lfsr_tag_flip(alt, lower_lower, lower_upper), @@ -1630,7 +1671,8 @@ static int lfsr_rbyd_append(lfs_t *lfs, lfsr_rbyd_t *rbyd_, prune = true; // cut while not following } else if (diverged - && !lfsr_tag_follow(alt, lower_lower, lower_upper, lower_tag_) + && !lfsr_tag_follow(alt, + lower_lower, lower_upper, lower_tag_) && (lower_tag_ < upper_tag_) != lfsr_tag_islt(alt)) { lfsr_tag_trim(alt, &lower_lower, &lower_upper); lfs_swap(&jump, &branch_); @@ -1926,12 +1968,12 @@ int lfsr_rbyd_commit(lfs_t *lfs, lfsr_rbyd_t *rbyd, // + 4-byte fcrc crc // + 4-byte fcrc crc-len (worst case) // + 1-byte crc tag - // + 4-byte crc len (worst case) + // + 4-byte crc+padding len (worst case) // + 4-byte crc crc // = 19 bytes // - 3-word crc with no following prog (end of block) // 1-byte crc tag - // + 4-byte crc len (worst case) + // + 4-byte crc+padding len (worst case) // + 4-byte crc crc // = 9 bytes // @@ -1939,6 +1981,11 @@ int lfsr_rbyd_commit(lfs_t *lfs, lfsr_rbyd_t *rbyd, rbyd_.off + 2+1+4+4 + 1+4+4, lfs->cfg->prog_size); + // not even space for the crc? + if (aligned > lfs->cfg->block_size) { + return LFS_ERR_RANGE; + } + // space for fcrc? uint8_t perturb = 0; if (aligned <= lfs->cfg->block_size - lfs->cfg->prog_size) { diff --git a/lfs.h b/lfs.h index 991dc68b..fbc20cee 100644 --- a/lfs.h +++ b/lfs.h @@ -84,7 +84,7 @@ enum lfs_error { LFS_ERR_NOMEM = -12, // No more memory available LFS_ERR_NOATTR = -61, // No data/attr available LFS_ERR_NAMETOOLONG = -36, // File name too long - LFS_ERR_RANGE = -7, // Result out of range + LFS_ERR_RANGE = -34, // Result out of range // TODO should all overflow errors actually be corrupt errors? LFS_ERR_OVERFLOW = -75, // Value too large for defined data type }; diff --git a/tests/test_rbyd.toml b/tests/test_rbyd.toml index c6b3cc66..13d37082 100644 --- a/tests/test_rbyd.toml +++ b/tests/test_rbyd.toml @@ -45,6 +45,7 @@ code = ''' [cases.test_rbyd_multi_commit] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -69,7 +70,6 @@ code = ''' // commit with one attribute rbyd = init_rbyd; lfs_bd_erase(&lfs, rbyd.block) => 0; - lfsr_rbyd_commit(&lfs, &rbyd, NULL) => 0; lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTR2(UATTR, 1, -1, "\xaa\xaa\xaa\xaa", 4, NULL)) => 0; lfsr_rbyd_fetch(&lfs, &rbyd, rbyd.block, NULL) => 0; @@ -86,6 +86,7 @@ code = ''' [cases.test_rbyd_commit_fetch_commit] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -192,6 +193,7 @@ code = ''' [cases.test_rbyd_multi_lookup] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -336,6 +338,7 @@ code = ''' [cases.test_rbyd_multi_get] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -431,7 +434,6 @@ code = ''' // so that lookups return strictly the tag greater than or equal to // the tag requested rbyd = init_rbyd; - rbyd = init_rbyd; lfs_bd_erase(&lfs, rbyd.block) => 0; lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTR2(UATTR, 1, -1, "\xaa\xaa\xaa\xaa", 4, @@ -455,6 +457,7 @@ code = ''' [cases.test_rbyd_multi_traverse] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -477,7 +480,6 @@ code = ''' // so that lookups return strictly the tag greater than or equal to // the tag requested rbyd = init_rbyd; - rbyd = init_rbyd; lfs_bd_erase(&lfs, rbyd.block) => 0; lfsr_rbyd_commit(&lfs, &rbyd, LFSR_ATTR2(UATTR, 1, -1, "\xaa\xaa\xaa\xaa", 4, NULL)) => 0; @@ -1650,12 +1652,16 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_multi_permutations] defines.N = 'range(1, 8)' in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= N' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -1742,7 +1748,10 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_large] @@ -1816,6 +1825,7 @@ code = ''' [cases.test_rbyd_remove] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -1904,6 +1914,7 @@ code = ''' [cases.test_rbyd_remove_permutations] defines.N = 'range(1, 7)' in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= N+1' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -2019,11 +2030,15 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_remove_missing] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 4' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -2115,6 +2130,7 @@ code = ''' [cases.test_rbyd_remove_again] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 8' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -2256,6 +2272,7 @@ code = ''' [cases.test_rbyd_remove_all] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -2338,6 +2355,7 @@ code = ''' [cases.test_rbyd_remove_all_permutations] defines.N = 'range(1, 7)' in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2*N+1' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -2459,12 +2477,16 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_remove_append_permutations] defines.N = 'range(1, 6)' in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= N+2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -2594,7 +2616,10 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' @@ -2753,6 +2778,7 @@ code = ''' [cases.test_rbyd_multi_create] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 3' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -3019,12 +3045,16 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_multi_create_permutations] defines.N = 'range(1, 8)' in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= N' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -3127,7 +3157,10 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_create_large] @@ -3204,6 +3237,7 @@ code = ''' [cases.test_rbyd_delete] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -3364,6 +3398,7 @@ code = ''' [cases.test_rbyd_delete_range] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -3606,6 +3641,7 @@ code = ''' [cases.test_rbyd_delete_permutations] defines.N = 'range(1, 7)' in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= N+1' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -3737,13 +3773,20 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_delete_range_permutations] defines.N = 'range(1, 7)' defines.M = 'range(1, 4)' in = 'lfs.c' +if = ''' + BLOCK_SIZE/PROG_SIZE >= N+N*M+1 + && BLOCK_SIZE >= 4096 +''' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -3894,11 +3937,15 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_delete_all] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -4015,6 +4062,7 @@ code = ''' [cases.test_rbyd_delete_all_range] in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= 2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -4143,6 +4191,10 @@ code = ''' [cases.test_rbyd_delete_all_permutations] defines.N = 'range(1, 7)' in = 'lfs.c' +if = ''' + BLOCK_SIZE/PROG_SIZE >= 2*N+1 + && BLOCK_SIZE >= 1024 +''' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -4282,13 +4334,20 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_delete_all_range_permutations] defines.N = 'range(1, 7)' defines.M = 'range(1, 4)' in = 'lfs.c' +if = ''' + BLOCK_SIZE/PROG_SIZE >= N+N*M + N + 1+M + && BLOCK_SIZE >= 4096 +''' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -4448,12 +4507,16 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_delete_create_permutations] defines.N = 'range(1, 6)' in = 'lfs.c' +if = 'BLOCK_SIZE/PROG_SIZE >= N+2' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -4602,13 +4665,20 @@ code = ''' worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } ''' [cases.test_rbyd_delete_create_range_permutations] defines.N = 'range(1, 6)' defines.M = 'range(1, 4)' in = 'lfs.c' +if = ''' + BLOCK_SIZE/PROG_SIZE >= N+N*M + 1 + 1+M + && BLOCK_SIZE >= 4096 +''' code = ''' lfs_t lfs; lfs_init(&lfs, cfg) => 0; @@ -4782,10 +4852,13 @@ code = ''' // test that tree is self-balancing, we should be strictly bounded // by height <= 2*log(n)+1, assume tags are roughly ~8 bytes - lfs_size_t n = 1 + N+N*M + 2 + 1+M; + lfs_size_t n = 1 + N+N*M + 1 + 1+M; printf("worst size: %u B (N=%u, estimate=%u)\n", worst_size, n, 8*n*(2*lfs_nlog2(n)+1)); printf("avg height: %u B (N=%u, estimate=%u)\n", worst_size / n, n, 8*(2*lfs_nlog2(n)+1)); - assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + // note this only holds true with byte-level progs + if (PROG_SIZE == 1) { + assert(worst_size / n <= 8*(2*lfs_nlog2(n)+1)); + } '''