From 97f867b28d2a154c9f9a2734da92f0e7a2d2a569 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 12 Jul 2023 14:44:39 -0500 Subject: [PATCH] Added powerloss testing over lfsr_mkdir, fixed grm bugs The grm bugs were mostly issues with: 1. Not maintaining the on-disk grm state in RAM (lfs->grm) correctly, this needs to be updated correctly after every commit or littlefs gets a confused. 2. lfsr_fs_fixgrm got a bit confused when it was missed when changing the no-rm encoding from 0 to -2. Added some inline functions to help avoid this in the future. 3. Leaking information due to mixing fixed sized and variable sized encodings of the grm delta in places. This is a bit tricky to write an assert for as we don't parse the full grm when we see a no-rm grm. --- lfs.c | 47 ++++++++++++++++++++++++++++++----------- scripts/dbglfs.py | 2 +- tests/t5_dirs.toml | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 13 deletions(-) diff --git a/lfs.c b/lfs.c index ad2e0ca5..f566c1f3 100644 --- a/lfs.c +++ b/lfs.c @@ -1561,11 +1561,19 @@ static int lfsr_gdelta_xor(lfs_t *lfs, // GRM (global remove) things +static inline bool lfsr_grm_hasrm(const lfsr_grm_t *grm) { + return grm->mid != LFSR_MID_RM; +} + +static inline void lfsr_grm_clearrm(lfsr_grm_t *grm) { + grm->mid = LFSR_MID_RM; +} + static lfs_ssize_t lfsr_grm_todisk(lfs_t *lfs, const lfsr_grm_t *grm, uint8_t buffer[static LFSR_GRM_DSIZE]) { (void)lfs; // encode no-rm as zero-size - if (grm->mid == LFSR_MID_RM) { + if (!lfsr_grm_hasrm(grm)) { return 0; } @@ -1614,7 +1622,7 @@ static lfs_ssize_t lfsr_grm_fromdisk(lfs_t *lfs, lfsr_grm_t *grm, // no rm, note we accept truncated grms here if (op == 0 || d_ == 0) { - grm->mid = LFSR_MID_RM; + lfsr_grm_clearrm(grm); return 0; } @@ -1697,7 +1705,10 @@ static int lfsr_grm_split(lfs_t *lfs, grm.mid += 1; } + // zero so we don't end up with trailing garbage uint8_t buf[LFSR_GRM_DSIZE]; + memset(buf, 0, LFSR_GRM_DSIZE); + d = lfsr_grm_todisk(lfs, &grm, buf); if (d < 0) { return d; @@ -6076,11 +6087,16 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid, // update our gstate for (lfs_size_t i = 0; i < attr_count; i++) { if (attrs[i].tag == LFSR_TAG_GRM) { - int err = lfsr_grm_xor(lfs, lfs->grm, attrs[i].data); - if (err) { - return err; + // make sure to zero to avoid leaking anything + memset(lfs->grm, 0, LFSR_GRM_DSIZE); + lfs_ssize_t d = lfsr_data_read(lfs, attrs[i].data, 0, + lfs->grm, LFSR_GRM_DSIZE); + if (d < 0) { + return d; } + // TODO wait does this get triggered incorrectly if we drop? + // since weight' != weight? // TODO use bool split? // we need to fix our grm, again, if a split occured // @@ -6996,8 +7012,8 @@ static int lfsr_mountinited(lfs_t *lfs) { return d; } - if (lfs->grm_.mid >= 0) { - LFS_DEBUG("Found pending grm (0x%"PRIx32".%"PRIx32")\n", + if (lfsr_grm_hasrm(&lfs->grm_)) { + LFS_DEBUG("Found pending grm (%"PRId32".%"PRId32")", lfs->grm_.mid, lfs->grm_.rid); } @@ -7006,6 +7022,13 @@ static int lfsr_mountinited(lfs_t *lfs) { } static int lfsr_formatinited(lfs_t *lfs) { + LFS_DEBUG("Formatting littlefs v%"PRId32".%"PRId32" " + "(bs=%"PRId32", bc=%"PRId32")", + LFS_DISK_VERSION_MAJOR, + LFS_DISK_VERSION_MINOR, + lfs->cfg->block_size, + lfs->cfg->block_count); + uint8_t buf[LFSR_SUPERCONFIG_DSIZE]; lfs_ssize_t d = lfsr_superconfig_todisk(lfs, buf); if (d < 0) { @@ -7459,11 +7482,11 @@ int lfsr_dir_read(lfs_t *lfs, lfsr_dir_t *dir, struct lfs_info *info) { /// Prepare the filesystem for mutation /// static int lfsr_fs_fixgrm(lfs_t *lfs) { - LFS_ASSERT(lfs->grm_.mid != LFSR_MID_RM); + LFS_ASSERT(lfsr_grm_hasrm(&lfs->grm_)); // find our mdir lfsr_mdir_t mdir; - LFS_ASSERT((lfs_size_t)lfs->grm_.mid < lfs->mtree.weight); + LFS_ASSERT(lfs->grm_.mid < (lfs_ssize_t)lfsr_mtree_weight(lfs)); int err = lfsr_mtree_lookup(lfs, lfs->grm_.mid, &mdir); if (err) { return err; @@ -7476,14 +7499,14 @@ static int lfsr_fs_fixgrm(lfs_t *lfs) { LFSR_ATTR(-1, GRM, 0, NULL, 0))); // mark grm as taken care of - lfs->grm_.mid = 0; + lfsr_grm_clearrm(&lfs->grm_); return 0; } static int lfsr_fs_preparemutation(lfs_t *lfs) { // fix pending grms - if (lfs->grm_.mid != LFSR_MID_RM) { - LFS_DEBUG("Fixing grm (0x%"PRIx32".%"PRIx32")", + if (lfsr_grm_hasrm(&lfs->grm_)) { + LFS_DEBUG("Fixing grm (%"PRId32".%"PRId32")", lfs->grm_.mid, lfs->grm_.rid); int err = lfsr_fs_fixgrm(lfs); diff --git a/scripts/dbglfs.py b/scripts/dbglfs.py index cc1d4c1b..6c5d6df9 100755 --- a/scripts/dbglfs.py +++ b/scripts/dbglfs.py @@ -673,7 +673,7 @@ def grepr(tag, data): rid, d_ = fromleb128(data[d:]); d += d_ return 'grm %s' % ( 'none' if op == 0 - else 'rm 0x%x.%x' % (mid, rid) if op == 1 + else 'rm %d.%d' % (mid, rid) if op == 1 else '0x%x' % op) else: return 'gstate 0x%02x %d' % (tag, len(data)) diff --git a/tests/t5_dirs.toml b/tests/t5_dirs.toml index 060c8424..724d51b8 100644 --- a/tests/t5_dirs.toml +++ b/tests/t5_dirs.toml @@ -1079,6 +1079,58 @@ code = ''' lfsr_unmount(&lfs) => 0; ''' +[cases.t5_dirs_mkdir_many_pl] +defines.N = [1, 2, 4, 8, 16, 32, 64, 128, 256, 512] +reentrant = true +code = ''' + // format once per test + lfs_t lfs; + int err = lfsr_mount(&lfs, cfg); + if (err) { + lfsr_format(&lfs, cfg) => 0; + lfsr_mount(&lfs, cfg) => 0; + } + + // make this many directories + for (lfs_size_t i = 0; i < N; i++) { + char name[256]; + sprintf(name, "dir%04d", i); + int err = lfsr_mkdir(&lfs, name); + assert(!err || err == LFS_ERR_EXIST); + } + + // check that our mkdir worked + for (lfs_size_t i = 0; i < N; i++) { + char name[256]; + sprintf(name, "dir%04d", i); + struct lfs_info info; + lfsr_stat(&lfs, name, &info) => 0; + assert(strcmp(info.name, name) == 0); + assert(info.type == LFS_TYPE_DIR); + } + + lfsr_dir_t dir; + lfsr_dir_open(&lfs, &dir, "/") => 0; + struct lfs_info info; + lfsr_dir_read(&lfs, &dir, &info) => 0; + assert(strcmp(info.name, ".") == 0); + assert(info.type == LFS_TYPE_DIR); + lfsr_dir_read(&lfs, &dir, &info) => 0; + assert(strcmp(info.name, "..") == 0); + assert(info.type == LFS_TYPE_DIR); + for (lfs_size_t i = 0; i < N; i++) { + char name[256]; + sprintf(name, "dir%04d", i); + lfsr_dir_read(&lfs, &dir, &info) => 0; + assert(strcmp(info.name, name) == 0); + assert(info.type == LFS_TYPE_DIR); + } + lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT; + + lfsr_unmount(&lfs) => 0; +''' + + #[cases.test_dirs_root]