Github/littlefs
1
Fork 0
Code Actions Activity

Slightly reworked mdir staging in in lfs3_mdir_commit_

Browse Source 
I've noticed a common pattern where we tend to create copies in multiple
function frames in order to allow fallback in case of errors. This risks
redundant stack allocations across layers.

To avoid this, this commit adopts old + staging arguments for most of
the internal mdir commit functions:

  static int lfs3_mdir_commit_(lfs3_t *lfs3,
          lfs3_mdir_t *mdir_, lfs3_mdir_t *mdir,
          ...);

We already needed this for lfs3_mdir_compact__, so hey, points for
consistency.

Saves a tiny bit of code:

           code          stack          ctx
  before: 37964           2424          636
  after:  37936 (-0.1%)   2424 (+0.0%)  636 (+0.0%)
This commit is contained in:
Christopher Haster
2025-06-29 02:13:44 -05:00
parent 4747477057
commit 8ee08a5b89
1 changed files with 59 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs3.c
+59 -70
View File
@@ -8123,7 +8123,7 @@ static int lfs3_mdir_swap__(lfs3_t *lfs3, lfs3_mdir_t *mdir_,
// low-level mdir commit, does not handle mtree/mlist/compaction/etc // low-level mdir commit, does not handle mtree/mlist/compaction/etc
#ifndef LFS3_RDONLY #ifndef LFS3_RDONLY
static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir, static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir_,
lfs3_srid_t start_rid, lfs3_srid_t end_rid, lfs3_srid_t start_rid, lfs3_srid_t end_rid,
lfs3_smid_t mid, const lfs3_rattr_t *rattrs, lfs3_size_t rattr_count) { lfs3_smid_t mid, const lfs3_rattr_t *rattrs, lfs3_size_t rattr_count) {
// since we only ever commit to one mid or split, we can ignore the // since we only ever commit to one mid or split, we can ignore the
@@ -8164,14 +8164,14 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
// reset shrub if it doesn't live in our block, this happens // reset shrub if it doesn't live in our block, this happens
// when converting from a btree // when converting from a btree
if (!lfs3_bshrub_isbshrub(bshrub_)) { if (!lfs3_bshrub_isbshrub(bshrub_)) {
bshrub_->shrub_.blocks[0] = mdir->rbyd.blocks[0]; bshrub_->shrub_.blocks[0] = mdir_->rbyd.blocks[0];
bshrub_->shrub_.trunk = LFS3_RBYD_ISSHRUB | 0; bshrub_->shrub_.trunk = LFS3_RBYD_ISSHRUB | 0;
bshrub_->shrub_.weight = 0; bshrub_->shrub_.weight = 0;
} }
// commit to shrub // commit to shrub
int err = lfs3_shrub_commit(lfs3, int err = lfs3_shrub_commit(lfs3,
&mdir->rbyd, &bshrub_->shrub_, &mdir_->rbyd, &bshrub_->shrub_,
rid_, rattrs_, rattr_count_); rid_, rattrs_, rattr_count_);
if (err) { if (err) {
return err; return err;
@@ -8212,14 +8212,14 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
} }
// compact our shrub // compact our shrub
err = lfs3_shrub_compact(lfs3, &mdir->rbyd, &shrub, err = lfs3_shrub_compact(lfs3, &mdir_->rbyd, &shrub,
&shrub); &shrub);
if (err) { if (err) {
return err; return err;
} }
// write our new shrub tag // write our new shrub tag
err = lfs3_rbyd_appendrattr(lfs3, &mdir->rbyd, err = lfs3_rbyd_appendrattr(lfs3, &mdir_->rbyd,
rid - lfs3_smax(start_rid, 0), rid - lfs3_smax(start_rid, 0),
LFS3_RATTR_SHRUB(LFS3_TAG_BSHRUB, 0, &shrub)); LFS3_RATTR_SHRUB(LFS3_TAG_BSHRUB, 0, &shrub));
if (err) { if (err) {
@@ -8228,7 +8228,7 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
// append the rattr // append the rattr
} else { } else {
err = lfs3_rbyd_appendrattr(lfs3, &mdir->rbyd, err = lfs3_rbyd_appendrattr(lfs3, &mdir_->rbyd,
rid - lfs3_smax(start_rid, 0), rid - lfs3_smax(start_rid, 0),
LFS3_RATTR_DATA(tag, 0, &data)); LFS3_RATTR_DATA(tag, 0, &data));
if (err) { if (err) {
@@ -8248,8 +8248,8 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
// only compact once, first compact should // only compact once, first compact should
// stage the new block // stage the new block
&& ((lfs3_bshrub_t*)o)->shrub_.blocks[0] && ((lfs3_bshrub_t*)o)->shrub_.blocks[0]
!= mdir->rbyd.blocks[0]) { != mdir_->rbyd.blocks[0]) {
int err = lfs3_shrub_compact(lfs3, &mdir->rbyd, int err = lfs3_shrub_compact(lfs3, &mdir_->rbyd,
&((lfs3_bshrub_t*)o)->shrub_, &((lfs3_bshrub_t*)o)->shrub_,
&((lfs3_bshrub_t*)o)->shrub); &((lfs3_bshrub_t*)o)->shrub);
if (err) { if (err) {
@@ -8272,7 +8272,7 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
// first lets check if the attr changed, we don't want // first lets check if the attr changed, we don't want
// to append attrs unless we have to // to append attrs unless we have to
lfs3_data_t data; lfs3_data_t data;
int err = lfs3_mdir_lookup(lfs3, mdir, int err = lfs3_mdir_lookup(lfs3, mdir_,
LFS3_TAG_ATTR(attrs_[j].type), LFS3_TAG_ATTR(attrs_[j].type),
NULL, &data); NULL, &data);
if (err && err != LFS3_ERR_NOENT) { if (err && err != LFS3_ERR_NOENT) {
@@ -8291,7 +8291,7 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
} }
// append the custom attr // append the custom attr
err = lfs3_rbyd_appendrattr(lfs3, &mdir->rbyd, err = lfs3_rbyd_appendrattr(lfs3, &mdir_->rbyd,
rid - lfs3_smax(start_rid, 0), rid - lfs3_smax(start_rid, 0),
// removing or updating? // removing or updating?
(lfs3_attr_isnoattr(&attrs_[j])) (lfs3_attr_isnoattr(&attrs_[j]))
@@ -8311,7 +8311,7 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
} else { } else {
LFS3_ASSERT(!lfs3_tag_isinternal(rattrs[i].tag)); LFS3_ASSERT(!lfs3_tag_isinternal(rattrs[i].tag));
int err = lfs3_rbyd_appendrattr(lfs3, &mdir->rbyd, int err = lfs3_rbyd_appendrattr(lfs3, &mdir_->rbyd,
rid - lfs3_smax(start_rid, 0), rid - lfs3_smax(start_rid, 0),
rattrs[i]); rattrs[i]);
if (err) { if (err) {
@@ -8329,9 +8329,10 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
// If we finish the commit it becomes immediately visible, but we really // If we finish the commit it becomes immediately visible, but we really
// need to atomically remove this mdir from the mtree. Leave the actual // need to atomically remove this mdir from the mtree. Leave the actual
// remove up to upper layers. // remove up to upper layers.
if (mdir->rbyd.weight == 0 if (mdir_->rbyd.weight == 0
// unless we are an mroot // unless we are an mroot
&& !(mdir->mid == -1 || lfs3_mdir_cmp(mdir, &lfs3->mroot) == 0)) { && !(mdir_->mid == -1
|| lfs3_mdir_cmp(mdir_, &lfs3->mroot) == 0)) {
// note! we can no longer read from this mdir as our pcache may // note! we can no longer read from this mdir as our pcache may
// be clobbered // be clobbered
return LFS3_ERR_NOENT; return LFS3_ERR_NOENT;
@@ -8339,7 +8340,7 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
// append any gstate? // append any gstate?
if (start_rid <= -2) { if (start_rid <= -2) {
int err = lfs3_rbyd_appendgdelta(lfs3, &mdir->rbyd); int err = lfs3_rbyd_appendgdelta(lfs3, &mdir_->rbyd);
if (err) { if (err) {
return err; return err;
} }
@@ -8349,24 +8350,24 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
// //
// note this is before we calculate gcksumdelta, otherwise // note this is before we calculate gcksumdelta, otherwise
// everything would get all self-referential // everything would get all self-referential
uint32_t cksum = mdir->rbyd.cksum; uint32_t cksum = mdir_->rbyd.cksum;
// append gkcsumdelta? // append gkcsumdelta?
if (start_rid <= -2) { if (start_rid <= -2) {
// figure out changes to our gcksumdelta // figure out changes to our gcksumdelta
mdir->gcksumdelta ^= lfs3_crc32c_cube(lfs3->gcksum_p) mdir_->gcksumdelta ^= lfs3_crc32c_cube(lfs3->gcksum_p)
^ lfs3_crc32c_cube(lfs3->gcksum ^ cksum) ^ lfs3_crc32c_cube(lfs3->gcksum ^ cksum)
^ lfs3->gcksum_d; ^ lfs3->gcksum_d;
int err = lfs3_rbyd_appendrattr_(lfs3, &mdir->rbyd, LFS3_RATTR_LE32( int err = lfs3_rbyd_appendrattr_(lfs3, &mdir_->rbyd, LFS3_RATTR_LE32(
LFS3_TAG_GCKSUMDELTA, 0, mdir->gcksumdelta)); LFS3_TAG_GCKSUMDELTA, 0, mdir_->gcksumdelta));
if (err) { if (err) {
return err; return err;
} }
} }
// finalize commit // finalize commit
int err = lfs3_rbyd_appendcksum_(lfs3, &mdir->rbyd, cksum); int err = lfs3_rbyd_appendcksum_(lfs3, &mdir_->rbyd, cksum);
if (err) { if (err) {
return err; return err;
} }
@@ -8374,7 +8375,7 @@ static int lfs3_mdir_commit__(lfs3_t *lfs3, lfs3_mdir_t *mdir,
// success? // success?
// xor our new cksum // xor our new cksum
lfs3->gcksum ^= mdir->rbyd.cksum; lfs3->gcksum ^= mdir_->rbyd.cksum;
return 0; return 0;
} }
@@ -8499,8 +8500,9 @@ static lfs3_ssize_t lfs3_mdir_estimate__(lfs3_t *lfs3, const lfs3_mdir_t *mdir,
#endif #endif
#ifndef LFS3_RDONLY #ifndef LFS3_RDONLY
static int lfs3_mdir_compact__(lfs3_t *lfs3, lfs3_mdir_t *mdir_, static int lfs3_mdir_compact__(lfs3_t *lfs3,
const lfs3_mdir_t *mdir, lfs3_srid_t start_rid, lfs3_srid_t end_rid) { lfs3_mdir_t *mdir_, const lfs3_mdir_t *mdir,
lfs3_srid_t start_rid, lfs3_srid_t end_rid) {
// this is basically the same as lfs3_rbyd_compact, but with special // this is basically the same as lfs3_rbyd_compact, but with special
// handling for inlined trees. // handling for inlined trees.
// //
@@ -8613,17 +8615,27 @@ static int lfs3_mdir_compact__(lfs3_t *lfs3, lfs3_mdir_t *mdir_,
// mid-level mdir commit, this one will at least compact on overflow // mid-level mdir commit, this one will at least compact on overflow
#ifndef LFS3_RDONLY #ifndef LFS3_RDONLY
static int lfs3_mdir_commit_(lfs3_t *lfs3, lfs3_mdir_t *mdir, static int lfs3_mdir_commit_(lfs3_t *lfs3,
lfs3_mdir_t *mdir_, lfs3_mdir_t *mdir,
lfs3_srid_t start_rid, lfs3_srid_t end_rid, lfs3_srid_t start_rid, lfs3_srid_t end_rid,
lfs3_srid_t *split_rid_, lfs3_srid_t *split_rid_,
lfs3_smid_t mid, const lfs3_rattr_t *rattrs, lfs3_size_t rattr_count) { lfs3_smid_t mid, const lfs3_rattr_t *rattrs, lfs3_size_t rattr_count) {
// make a copy // make a copy
lfs3_mdir_t mdir_ = *mdir; *mdir_ = *mdir;
// mark as erased in case of failure // mark our mdir as unerased in case we fail
lfs3_mdir_claim(mdir); lfs3_mdir_claim(mdir);
// mark any copies of our mdir as unerased in case we fail
if (lfs3_mdir_cmp(mdir, &lfs3->mroot) == 0) {
lfs3_mdir_claim(&lfs3->mroot);
}
for (lfs3_omdir_t *o = lfs3->omdirs; o; o = o->next) {
if (lfs3_mdir_cmp(&o->mdir, mdir) == 0) {
lfs3_mdir_claim(&o->mdir);
}
}
// try to commit // try to commit
int err = lfs3_mdir_commit__(lfs3, &mdir_, start_rid, end_rid, int err = lfs3_mdir_commit__(lfs3, mdir_, start_rid, end_rid,
mid, rattrs, rattr_count); mid, rattrs, rattr_count);
if (err) { if (err) {
if (err == LFS3_ERR_RANGE || err == LFS3_ERR_CORRUPT) { if (err == LFS3_ERR_RANGE || err == LFS3_ERR_CORRUPT) {
@@ -8631,9 +8643,6 @@ static int lfs3_mdir_commit_(lfs3_t *lfs3, lfs3_mdir_t *mdir,
} }
return err; return err;
} }
// update mdir
*mdir = mdir_;
return 0; return 0;
compact:; compact:;
@@ -8642,7 +8651,8 @@ compact:;
bool overrecyclable = true; bool overrecyclable = true;
// check if we're within our compaction threshold // check if we're within our compaction threshold
lfs3_ssize_t estimate = lfs3_mdir_estimate__(lfs3, mdir, start_rid, end_rid, lfs3_ssize_t estimate = lfs3_mdir_estimate__(lfs3, mdir,
start_rid, end_rid,
split_rid_); split_rid_);
if (estimate < 0) { if (estimate < 0) {
return estimate; return estimate;
@@ -8654,7 +8664,7 @@ compact:;
} }
// swap blocks, increment revision count // swap blocks, increment revision count
err = lfs3_mdir_swap__(lfs3, &mdir_, mdir, false); err = lfs3_mdir_swap__(lfs3, mdir_, mdir, false);
if (err) { if (err) {
if (err == LFS3_ERR_NOSPC || err == LFS3_ERR_CORRUPT) { if (err == LFS3_ERR_NOSPC || err == LFS3_ERR_CORRUPT) {
overrecyclable &= (err != LFS3_ERR_CORRUPT); overrecyclable &= (err != LFS3_ERR_CORRUPT);
@@ -8670,7 +8680,7 @@ compact:;
"-> 0x{%"PRIx32",%"PRIx32"}", "-> 0x{%"PRIx32",%"PRIx32"}",
lfs3_dbgmbid(lfs3, mdir->mid), lfs3_dbgmbid(lfs3, mdir->mid),
mdir->rbyd.blocks[0], mdir->rbyd.blocks[1], mdir->rbyd.blocks[0], mdir->rbyd.blocks[1],
mdir_.rbyd.blocks[0], mdir_.rbyd.blocks[1]); mdir_->rbyd.blocks[0], mdir_->rbyd.blocks[1]);
#endif #endif
// don't copy over gcksum if relocating // don't copy over gcksum if relocating
@@ -8680,7 +8690,7 @@ compact:;
} }
// compact our mdir // compact our mdir
err = lfs3_mdir_compact__(lfs3, &mdir_, mdir, start_rid_, end_rid); err = lfs3_mdir_compact__(lfs3, mdir_, mdir, start_rid_, end_rid);
if (err) { if (err) {
LFS3_ASSERT(err != LFS3_ERR_RANGE); LFS3_ASSERT(err != LFS3_ERR_RANGE);
// bad prog? try another block // bad prog? try another block
@@ -8695,7 +8705,7 @@ compact:;
// //
// upper layers should make sure this can't fail by limiting the // upper layers should make sure this can't fail by limiting the
// maximum commit size // maximum commit size
err = lfs3_mdir_commit__(lfs3, &mdir_, start_rid_, end_rid, err = lfs3_mdir_commit__(lfs3, mdir_, start_rid_, end_rid,
mid, rattrs, rattr_count); mid, rattrs, rattr_count);
if (err) { if (err) {
LFS3_ASSERT(err != LFS3_ERR_RANGE); LFS3_ASSERT(err != LFS3_ERR_RANGE);
@@ -8711,14 +8721,12 @@ compact:;
if (relocated) { if (relocated) {
lfs3->gcksum_d ^= mdir->gcksumdelta; lfs3->gcksum_d ^= mdir->gcksumdelta;
} }
// update mdir
*mdir = mdir_;
return 0; return 0;
relocate:; relocate:;
#ifndef LFS3_2BONLY #ifndef LFS3_2BONLY
// needs relocation? bad prog? ok, try allocating a new mdir // needs relocation? bad prog? ok, try allocating a new mdir
err = lfs3_mdir_alloc__(lfs3, &mdir_, mdir->mid, relocated); err = lfs3_mdir_alloc__(lfs3, mdir_, mdir->mid, relocated);
if (err && !(err == LFS3_ERR_NOSPC && overrecyclable)) { if (err && !(err == LFS3_ERR_NOSPC && overrecyclable)) {
return err; return err;
} }
@@ -8733,7 +8741,7 @@ compact:;
relocated = false; relocated = false;
overrecyclable = false; overrecyclable = false;
err = lfs3_mdir_swap__(lfs3, &mdir_, mdir, true); err = lfs3_mdir_swap__(lfs3, mdir_, mdir, true);
if (err) { if (err) {
// bad prog? can't do much here, mdir stuck // bad prog? can't do much here, mdir stuck
if (err == LFS3_ERR_CORRUPT) { if (err == LFS3_ERR_CORRUPT) {
@@ -8863,24 +8871,11 @@ static int lfs3_mdir_commit(lfs3_t *lfs3, lfs3_mdir_t *mdir,
} }
} }
// create a copy
lfs3_mdir_t mdir_[2];
mdir_[0] = *mdir;
// mark our mdir as unerased in case we fail
lfs3_mdir_claim(mdir);
// mark any copies of our mdir as unerased in case we fail
if (lfs3_mdir_cmp(mdir, &lfs3->mroot) == 0) {
lfs3_mdir_claim(&lfs3->mroot);
}
for (lfs3_omdir_t *o = lfs3->omdirs; o; o = o->next) {
if (lfs3_mdir_cmp(&o->mdir, mdir) == 0) {
lfs3_mdir_claim(&o->mdir);
}
}
// attempt to commit/compact the mdir normally // attempt to commit/compact the mdir normally
lfs3_mdir_t mdir_[2];
lfs3_srid_t split_rid; lfs3_srid_t split_rid;
int err = lfs3_mdir_commit_(lfs3, &mdir_[0], -2, -1, &split_rid, int err = lfs3_mdir_commit_(lfs3, &mdir_[0], mdir, -2, -1,
&split_rid,
mdir->mid, rattrs, rattr_count); mdir->mid, rattrs, rattr_count);
if (err && err != LFS3_ERR_RANGE if (err && err != LFS3_ERR_RANGE
&& err != LFS3_ERR_NOENT) { && err != LFS3_ERR_NOENT) {
@@ -9162,19 +9157,12 @@ static int lfs3_mdir_commit(lfs3_t *lfs3, lfs3_mdir_t *mdir,
lfs3->gcksum ^= lfs3->mroot.rbyd.cksum; lfs3->gcksum ^= lfs3->mroot.rbyd.cksum;
} }
// mark any copies of our mroot as unerased
lfs3_mdir_claim(&lfs3->mroot);
for (lfs3_omdir_t *o = lfs3->omdirs; o; o = o->next) {
if (lfs3_mdir_cmp(&o->mdir, &lfs3->mroot) == 0) {
lfs3_mdir_claim(&o->mdir);
}
}
// commit new mtree into our mroot // commit new mtree into our mroot
// //
// note end_rid=0 here will delete any files leftover from a split // note end_rid=0 here will delete any files leftover from a split
// in our mroot // in our mroot
err = lfs3_mdir_commit_(lfs3, &mroot_, -2, 0, NULL, err = lfs3_mdir_commit_(lfs3, &mroot_, &lfs3->mroot, -2, 0,
NULL,
-1, LFS3_RATTRS( -1, LFS3_RATTRS(
LFS3_RATTR_BTREE( LFS3_RATTR_BTREE(
LFS3_TAG_MASK8 | LFS3_TAG_MTREE, 0, LFS3_TAG_MASK8 | LFS3_TAG_MTREE, 0,
@@ -9198,9 +9186,9 @@ static int lfs3_mdir_commit(lfs3_t *lfs3, lfs3_mdir_t *mdir,
while (lfs3_mdir_cmp(&mrootchild_, &mrootchild) != 0 while (lfs3_mdir_cmp(&mrootchild_, &mrootchild) != 0
&& !lfs3_mdir_ismrootanchor(&mrootchild)) { && !lfs3_mdir_ismrootanchor(&mrootchild)) {
// find the mroot's parent // find the mroot's parent
lfs3_mdir_t mrootparent_; lfs3_mdir_t mrootparent;
err = lfs3_mroot_parent(lfs3, mrootchild.rbyd.blocks, err = lfs3_mroot_parent(lfs3, mrootchild.rbyd.blocks,
&mrootparent_); &mrootparent);
if (err) { if (err) {
LFS3_ASSERT(err != LFS3_ERR_NOENT); LFS3_ASSERT(err != LFS3_ERR_NOENT);
goto failed; goto failed;
@@ -9211,8 +9199,6 @@ static int lfs3_mdir_commit(lfs3_t *lfs3, lfs3_mdir_t *mdir,
mrootchild.rbyd.blocks[0], mrootchild.rbyd.blocks[1], mrootchild.rbyd.blocks[0], mrootchild.rbyd.blocks[1],
mrootchild_.rbyd.blocks[0], mrootchild_.rbyd.blocks[1]); mrootchild_.rbyd.blocks[0], mrootchild_.rbyd.blocks[1]);
mrootchild = mrootparent_;
// make sure mtree/mroot changes are on-disk before committing // make sure mtree/mroot changes are on-disk before committing
// metadata // metadata
err = lfs3_bd_sync(lfs3); err = lfs3_bd_sync(lfs3);
@@ -9220,11 +9206,13 @@ static int lfs3_mdir_commit(lfs3_t *lfs3, lfs3_mdir_t *mdir,
goto failed; goto failed;
} }
// xor mrootchild's cksum // xor mrootparent's cksum
lfs3->gcksum ^= mrootparent_.rbyd.cksum; lfs3->gcksum ^= mrootparent.rbyd.cksum;
// commit mrootchild // commit mrootchild
err = lfs3_mdir_commit_(lfs3, &mrootparent_, -2, -1, NULL, lfs3_mdir_t mrootparent_;
err = lfs3_mdir_commit_(lfs3, &mrootparent_, &mrootparent, -2, -1,
NULL,
-1, LFS3_RATTRS( -1, LFS3_RATTRS(
LFS3_RATTR_MPTR( LFS3_RATTR_MPTR(
LFS3_TAG_MROOT, 0, LFS3_TAG_MROOT, 0,
@@ -9235,6 +9223,7 @@ static int lfs3_mdir_commit(lfs3_t *lfs3, lfs3_mdir_t *mdir,
goto failed; goto failed;
} }
mrootchild = mrootparent;
mrootchild_ = mrootparent_; mrootchild_ = mrootparent_;
} }