From 7925f9f019ddcff9d2f8bfd3ef277117e35e85d1 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 8 May 2023 12:42:15 -0500 Subject: [PATCH] Some more mtree split/uninlining tests and fixes Currently relying on lfsr_rbyd_append/appendattrs to inject extra attributes during lfsr_mdir_commit, need to consider if this is really the best solution. This probably results in more function calls than we really need. --- lfs.c | 233 ++++++++++++++++++++++++++++++++++-------- tests/test_mtree.toml | 171 +++++++++++++++++++++++-------- 2 files changed, 323 insertions(+), 81 deletions(-) diff --git a/lfs.c b/lfs.c index 193ccb5e..795f17ea 100644 --- a/lfs.c +++ b/lfs.c @@ -601,6 +601,7 @@ enum lfsr_tag_type { LFSR_TAG_STRUCT = 0x3000, LFSR_TAG_INLINED = 0x3000, + LFSR_TAG_MKINLINED = 0x3004, // test only? LFSR_TAG_BLOCK = 0x3100, LFSR_TAG_MDIR = 0x3200, LFSR_TAG_RMMDIR = 0x3202, @@ -2479,6 +2480,20 @@ failed:; return err; } +static int lfsr_rbyd_appendall(lfs_t *lfs, lfsr_rbyd_t *rbyd, + const lfsr_attr_t *attrs, lfs_size_t attr_count) { + // append each tag to the tree + for (lfs_size_t i = 0; i < attr_count; i++) { + int err = lfsr_rbyd_append(lfs, rbyd, + attrs[i].id, attrs[i].tag, attrs[i].delta, attrs[i].data); + if (err) { + return err; + } + } + + return 0; +} + static int lfsr_rbyd_commit(lfs_t *lfs, lfsr_rbyd_t *rbyd, const lfsr_attr_t *attrs, lfs_size_t attr_count) { // must fetch before mutating! @@ -2507,12 +2522,9 @@ static int lfsr_rbyd_commit(lfs_t *lfs, lfsr_rbyd_t *rbyd, } // append each tag to the tree - for (lfs_size_t i = 0; i < attr_count; i++) { - err = lfsr_rbyd_append(lfs, &rbyd_, - attrs[i].id, attrs[i].tag, attrs[i].delta, attrs[i].data); - if (err) { - goto failed; - } + err = lfsr_rbyd_appendall(lfs, &rbyd_, attrs, attr_count); + if (err) { + goto failed; } // align to the next prog unit @@ -5126,19 +5138,77 @@ static lfs_ssize_t lfsr_mdir_get(lfs_t *lfs, const lfsr_mdir_t *mdir, return lfsr_rbyd_get(lfs, &mdir->rbyd, id, tag, buffer, size); } + +// mtree is the core tree of mdirs in littlefs + +static inline int lfsr_mtree_isinlined(lfs_t *lfs) { + return lfsr_btree_isnull(&lfs->mtree); +} + +static inline lfs_ssize_t lfsr_mtree_weight(lfs_t *lfs) { + // inlined mdir? + if (lfsr_mtree_isinlined(lfs)) { + return lfs->supermdir.rbyd.weight; + } else { + return lfsr_btree_weight(&lfs->mtree); + } +} + +static int lfsr_mtree_lookup(lfs_t *lfs, lfs_ssize_t mid, lfsr_mdir_t *mdir_) { + // TODO should we really allow -1=>supermdir lookup? + LFS_ASSERT(mid >= -1); + LFS_ASSERT(mid < lfsr_mtree_weight(lfs)); + + // looking up supermdir? + if (mid < 0) { + *mdir_ = lfs->supermdir; + return 0; + + // look up mdir in actual mtree + } else { + lfsr_tag_t tag; + lfsr_data_t data; + int err = lfsr_btree_lookup(lfs, &lfs->mtree, mid, + &tag, NULL, &data, false); + if (err) { + return err; + } + LFS_ASSERT(tag == LFSR_TAG_MDIR); + + // decode mpair + lfsr_mpair_t mpair; + lfs_ssize_t d = lfsr_mpair_fromdisk(lfs, &mpair, data); + if (d < 0) { + return d; + } + + // fetch mdir + return lfsr_mdir_fetch(lfs, mdir_, mid, mpair, NULL); + } +} + + + + // TODO how much of this code can we share with btree_commit? // TODO share commit? // TODO share split? // TODO would be awfully convenient if c supported multiple returns static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, const lfsr_attr_t *attrs, lfs_size_t attr_count) { - // scratch space for unrolled tail recursion - uint8_t recurse_buf[LFSR_BTREE_DSIZE]; - lfsr_attr_t recurse_attrs[3]; +// // scratch space for unrolled tail recursion +// uint8_t recurse_buf[LFSR_BTREE_DSIZE]; +// lfsr_attr_t recurse_attrs[3]; + + // TODO need both dirty_mtree and uninlining? + bool dirty_mtree = false; while (true) { // try to commit - int err = lfsr_rbyd_commit(lfs, &mdir->rbyd, attrs, attr_count); + // TODO do we need a backup rbyd here? + // TODO this was a quick hack, should rbyd_ be the same as mdir_? + lfsr_rbyd_t rbyd_ = mdir->rbyd; + int err = lfsr_rbyd_appendall(lfs, &rbyd_, attrs, attr_count); if (err && err != LFS_ERR_RANGE) { //TODO should we also move if there is corruption here? return err; @@ -5147,6 +5217,40 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, goto compact; } + // append our mtree? + if (mdir->mid == -1 && dirty_mtree) { + lfsr_tag_t tag; + uint8_t buf[LFSR_BTREE_DSIZE]; + lfs_ssize_t d = lfsr_btree_todisk(lfs, &lfs->mtree, &tag, buf); + if (d < 0) { + return d; + } + + // TODO yeah we're going to need a wide-rm + err = lfsr_rbyd_appendall(lfs, &rbyd_, LFSR_ATTRS( + LFSR_ATTR(-1, RMMDIR, 0, NULL, 0), + LFSR_ATTR(-1, RMBTREE, 0, NULL, 0), + LFSR_ATTR_(-1, tag, 0, buf, d))); + if (err && err != LFS_ERR_RANGE) { + //TODO should we also move if there is corruption here? + return err; + } + if (err) { + goto compact; + } + } + + // finalize commit + err = lfsr_rbyd_commit(lfs, &rbyd_, NULL, 0); + if (err && err != LFS_ERR_RANGE) { + //TODO should we also move if there is corruption here? + return err; + } + if (err) { + goto compact; + } + mdir->rbyd = rbyd_; + // TODO synchronize open mdirs? // synchronize supermdir if (mdir->mid == -1 && mdir != &lfs->supermdir) { @@ -5173,7 +5277,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, // supermdirs without inlined mdirs must fit, skip the check for // compaction threshold in this case, we'll error in lfsr_rbyd_append // if we don't fit - if (!(mdir->mid < 0 && !lfsr_btree_isnull(&lfs->mtree))) { + if (!(mdir->mid < 0 && !lfsr_mtree_isinlined(lfs))) { // check if we're within our compaction threshold, otherwise we // need to split // @@ -5189,7 +5293,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, // are we inlined into the supermdir? we need to uninline // before we split, and it's possible uninlining makes the mdir // small enough that we don't even need to split - if (lfsr_btree_isnull(&lfs->mtree)) { + if (lfsr_mtree_isinlined(lfs)) { uninlining = true; // do we still need to split? @@ -5265,7 +5369,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, // note that unlining only triggers on compact, so we should never // end up id>=0 outside of a compact // - if (mdir_.mid < 0 && !lfsr_btree_isnull(&lfs->mtree) && id >= 0) { + if (mdir_.mid < 0 && !lfsr_mtree_isinlined(lfs) && id >= 0) { break; } @@ -5284,9 +5388,14 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, // upper layers should make sure this can't fail by limiting the // maximum commit size // - // take care to skip superattrs (id=-1) if we're uninlining + // take care to skip superattrs (id=-1) if we're uninlining, or only + // allow superattrs if we've uninlined and are now committing to our + // supermdir for (lfs_size_t i = 0; i < attr_count; i++) { - if (!(uninlining && attrs[i].id < 0)) { + if (!(uninlining && attrs[i].id < 0) + && !(mdir_.mid < 0 + && !lfsr_mtree_isinlined(lfs) + && attrs[i].id >= 0)) { err = lfsr_rbyd_append(lfs, &mdir_.rbyd, attrs[i].id, attrs[i].tag, attrs[i].delta, attrs[i].data); @@ -5297,6 +5406,29 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, } } + // append our mtree? + if (mdir->mid == -1 && dirty_mtree) { + lfsr_tag_t tag; + uint8_t buf[LFSR_BTREE_DSIZE]; + lfs_ssize_t d = lfsr_btree_todisk(lfs, &lfs->mtree, &tag, buf); + if (d < 0) { + return d; + } + + // TODO yeah we're going to need a wide-rm + err = lfsr_rbyd_appendall(lfs, &mdir_.rbyd, LFSR_ATTRS( + LFSR_ATTR(-1, RMMDIR, 0, NULL, 0), + LFSR_ATTR(-1, RMBTREE, 0, NULL, 0), + LFSR_ATTR_(-1, tag, 0, buf, d))); + if (err && err != LFS_ERR_RANGE) { + //TODO should we also move if there is corruption here? + return err; + } + if (err) { + goto compact; + } + } + // finalize commit err = lfsr_rbyd_commit(lfs, &mdir_.rbyd, NULL, 0); if (err) { @@ -5345,25 +5477,33 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, return err; } - // prepare commit to supermdir - lfsr_tag_t tag; - d = lfsr_btree_todisk(lfs, &lfs->mtree, &tag, recurse_buf); - if (d < 0) { - return d; - } - - // TODO yeah we're going to need a wide-rm - recurse_attrs[0] = LFSR_ATTR(-1, RMMDIR, 0, NULL, 0); - recurse_attrs[1] = LFSR_ATTR(-1, RMBTREE, 0, NULL, 0); - recurse_attrs[2] = LFSR_ATTR_(-1, tag, 0, recurse_buf, d); - attrs = recurse_attrs; - attr_count = 3; + // mark mtree as dirty and tail recurse to write it and any pending + // superattrs to the supermdir + dirty_mtree = true; continue; + +// // prepare commit to supermdir +// lfsr_tag_t tag; +// d = lfsr_btree_todisk(lfs, &lfs->mtree, &tag, recurse_buf); +// if (d < 0) { +// return d; +// } +// +// // TODO yeah we're going to need a wide-rm +// recurse_attrs[0] = LFSR_ATTR(-1, RMMDIR, 0, NULL, 0); +// recurse_attrs[1] = LFSR_ATTR(-1, RMBTREE, 0, NULL, 0); +// recurse_attrs[2] = LFSR_ATTR_(-1, tag, 0, recurse_buf, d); +// attrs = recurse_attrs; +// attr_count = 3; +// continue; } split:; // didn't fit, split mdir + // note that we should never have an mtree update here + LFS_ASSERT(!dirty_mtree); + // first figure out which id we need to split around LFS_ASSERT(lower_id > 0); lfs_ssize_t split_id = lfsr_rbyd_bisect(lfs, &mdir->rbyd, @@ -5615,19 +5755,27 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, return err; } - // prepare commit to supermdir - lfs_ssize_t d = lfsr_btree_todisk(lfs, &lfs->mtree, &tag, recurse_buf); - if (d < 0) { - return d; + // mark mtree as dirty and tail recurse to write it to the supermdir + dirty_mtree = true; + // only include superattrs if we're uninlining + if (!uninlining) { + attr_count = 0; } - - // TODO yeah we're going to need a wide-rm - recurse_attrs[0] = LFSR_ATTR(-1, RMMDIR, 0, NULL, 0); - recurse_attrs[1] = LFSR_ATTR(-1, RMBTREE, 0, NULL, 0); - recurse_attrs[2] = LFSR_ATTR_(-1, tag, 0, recurse_buf, d); - attrs = recurse_attrs; - attr_count = 3; continue; + +// // prepare commit to supermdir +// lfs_ssize_t d = lfsr_btree_todisk(lfs, &lfs->mtree, &tag, recurse_buf); +// if (d < 0) { +// return d; +// } +// +// // TODO yeah we're going to need a wide-rm +// recurse_attrs[0] = LFSR_ATTR(-1, RMMDIR, 0, NULL, 0); +// recurse_attrs[1] = LFSR_ATTR(-1, RMBTREE, 0, NULL, 0); +// recurse_attrs[2] = LFSR_ATTR_(-1, tag, 0, recurse_buf, d); +// attrs = recurse_attrs; +// attr_count = 3; +// continue; } // done @@ -5636,6 +5784,9 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, + + + /// Superblock things /// // These are all leb128s, but we can expect smaller encodings @@ -6010,7 +6161,9 @@ static int lfsr_mountinited(lfs_t *lfs) { return err; } - if (err != LFS_ERR_NOENT && id == -1) { + if (err != LFS_ERR_NOENT + && id == -1 + && lfsr_tag_suptype(tag) == LFSR_TAG_STRUCT) { if (tag != LFSR_TAG_MDIR && tag != LFSR_TAG_BTREE) { LFS_ERROR("Weird superstruct? 0x%"PRIx32, tag); return LFS_ERR_CORRUPT; diff --git a/tests/test_mtree.toml b/tests/test_mtree.toml index 001e4dfa..44f1bf7d 100644 --- a/tests/test_mtree.toml +++ b/tests/test_mtree.toml @@ -7,81 +7,170 @@ code = ''' lfsr_unmount(&lfs) => 0; ''' -# test a single supermdir with many commits -[cases.test_mtree_one_supermdir_many_commits] -defines.N = 5000 +# test a single supermdir with a custom attribute +[cases.test_mtree_one_supermdir_attr] in = 'lfs.c' code = ''' lfs_t lfs; lfsr_format(&lfs, cfg) => 0; - for (lfs_size_t i = 0; i < N; i++) { - lfsr_mount(&lfs, cfg) => 0; - lfsr_mdir_commit(&lfs, &lfs.supermdir, &(lfs_ssize_t){-1}, - NULL, 0) => 0; - lfsr_unmount(&lfs) => 0; - } + lfsr_mount(&lfs, cfg) => 0; + lfsr_mdir_commit(&lfs, &lfs.supermdir, &(lfs_ssize_t){-1}, LFSR_ATTRS( + LFSR_ATTR(-1, UATTR(1), 0, "ardvark", 7))) => 0; + + uint8_t buffer[7]; + lfsr_mdir_get(&lfs, &lfs.supermdir, + -1, LFSR_TAG_UATTR(1), buffer, 7) => 7; + assert(memcmp(buffer, "ardvark", 7) == 0); + lfsr_unmount(&lfs) => 0; lfsr_mount(&lfs, cfg) => 0; + lfsr_mdir_get(&lfs, &lfs.supermdir, + -1, LFSR_TAG_UATTR(1), buffer, 7) => 7; + assert(memcmp(buffer, "ardvark", 7) == 0); + lfsr_unmount(&lfs) => 0; +''' + +# test a single supermdir with many commits +[cases.test_mtree_one_supermdir_many_commits] +defines.N = [5, 5000] +in = 'lfs.c' +code = ''' + const char *alphas = "abcdefghijklmnopqrstuvwxyz"; + lfs_t lfs; + lfsr_format(&lfs, cfg) => 0; + + lfsr_mount(&lfs, cfg) => 0; + for (lfs_size_t i = 0; i < N; i++) { + lfsr_mdir_commit(&lfs, &lfs.supermdir, &(lfs_ssize_t){-1}, LFSR_ATTRS( + LFSR_ATTR(-1, UATTR(1), 0, &alphas[i % 26], 1))) => 0; + + uint8_t buffer[4]; + lfsr_mdir_get(&lfs, &lfs.supermdir, + -1, LFSR_TAG_UATTR(1), buffer, 4) => 1; + assert(memcmp(buffer, &alphas[i % 26], 1) == 0); + } + lfsr_unmount(&lfs) => 0; + + lfsr_mount(&lfs, cfg) => 0; + uint8_t buffer[4]; + lfsr_mdir_get(&lfs, &lfs.supermdir, + -1, LFSR_TAG_UATTR(1), buffer, 4) => 1; + assert(memcmp(buffer, &alphas[(N-1) % 26], 1) == 0); lfsr_unmount(&lfs) => 0; ''' # TODO test many supermdirs -# try creating a few entries in our mdir -[cases.test_mtree_entries] -defines.N = 5 +# try creating a range of entries that may or may not split our mtree +[cases.test_mtree_splitting] +defines.N = [5, 10, 20, 40, 80, 160, 320] in = 'lfs.c' code = ''' + const char *alphas = "abcdefghijklmnopqrstuvwxyz"; lfs_t lfs; lfsr_format(&lfs, cfg) => 0; - const char *alphas = "abcdefghijklmnopqrstuvwxyz"; lfsr_mount(&lfs, cfg) => 0; - lfsr_mdir_t mdir = lfs.supermdir; - lfs_ssize_t rid = -1; + lfsr_mdir_t mdir; + lfsr_mtree_lookup(&lfs, lfsr_mtree_weight(&lfs)-1, &mdir) => 0; + + lfs_ssize_t rid = 0; for (lfs_size_t i = 0; i < N; i++) { - rid += 1; lfsr_mdir_commit(&lfs, &mdir, &rid, LFSR_ATTRS( - LFSR_ATTR(rid, MKREG, +1, &alphas[i % 26], 1))) => 0; + LFSR_ATTR(rid, MKINLINED, +1, &alphas[i % 26], 1))) => 0; + + uint8_t buffer[4]; + lfsr_mdir_get(&lfs, &mdir, rid, LFSR_TAG_INLINED, buffer, 4) => 1; + assert(memcmp(buffer, &alphas[i % 26], 1) == 0); + + rid += 1; } lfsr_unmount(&lfs) => 0; -// TODO -// lfsr_mount(&lfs, cfg) => 0; -// for (lfs_size_t i = 0; i < N; i++) { -// uint8_t buffer[4]; -// lfsr_mdir_get(&lfs, &lfs.supermdir, i, LFSR_TAG_REG, buffer, 4) => 1; -// assert(memcmp(buffer, &alphas[i % 26], 1) == 0); -// } -// lfsr_unmount(&lfs) => 0; + lfsr_mount(&lfs, cfg) => 0; + lfs_ssize_t mid = -1; + rid = 0; + mdir = lfs.supermdir; + + for (lfs_size_t i = 0; i < N; i++) { + if (rid >= (lfs_ssize_t)mdir.rbyd.weight) { + mid += 1; + rid = 0; + lfsr_mtree_lookup(&lfs, mid, &mdir) => 0; + } + + uint8_t buffer[4]; + lfsr_mdir_get(&lfs, &mdir, rid, LFSR_TAG_INLINED, buffer, 4) => 1; + assert(memcmp(buffer, &alphas[i % 26], 1) == 0); + + rid += 1; + } + lfsr_unmount(&lfs) => 0; ''' -# try creating many entries, this should trigger a split -[cases.test_mtree_split] -defines.N = 500 +# create a range of entries, and commit to the supermdir several times, +# this makes it more likely to force uninlining without necessarily +# splitting +[cases.test_mtree_uninlining] +defines.N = [5, 10, 20, 40, 80, 160, 320] +defines.M = [5, 5000] in = 'lfs.c' code = ''' + const char *alphas = "abcdefghijklmnopqrstuvwxyz"; lfs_t lfs; lfsr_format(&lfs, cfg) => 0; - const char *alphas = "abcdefghijklmnopqrstuvwxyz"; lfsr_mount(&lfs, cfg) => 0; - lfsr_mdir_t mdir = lfs.supermdir; - lfs_ssize_t rid = -1; + lfsr_mdir_t mdir; + lfsr_mtree_lookup(&lfs, lfsr_mtree_weight(&lfs)-1, &mdir) => 0; + + lfs_ssize_t rid = 0; for (lfs_size_t i = 0; i < N; i++) { - rid += 1; lfsr_mdir_commit(&lfs, &mdir, &rid, LFSR_ATTRS( - LFSR_ATTR(rid, MKREG, +1, &alphas[i % 26], 1))) => 0; + LFSR_ATTR(rid, MKINLINED, +1, &alphas[i % 26], 1))) => 0; + + uint8_t buffer[4]; + lfsr_mdir_get(&lfs, &mdir, rid, LFSR_TAG_INLINED, buffer, 4) => 1; + assert(memcmp(buffer, &alphas[i % 26], 1) == 0); + + rid += 1; + } + + for (lfs_size_t i = 0; i < M; i++) { + lfsr_mdir_commit(&lfs, &lfs.supermdir, &(lfs_ssize_t){-1}, LFSR_ATTRS( + LFSR_ATTR(-1, UATTR(1), 0, &alphas[i % 26], 1))) => 0; + + uint8_t buffer[4]; + lfsr_mdir_get(&lfs, &lfs.supermdir, + -1, LFSR_TAG_UATTR(1), buffer, 4) => 1; + assert(memcmp(buffer, &alphas[i % 26], 1) == 0); } lfsr_unmount(&lfs) => 0; -// TODO -// lfsr_mount(&lfs, cfg) => 0; -// for (lfs_size_t i = 0; i < N; i++) { -// uint8_t buffer[4]; -// lfsr_mdir_get(&lfs, &lfs.supermdir, i, LFSR_TAG_REG, buffer, 4) => 1; -// assert(memcmp(buffer, &alphas[i % 26], 1) == 0); -// } -// lfsr_unmount(&lfs) => 0; + lfsr_mount(&lfs, cfg) => 0; + lfs_ssize_t mid = -1; + rid = 0; + mdir = lfs.supermdir; + + for (lfs_size_t i = 0; i < N; i++) { + if (rid >= (lfs_ssize_t)mdir.rbyd.weight) { + mid += 1; + rid = 0; + lfsr_mtree_lookup(&lfs, mid, &mdir) => 0; + } + + uint8_t buffer[4]; + lfsr_mdir_get(&lfs, &mdir, rid, LFSR_TAG_INLINED, buffer, 4) => 1; + assert(memcmp(buffer, &alphas[i % 26], 1) == 0); + + rid += 1; + } + + uint8_t buffer[4]; + lfsr_mdir_get(&lfs, &lfs.supermdir, + -1, LFSR_TAG_UATTR(1), buffer, 4) => 1; + assert(memcmp(buffer, &alphas[(M-1) % 26], 1) == 0); + lfsr_unmount(&lfs) => 0; ''' +