Skip to content

Commit

Permalink
Locked down out-of-order writes, more tests
Browse files Browse the repository at this point in the history
The main test additions are the test_powerloss tests, intended to be
high-level tests over difficult/weird powerloss environments (such as
out-of-order writes!):

- test_powerloss_dir_many - 2242 pls
- test_powerloss_file_many - 8856 pls
- test_powerloss_file_pl_fuzz - 384508 pls
- test_powerloss_filedir_pl_fuzz - 268339 pls

But there was also a bunch of other test movement in the late-stage/
high-level tests. I'm trying to keep the core of these tests somewhat
consistent so we have a nice template to extend for future testing, in
case we want to test other environmentalish concerns, but not all of
these tests make sense in all of these contexts:

                        badblocks  powerloss  relocations  exhaustion
  dir_many                      y          y            y
  dir_fuzz                      y                       y           y
  file_many                     y          y            y
  file_fuzz                     y                       y           y
  fwrite_fuzz                   y                                   y
  orphanzombie_fuzz             y                       y           y
  orphanzombiedir_fuzz          y                       y           y
  file_pl_fuzz                             y            y
  filedir_pl_fuzz                          y            y

Why not:

- dir/file_many+exhaustion? - Needs to be unbounded
- dir/file_fuzz+powerloss? - Takes O(n^2)
- fwrite_fuzz+powerloss? - Takes O(n^2)
- fwrite_fuzz+relocations? - Doesn't really test anything
- orphanzombie*_fuzz+powerloss? - Powerloss kills zombies
- file*_pl_fuzz+badblocks? - PL + Badblocks currently incompactible
- file*_pl_fuzz+exhaustion? - PL + Badblocks currently incompactible

---

Of course, in order to actually get out-of-order write testing working,
we need to implement out-of-order write syncing.

Fortunately this was a simple exercise in placing lfsr_bd_sync calls
before any mdir commits where we may have unsynced data:

- in lfsr_file_sync, to sync any pending file data
- in lfsr_mdir_commit, to sync any mroot/mtree changes

We also call lfsr_bd_sync _after_ mdir commits in case users expect to
sequence any filesystem-external operations such as network, UI, etc. In
theory this could be optional, but no users have really requested it
yet, so leave that for a potential future improvement:

- in lfsr_mdir_commit
- in lfsr_formatinited (really just because we don't go through
  lfsr_mdir_commit)

Note that lfsr_rbyd_commit has been relaxed in the scheme. It only
flushes caches, and does _not_ call lfsr_bd_sync. This is useful for
allowing multiple B-tree nodes to be written out-of-order, also long as
the whole thing is synchronized before any mdir commit.

All of these lfsr_bd_sync calls add a bit of code, but not really an
amount to care about:

           code          stack
  before: 33678           2600
  after:  33766 (+0.3%)   2600 (+0.0%)
  • Loading branch information
geky committed Jun 1, 2024
1 parent bb3ef46 commit 9914897
Show file tree
Hide file tree
Showing 7 changed files with 5,255 additions and 1,692 deletions.
48 changes: 44 additions & 4 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -3461,8 +3461,8 @@ static int lfsr_rbyd_appendcksum(lfs_t *lfs, lfsr_rbyd_t *rbyd) {
return err;
}

// flush our caches, finalizing the commit on-disk
err = lfsr_bd_sync(lfs);
// flush any pending progs
err = lfsr_bd_flush(lfs, NULL, false);
if (err) {
return err;
}
Expand Down Expand Up @@ -7115,6 +7115,13 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
}
}

// make sure mtree/mroot changes are on-disk before committing
// metadata
err = lfsr_bd_sync(lfs);
if (err) {
goto failed;
}

// commit new mtree into our mroot
//
// note end_rid=0 here will delete any files leftover from a split
Expand Down Expand Up @@ -7157,6 +7164,13 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,

mrootchild = mrootparent_;

// make sure mtree/mroot changes are on-disk before committing
// metadata
err = lfsr_bd_sync(lfs);
if (err) {
goto failed;
}

// commit mrootchild
uint8_t mrootchild_buf[LFSR_MPTR_DSIZE];
err = lfsr_mdir_commit_(lfs, &mrootparent_, -1, -1, NULL,
Expand Down Expand Up @@ -7186,6 +7200,13 @@ 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]);

// make sure mtree/mroot changes are on-disk before committing
// metadata
err = lfsr_bd_sync(lfs);
if (err) {
goto failed;
}

// commit the new mroot anchor
lfsr_mdir_t mrootanchor_;
err = lfsr_mdir_swap__(lfs, &mrootanchor_, &mrootchild, true);
Expand Down Expand Up @@ -7215,6 +7236,12 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
// gstate must have been committed by a lower-level function at this point
LFS_ASSERT(lfsr_gdelta_iszero(lfs->grm_d, LFSR_GRM_DSIZE));

// sync on-disk state
err = lfsr_bd_sync(lfs);
if (err) {
return err;
}

// success? update in-device state, we must not error at this point

// toss our cksum into the filesystem seed for pseudorandom numbers
Expand Down Expand Up @@ -8672,8 +8699,14 @@ static int lfsr_formatinited(lfs_t *lfs) {
}
}

// sync on-disk state
int err = lfsr_bd_sync(lfs);
if (err) {
return err;
}

// test that mount works with our formatted disk
int err = lfsr_mountinited(lfs);
err = lfsr_mountinited(lfs);
if (err) {
return err;
}
Expand Down Expand Up @@ -11681,7 +11714,7 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// checkpoint the allocator again
lfs_alloc_ckpoint(lfs);

// commit our file's metadata
// commit any changes to our file's metadata
lfsr_attr_t attrs[2];
lfs_size_t attr_count = 0;
lfsr_data_t name_data;
Expand Down Expand Up @@ -11728,6 +11761,13 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
LFS_UNREACHABLE();
}

// make sure data is on-disk before committing metadata
err = lfsr_bd_sync(lfs);
if (err) {
goto failed;
}

// commit!
LFS_ASSERT(attr_count <= sizeof(attrs)/sizeof(lfsr_attr_t));

err = lfsr_mdir_commit(lfs, &file->o.mdir,
Expand Down
Loading

0 comments on commit 9914897

Please sign in to comment.