Skip to content

Commit

Permalink
Fixed a number of bshrub-related alloc/clobber test failures
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
geky committed Nov 21, 2023
1 parent 4793d2f commit 2a4aadc
Showing 1 changed file with 19 additions and 16 deletions.
35 changes: 19 additions & 16 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 2a4aadc

Please sign in to comment.