Github/littlefs
1
Fork 0
Code Actions Activity

Reworked rbyd/btree/mdir structs again so redund blocks are at the end

Browse Source 
For a couple reasons:

1. Organizing the overlaps this way avoid potential undefined behavior.
   It turns out C does define the overlap the "initial sequence" of
   union members, as long as the types are the same. But when we
   overlapped the block with the size/tag fields in lfsr_btree_t, it was
   probably undefined behavior.

   At the very least, it would introduce a need for quite a bit of
   preprocessing to make it work with different integer sizes and
   redundancy levels.

2. Overlapping the blocks at the end of the rbyd struct means our block
   array is natural ordered such that the first block is the "active"
   block, i.e. the block with the most recent revision count that passes
   checksums.

   This has been useful as a debugging tool, so I would like to continue
   the pattern. It is possible to mostly preserve this order with the
   previous method by intentional reversing the block array when
   logging or writing to disk, but it's a bit cumbersome.

2. It's unlikely we'll be able to use readonly variants of the rbyd/mdir
   structs for RAM savings. Unfortunately C makes this too cumbersome.
   Though if we do this should be revisited.

Here are the new overlaps. Note it's no longer possible to truncate the
types when readonly. If readonly struct are useful this will need to be
revisited again:

   lfsr_rbyd_t            lfsr_btree_t           lfsr_mdir_t
                                                  8b   8b   8b   8b
                                                .----+----+----+----.
    8b   8b   8b   8b      8b   8b   8b   8b    | mid.bid | mid.rid |
  .----+----+----+----.  .----+----+----+----.  |----+----+----+----|
  |       weight      |.>|       weight      |  |       weight      |
  |----+----+----+----|  |----+----+----+----|  |----+----+----+----|
  |       trunk       |  |   tag   |   size  |  |       trunk       |
  |----+----+----+----|  |----+----+----+----|  |----+----+----+----|
  |        off        |  |    inlined data   |  |        off        |
  |----+----+----+----|  |         |         |  |----+----+----+----|
  |        crc        |  |         v         |  |        crc        |
  |----+----+----+----|  |                   |  |----+----+----+----|
  |       block       |..|                   |.>|       blocks      |
  '----+----+----+----'  '----+----+----+----'  |                   |
                                                |                   |
                                                '----+----+----+----'
This commit is contained in:
Christopher Haster
2023-08-06 01:21:05 -05:00
parent fe941ef443
commit c37bab6040
3 changed files with 32 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+16 -16
View File
@@ -5640,9 +5640,9 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
"-> 0x{%"PRIx32",%"PRIx32"}"
", 0x{%"PRIx32",%"PRIx32"}",
mdir->mid.bid,
mdir->u.m.blocks[1], mdir->u.m.blocks[0],
mdir_.u.m.blocks[1], mdir_.u.m.blocks[0],
msibling_.u.m.blocks[1], msibling_.u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1],
mdir_.u.m.blocks[0], mdir_.u.m.blocks[1],
msibling_.u.m.blocks[0], msibling_.u.m.blocks[1]);
// because of defered commits, both children can still be reduced
// to zero, need to catch this here
@@ -5651,10 +5651,10 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
if (mdir_.u.m.weight == 0 && msibling_.u.m.weight == 0) {
LFS_DEBUG("Dropping mdir %"PRId16" 0x{%"PRIx32",%"PRIx32"}",
mdir_.mid.bid,
mdir_.u.m.blocks[1], mdir_.u.m.blocks[0]);
mdir_.u.m.blocks[0], mdir_.u.m.blocks[1]);
LFS_DEBUG("Dropping mdir %"PRId16" 0x{%"PRIx32",%"PRIx32"}",
msibling_.mid.bid,
msibling_.u.m.blocks[1], msibling_.u.m.blocks[0]);
msibling_.u.m.blocks[0], msibling_.u.m.blocks[1]);
// mark as dropped
mdir_.u.r.rbyd.trunk = 0;
msibling_.u.r.rbyd.trunk = 0;
@@ -5669,7 +5669,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
} else if (mdir_.u.m.weight == 0) {
LFS_DEBUG("Dropping mdir %"PRId16" 0x{%"PRIx32",%"PRIx32"}",
mdir_.mid.bid,
mdir_.u.m.blocks[1], mdir_.u.m.blocks[0]);
mdir_.u.m.blocks[0], mdir_.u.m.blocks[1]);
// mark as dropped
mdir_.u.r.rbyd.trunk = 0;
@@ -5691,7 +5691,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
} else if (msibling_.u.m.weight == 0) {
LFS_DEBUG("Dropping mdir %"PRId16" 0x{%"PRIx32",%"PRIx32"}",
msibling_.mid.bid,
msibling_.u.m.blocks[1], msibling_.u.m.blocks[0]);
msibling_.u.m.blocks[0], msibling_.u.m.blocks[1]);
// mark as dropped
msibling_.u.r.rbyd.trunk = 0;
@@ -5758,7 +5758,7 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
} else if (mdir->mid.bid != -1 && mdir_.u.m.weight == 0) {
LFS_DEBUG("Dropping mdir %"PRId16" 0x{%"PRIx32",%"PRIx32"}",
mdir->mid.bid,
mdir->u.m.blocks[1], mdir->u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
// mark as dropped
mdir_.u.r.rbyd.trunk = 0;
@@ -5784,8 +5784,8 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
LFS_DEBUG("Relocating mdir %"PRId16" 0x{%"PRIx32",%"PRIx32"} "
"-> 0x{%"PRIx32",%"PRIx32"}",
mdir->mid.bid,
mdir->u.m.blocks[1], mdir->u.m.blocks[0],
mdir_.u.m.blocks[1], mdir_.u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1],
mdir_.u.m.blocks[0], mdir_.u.m.blocks[1]);
// update our mtree
uint8_t buf[LFSR_MDIR_DSIZE];
@@ -5918,8 +5918,8 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
LFS_DEBUG("Relocating mroot 0x{%"PRIx32",%"PRIx32"} "
"-> 0x{%"PRIx32",%"PRIx32"}",
mchildroot.u.m.blocks[1], mchildroot.u.m.blocks[0],
mchildroot_[1], mchildroot_[0]);
mchildroot.u.m.blocks[0], mchildroot.u.m.blocks[1],
mchildroot_[0], mchildroot_[1]);
// commit mrootchild
uint8_t buf[LFSR_MDIR_DSIZE];
@@ -5951,9 +5951,9 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
LFS_DEBUG("Extending mroot 0x{%"PRIx32",%"PRIx32"}"
" -> 0x{%"PRIx32",%"PRIx32"}"
", 0x{%"PRIx32",%"PRIx32"}",
mchildroot.u.m.blocks[1], mchildroot.u.m.blocks[0],
mchildroot.u.m.blocks[1], mchildroot.u.m.blocks[0],
mchildroot_[1], mchildroot_[0]);
mchildroot.u.m.blocks[0], mchildroot.u.m.blocks[1],
mchildroot.u.m.blocks[0], mchildroot.u.m.blocks[1],
mchildroot_[0], mchildroot_[1]);
// copy magic/config from current mroot
lfsr_data_t magic;
@@ -6534,7 +6534,7 @@ cycle_detect:;
traversal->tortoise_blocks) == 0) {
LFS_ERROR("Cycle detected during mtree traversal "
"(0x{%"PRIx32",%"PRIx32"})",
traversal->mdir.u.m.blocks[1], traversal->mdir.u.m.blocks[0]);
traversal->mdir.u.m.blocks[0], traversal->mdir.u.m.blocks[1]);
return LFS_ERR_CORRUPT;
}
if (traversal->tortoise_step
lfs.h
+9 -11
View File
@@ -346,8 +346,6 @@ typedef struct lfs_cache {
// TODO do we get ram savings with a lfsr_rorbyd_t substruct? need to measure
typedef struct lfsr_rbyd {
// note this lines up with arrays of redundant blocks in other structures
lfs_block_t block;
// note this lines up with weight in lfsr_btree_t
lfs_size_t weight;
// off=0, trunk=0 => not yet committed
@@ -356,6 +354,8 @@ typedef struct lfsr_rbyd {
lfs_off_t trunk;
lfs_off_t off;
uint32_t crc;
// note this lines up with arrays of redundant blocks in lfsr_mdir_t
lfs_block_t block;
} lfsr_rbyd_t;
// The maximum size of inlined pointers in a btree, this depends on littlefs's
@@ -372,20 +372,15 @@ typedef struct lfsr_btree {
// weight is common to both representations and its sign-bit indicates
// if the btree is inlined
struct {
lfs_size_t _padding;
lfs_size_t weight;
} b;
struct {
// TODO complex ifdefs here to handle tag/size > 1/2 a word?
lfsr_tag_t tag;
uint8_t size;
uint8_t _padding;
lfs_size_t weight;
lfsr_tag_t tag;
uint16_t size;
uint8_t buffer[LFSR_BTREE_INLINESIZE];
} i;
struct {
// note the sign bit of the rbyd.block indicates if the btree is
// inlined or a normal btree
lfsr_rbyd_t rbyd;
} r;
} u;
@@ -397,12 +392,15 @@ typedef struct lfsr_mdir {
// here we make sure to line up our block array so it overlaps with
// the block stored as the first entry in the rbyd
struct {
lfs_block_t blocks[2];
lfs_size_t weight;
lfs_off_t trunk;
lfs_off_t off;
uint32_t crc;
lfs_block_t blocks[2];
} m;
struct {
lfs_block_t redund_block;
lfsr_rbyd_t rbyd;
lfs_block_t redund_block;
} r;
} u;
} lfsr_mdir_t;
tests/test_mtree.toml
+7 -7
View File
@@ -3506,7 +3506,7 @@ code = '''
mid_.bid,
mid_.rid,
tag_,
mdir->u.m.blocks[1], mdir->u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
// keep track of seen blocks
seen[mdir->u.m.blocks[1] / 8] |= 1 << (mdir->u.m.blocks[1] % 8);
@@ -3631,7 +3631,7 @@ code = '''
mid_.bid,
mid_.rid,
tag_,
mdir->u.m.blocks[1], mdir->u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
// keep track of seen blocks
seen[mdir->u.m.blocks[1] / 8] |= 1 << (mdir->u.m.blocks[1] % 8);
@@ -3767,7 +3767,7 @@ code = '''
mid_.bid,
mid_.rid,
tag_,
mdir->u.m.blocks[1], mdir->u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
// keep track of seen blocks
seen[mdir->u.m.blocks[1] / 8] |= 1 << (mdir->u.m.blocks[1] % 8);
@@ -3895,7 +3895,7 @@ code = '''
mid_.bid,
mid_.rid,
tag_,
mdir->u.m.blocks[1], mdir->u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
// keep track of seen blocks
seen[mdir->u.m.blocks[1] / 8] |= 1 << (mdir->u.m.blocks[1] % 8);
@@ -4031,7 +4031,7 @@ code = '''
mid_.bid,
mid_.rid,
tag_,
mdir->u.m.blocks[1], mdir->u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
// keep track of seen blocks
seen[mdir->u.m.blocks[1] / 8] |= 1 << (mdir->u.m.blocks[1] % 8);
@@ -4194,7 +4194,7 @@ code = '''
mid_.bid,
mid_.rid,
tag_,
mdir->u.m.blocks[1], mdir->u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
// keep track of seen blocks
seen[mdir->u.m.blocks[1] / 8] |= 1 << (mdir->u.m.blocks[1] % 8);
@@ -4298,7 +4298,7 @@ code = '''
mid_.bid,
mid_.rid,
tag_,
mdir->u.m.blocks[1], mdir->u.m.blocks[0]);
mdir->u.m.blocks[0], mdir->u.m.blocks[1]);
} else {
// this shouldn't happen
printf("traversal: %d.%d 0x%x %d\n",