Github/littlefs
1
Fork 0
Code Actions Activity

Found a better heuristic for did truncation, dropped mlimit, found bugs

Browse Source 
The idea here is to combine the current mtree size with the theoretical
upper bound on the number of directories in a single mdir, assuming our
block size, to give us a heuristic for did truncation that does not
require any extra state.

- Each directory needs 1 name tag, 1 did tag, and 1 dstart
- Each tag needs ~2 alts with our current compaction strategy
- Each tag/alt encodes to a minimum of 4 bytes
- We can also assume ~1/2 block utilization due to our split threshold

This gives us ~3*3*4*2 or ~72 bytes per directory at minimum, or
rounding down, ~block_size/32 directories per mdir.

This is a nice number because for common NOR flash geometry,
4096/32 = 128, so a filesystem with a single mdir encodes dids in a
single byte.

The biggest benefit though is being able to drop the mlimit state from
the lfs_t struct.

---

Unfortunately, this change revealed several bugs.

It turns out __builtin_clz in GCC is undefined at 0, which caused our
lfs_nlog2 function to return incorrect values at 1. This was causing
our dids to all collide when the mtree was inlined, which was resolved
by the linear scanning that resolves dids, but was severely limiting
what exactly our tests covered.

Now that this is fixed (with a simple if statement in lfs_nlog2,
lfs_nlog2 now always has defined behavior, even at 0), several bugs
needed fixing:

- We update the rid based on attrs in lfsr_mdir_commit before updating
  the mdir. If we have multiple attrs this causes the assert on
  rid-in-bounds to trigger incorrectly. Just removed that assert for now.

- We needed to adjust second grms if they are affected by the fixing
  of the first grm.

- Directory position updates are incorrectly updated if an unrelated
  weight change occurs before an opened directory, but is not a part of
  that opened directory.

  This is NOT fixed yet, the current implementation is just broken
  enough that I've just ripped it out for now (it was causing the
  read_with_rms test to fail because pos backed up into the "."/".."
  entries).

  This needs some thinking to fix.

Because of that last, unfixed bug, tests are not all passing at the
moment. To pass testing -DSEEK=0 is needed to disable the failing tests.
This commit is contained in:
Christopher Haster
2023-07-26 15:22:36 -05:00
parent d931c19dda
commit e0f416f6ca
3 changed files with 36 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+31 -35
View File
@@ -3354,7 +3354,7 @@ static int lfsr_rbyd_estimate(lfs_t *lfs, const lfsr_rbyd_t *rbyd,
// fortunately the self-balancing nature of rybds give us a tight
// bound on the number of alt pointers
real_dsize
+= (2*lfs_nlog2(count+1)+1) * LFSR_TAG_DSIZE
+= (2*lfs_nlog2(count)+1) * LFSR_TAG_DSIZE
+ LFSR_TAG_DSIZE
+ lfsr_data_size(data);
@@ -6242,7 +6242,6 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
for (lfs_size_t i = 0; i < attr_count; i++) {
if (opened->mdir.mid == mdir->mid
&& opened->rid >= attrs[i].id) {
LFS_ASSERT(opened->rid <= (lfs_ssize_t)mdir->rbyd.weight);
// removed?
if (opened->rid + attrs[i].delta < attrs[i].id) {
// note we have different behavior for files and dirs
@@ -6256,16 +6255,6 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
}
} else {
opened->rid += attrs[i].delta;
// adjust dir position
if (type == LFS_TYPE_DIR) {
((lfsr_dir_t*)opened)->pos += attrs[i].delta;
}
}
} else if (opened->mdir.mid > mdir->mid) {
// adjust dir position
if (type == LFS_TYPE_DIR) {
((lfsr_dir_t*)opened)->pos += attrs[i].delta;
}
}
}
@@ -6307,9 +6296,6 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir, lfs_ssize_t *rid,
// update our mroot and mtree
lfs->mroot = mroot_;
lfs->mtree = mtree_;
lfs->mlimit = lfs_max32(lfs->mlimit, lfs_max32(
mdir_.rbyd.weight,
msibling_.rbyd.weight));
return 0;
}
@@ -6564,10 +6550,6 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
return err;
}
// keep track of the largest mdir we've seen, reset here
// since this is the first mdir we see
lfs->mlimit = traversal->mdir.rbyd.weight;
if (mid_) {
*mid_ = -1;
}
@@ -6605,9 +6587,6 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
return err;
}
// keep track of the largest mdir we've seen
lfs->mlimit = lfs_max32(lfs->mlimit, traversal->mdir.rbyd.weight);
if (mid_) {
*mid_ = -1;
}
@@ -6726,9 +6705,6 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
return err;
}
// keep track of the largest mdir we've seen
lfs->mlimit = lfs_max32(lfs->mlimit, traversal->mdir.rbyd.weight);
if (mid_) {
*mid_ = mid;
}
@@ -7404,22 +7380,34 @@ int lfsr_mkdir(lfs_t *lfs, const char *path) {
}
// Our directory needs an arbitrary directory-id. To find one with
// hopefully few collisions, we use a hash of the full path. Since
// we have a CRC handy, we can use that.
// hopefully few collisions, we use a hash of the full path using our CRC,
// since we have it handy.
//
// We also truncate to make better use of our leb128 encoding. This is
// pretty arbitrary, but we don't want collisions, so we truncate to
// our best >= estimate of the number of metadata entries. Worst case
// this is >= ~2x the number of dids in the system, since each dir needs
// a dir entry and dstart entry.
// relatively arbitrary, but if we truncate too much we risk increasing
// the number of collisions, so we want to aim for ~2x the number dids
// in the system. We don't actually know the number of dids in the system,
// but we can use a heuristic based on the maximum possible number of
// directories in the current mtree assuming our block size.
//
// - Each directory needs 1 name tag, 1 did tag, and 1 dstart
// - Each tag needs ~2 alts with our current compaction strategy
// - Each tag/alt encodes to a minimum of 4 bytes
// - We can also assume ~1/2 block utilization due to our split threshold
//
// This gives us ~3*3*4*2 or ~72 bytes per directory at minimum.
// Multiplying by 2 and rounding down to the nearest power of 2 for cheaper
// division gives us a heuristic of ~block_size/32 directories per mdir.
//
// This is a nice number because for common NOR flash geometry,
// 4096/32 = 128, so a filesystem with a single mdir encodes dids in a
// single byte.
//
lfs_size_t did = lfs_crc32c(0, path, strlen(path))
// This complicated bit of logic determines the upper limit of
// metadata entries, limited to 2^32. This is done post-log to
// try to avoid issues with integer overflow.
// note we need to be careful to catch integer overflow
& ((1 << lfs_min32(
lfs_nlog2(lfsr_mtree_weight(lfs))
+ lfs_nlog2(lfs->mlimit),
+ lfs_nlog2(lfs->cfg->block_size/32),
32)) - 1);
// Check if we have a collision. If we do, search for the next
@@ -7957,6 +7945,14 @@ static int lfsr_fs_fixgrm(lfs_t *lfs) {
lfsr_grm_t grm_ = lfs->grm_;
lfsr_grm_poprm(&grm_);
// TODO should this just be implicit in lfsr_mdir_commit? compare cost?
// make sure to adjust any remaining grms
if (grm_.rms[0].mid == lfs->grm_.rms[0].mid
&& grm_.rms[0].rid >= lfs->grm_.rms[0].rid) {
LFS_ASSERT(grm_.rms[0].rid != lfs->grm_.rms[0].rid);
grm_.rms[0].rid -= 1;
}
uint8_t buf[LFSR_GRM_DSIZE];
lfs_ssize_t d = lfsr_grm_todisk(lfs, &grm_, buf);
if (d < 0) {