Github/littlefs
1
Fork 0
Code Actions Activity

Don't duplicate config across all mroots, only magic

Browse Source 
So, duplicating the config across multiple mroots allows a better
chance of manual recovery if things go wrong, right?

   .--------.  .--------.
  .|littlefs|->|littlefs|
  ||bs=4096 | ||bs=4096 |
  ||bc=256  | ||bc=256  |
  ||crc32c  | ||root dir|
  ||        | ||crc32c  |
  |'--------' |'--------'
  '--------'  '--------'

Well, this was the original thinking. But now I'm starting to think the
duplicated config isn't actually all that useful:

1. The config may be out-of-date, since only the last mroot is mutable.

   This is becoming more common as littlefs evolves (on-disk version
   bumps, compat flag changes, fs_grow, etc).

2. The most important bit of information is where the mtree is. And this
   information is _only_ available on the last mroot because of
   mutability requirements.

   At the very least, all mroots point to the mtree (not Rome). So
   finding the mtree may not be that hard if you find any mroot. But
   this also gives you all the config sooo...

3. gstate is going to be hard (impossible?) to reconstruct anyways.
   Though for reading this may only be an issue for interrupted grms.

4. Let's be honest, manual recovery is not going to be a common
   occurence for these devices.

Point 1. is a the main issue and actually highlights a real risk with
duplicated config: It's easy to pick up out-of-date config.

In other implementations this risks easy mistakes that are hard to
notice until complex filesystem states. Assuming the first mroot
contains up-to-date config for the obvious example.

Even in our current implementation, duplicated config already poses some
tricky hard-to-get-right problems. What happens if we run into an
unknown compat flag on a not-last mroot? Hint, our implementation was
broken!

---

So this commit changes mroot extension to _not_ duplicate config, but
instead just rewrite the magic string and mroot chain to the new mroot
anchor:

   .--------.  .--------.
  .|littlefs|->|littlefs|
  ||crc32c  | ||bs=4096 |
  ||        | ||bc=256  |
  ||        | ||root dir|
  ||        | ||crc32c  |
  |'--------' |'--------'
  '--------'  '--------'

This leads to a nice simplification in lfsr_mdir_commit, so that's a
plus.

It does make lfsr_mount a bit trickier, since we need to figure out which
mroot is the last mroot before checking for config. But we would need
something trickier in lfsr_mount anyways to handle the above out-of-date
issues anyways.

The current implementation just does a redundant mroot lookup to figure
out if the current mroot is the last mroot. In theory this could be
avoided, but I couldn't figure out how to without making the code
unreasonably complex (lfsr_mount is already intertwined with
lfsr_traversal_read).

The end result is a bit of code and stack savings, thanks to
lfsr_mdir_commit being on the stack-depth hot-path (deep-path?):

           code          stack
  before: 34060           2880
  after:  33996 (-0.2%)   2864 (-0.6%)

It's also worth noting that there are plans to add block-level
redundancy at some point. Maybe it's best to leave recovering from
missing blocks to block-level redundancy which is actually designed for
this, and let the mroot chain do what the mroot chain does best:
allowing the mroots to participate in wear-leveling.
This commit is contained in:
Christopher Haster
2024-03-20 00:38:54 -05:00
parent c71725d627
commit 9cf115685b
1 changed files with 43 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+43 -71
View File
@@ -6806,52 +6806,20 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
mrootchild.rbyd.blocks[0], mrootchild.rbyd.blocks[1],
mrootchild_.rbyd.blocks[0], mrootchild_.rbyd.blocks[1]);
// compact into the new mroot anchor
// commit the new mroot anchor
lfsr_mdir_t mrootanchor_;
err = lfsr_mdir_swap__(lfs, &mrootanchor_, &mrootchild, -1);
if (err) {
return err;
}
// copy only the config over
lfsr_tag_t tag = 0;
while (true) {
lfsr_srid_t rid;
lfsr_rid_t weight;
lfsr_data_t data;
err = lfsr_rbyd_lookupnext(lfs, &mrootchild.rbyd,
-1, tag+1,
&rid, &tag, &weight, &data);
if (err) {
if (err == LFS_ERR_NOENT) {
break;
}
return err;
}
if (rid != -1 || lfsr_tag_suptype(tag) != LFSR_TAG_CONFIG) {
break;
}
// write the tag
err = lfsr_rbyd_appendcompactattr(lfs, &mrootanchor_.rbyd,
tag, weight, data);
if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE);
return err;
}
}
err = lfsr_rbyd_appendcompaction(lfs, &mrootanchor_.rbyd, 0);
if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE);
return err;
}
// and commit our new mroot
err = lfsr_mdir_commit__(lfs, &mrootanchor_, -1, -1,
-1, LFSR_ATTRS(
LFSR_ATTR(
LFSR_TAG_SUB | LFSR_TAG_MROOT, 0,
LFSR_TAG_MAGIC, 0,
LFSR_DATA_BUF("littlefs", 8)),
LFSR_ATTR(
LFSR_TAG_MROOT, 0,
LFSR_DATA_FROMMPTR(lfsr_mdir_mptr(&mrootchild_)))));
if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE);
@@ -7942,31 +7910,10 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg);
static int lfs_deinit(lfs_t *lfs);
static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
// has magic string?
lfsr_data_t data;
int err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_MAGIC,
&data);
if (err) {
if (err == LFS_ERR_NOENT) {
LFS_ERROR("No littlefs magic found");
return LFS_ERR_INVAL;
}
return err;
}
lfs_scmp_t cmp = lfsr_data_cmp(lfs, data, "littlefs", 8);
if (cmp < 0) {
return cmp;
}
// treat corrupted magic as no magic
if (cmp != LFS_CMP_EQ) {
LFS_ERROR("No littlefs magic found");
return LFS_ERR_INVAL;
}
// check the disk version
err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_VERSION,
uint8_t version[2] = {0, 0};
lfsr_data_t data;
int err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_VERSION,
&data);
if (err) {
if (err == LFS_ERR_NOENT) {
@@ -7975,8 +7922,6 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
}
return err;
}
uint8_t version[2] = {0, 0};
lfs_ssize_t d = lfsr_data_read(lfs, &data, version, 2);
if (d < 0) {
return err;
@@ -8008,7 +7953,6 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
}
}
// incompatible rcompat flags?
if (lfsr_rcompat_isincompat(rcompat)) {
LFS_ERROR("Incompatible rcompat flags 0x%0"PRIx16
" (!= 0x%0"PRIx16")",
@@ -8032,7 +7976,6 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
}
}
// incompatible wcompat flags?
// TODO switch to readonly?
if (lfsr_wcompat_isincompat(wcompat)) {
LFS_ERROR("Incompatible wcompat flags 0x%0"PRIx16
@@ -8046,6 +7989,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
// ignore these anyways
// check the on-disk geometry
lfsr_geometry_t geometry;
err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_GEOMETRY,
&data);
if (err) {
@@ -8055,8 +7999,6 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
}
return err;
}
lfsr_geometry_t geometry;
err = lfsr_data_readgeometry(lfs, &data, &geometry);
if (err) {
return err;
@@ -8174,14 +8116,44 @@ static int lfsr_mountinited(lfs_t *lfs) {
if (tinfo.tag == LFSR_TAG_MDIR) {
// found an mroot?
if (tinfo.u.mdir.mid == -1) {
err = lfsr_mountmroot(lfs, &tinfo.u.mdir);
// check for the magic string, all mroot should have this
lfsr_data_t data;
int err = lfsr_mdir_lookup(lfs, &tinfo.u.mdir, LFSR_TAG_MAGIC,
&data);
if (err) {
if (err == LFS_ERR_NOENT) {
LFS_ERROR("No littlefs magic found");
return LFS_ERR_INVAL;
}
return err;
}
// keep track of the last mroot we see, this is the
// active mroot
lfs->mroot = tinfo.u.mdir;
// treat corrupted magic as no magic
lfs_scmp_t cmp = lfsr_data_cmp(lfs, data, "littlefs", 8);
if (cmp < 0) {
return cmp;
}
if (cmp != LFS_CMP_EQ) {
LFS_ERROR("No littlefs magic found");
return LFS_ERR_INVAL;
}
// are we the last mroot?
err = lfsr_mdir_lookup(lfs, &tinfo.u.mdir, LFSR_TAG_MROOT,
NULL);
if (err && err != LFS_ERR_NOENT) {
return err;
}
if (err == LFS_ERR_NOENT) {
// track active mroot
lfs->mroot = tinfo.u.mdir;
// mount/validate config in active mroot
err = lfsr_mountmroot(lfs, &lfs->mroot);
if (err) {
return err;
}
}
} else {
// found a direct mdir? keep track of this