From 2a4aadca0ea76cc4e690f5b03f6a9000bcb8961a Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 21 Nov 2023 02:33:56 -0600 Subject: [PATCH] Fixed a number of bshrub-related alloc/clobber test failures - There was a lingering strict pcache assert in lfs_bd_erase. Very unlikely to hit, but it is possible and shouldn't be an assert now that pcache can be left in an arbitrary state. That being said, it was asserting on an actual bug in this case. - Our btree traversal was not traversing the roots of zero-weight btrees. Zero-weight btrees can happen as an intermediary step during btree/bshrub carving. If the stars align with the block allocator and intermediary carving states this can cause incorrect block allocations. - Staged updates to bsprouts/bshrubs need to be played out before updates to opened mdirs lfsr_mdir_commit, this is just because lfsr_file_isbsprout/isbshrub depend on mdir.block and updating the mdirs first corrupts this. Maybe a different organization to this code would be useful, it is already full of TODOs. --- lfs.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/lfs.c b/lfs.c index eddbe6b9..012f02ca 100644 --- a/lfs.c +++ b/lfs.c @@ -352,7 +352,6 @@ static int lfs_bd_erase(lfs_t *lfs, lfs_block_t block) { LFS_ASSERT(block < lfs->cfg->block_count); // make sure any caches are outdated appropriately here - LFS_ASSERT(lfs->pcache.block != block); if (lfs->rcache.block == block) { lfs_cache_drop(lfs, &lfs->rcache); } @@ -4644,7 +4643,9 @@ static int lfsr_btree_traversalread(lfs_t *lfs, const lfsr_btree_t *btree, lfsr_binfo_t *binfo) { while (true) { // in range? - if (btraversal->bid >= (lfsr_bid_t)btree->weight) { + if (btraversal->bid >= (lfsr_bid_t)btree->weight + // make sure we traverse the root even if weight=0 + && btraversal->branch.trunk != 0) { return LFS_ERR_NOENT; } @@ -4654,6 +4655,7 @@ static int lfsr_btree_traversalread(lfs_t *lfs, const lfsr_btree_t *btree, btraversal->rid = btraversal->bid; btraversal->branch = *btree; + // traverse the root if (btraversal->rid == 0) { binfo->bid = btree->weight-1; binfo->tag = LFSR_TAG_BRANCH; @@ -6310,6 +6312,21 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, } } + // update any staged bsprout/bshrub changes, note the order here + // matters since issprout/isbshrub depends on file mdir + // + // TODO merge with below? maybe? + for (lfsr_openedmdir_t *opened = lfs->opened[LFS_TYPE_REG-LFS_TYPE_REG]; + opened; + opened = opened->next) { + lfsr_file_t *file = (lfsr_file_t*)opened; + if (lfsr_file_isbsprout(file)) { + file->u.bsprout.data = file->u.bsprout.data_; + } else if (lfsr_file_isbshrub(file)) { + file->u.bshrub.rbyd = file->u.bshrub.rbyd_; + } + } + // update any opened mdirs for (int type = LFS_TYPE_REG; type < LFS_TYPE_REG+3; type++) { for (lfsr_openedmdir_t *opened = lfs->opened[type-LFS_TYPE_REG]; @@ -6420,20 +6437,6 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, } } - // update any staged bsprout/bshrub changes - // - // TODO merge with above? maybe? - for (lfsr_openedmdir_t *opened = lfs->opened[LFS_TYPE_REG-LFS_TYPE_REG]; - opened; - opened = opened->next) { - lfsr_file_t *file = (lfsr_file_t*)opened; - if (lfsr_file_isbsprout(file)) { - file->u.bsprout.data = file->u.bsprout.data_; - } else if (lfsr_file_isbshrub(file)) { - file->u.bshrub.rbyd = file->u.bshrub.rbyd_; - } - } - // update mdir to follow requested rid if (mid != -1 && msibling_.u.m.weight > 0