Github/littlefs
1
Fork 0
Code Actions Activity

Extended test_files to test file btrees (up to 4*BLOCK_SIZE)

Browse Source 
Unfortunately, the tests are starting to take a painfully long time to
run. Some of this is because, in order to get interesting file
topologies, we need to move a ton of data around, but some of this is
also because our current write implementation has some problematically
expensive corner cases.

I have quite a few ideas on how to improve this, but in the meantime the
tests needed to be aggressively trimmed in order to keep development
tolerable (A happy developer is a productive developer).

This mainly meant:

- Disabled powerloss testing on file tests for now.

  The reality is that naivly powerloss testing the file tests, i.e.
  just truncating the file after each restart, provides very little
  value and adds an extreme amount of runtime.

  Removed for now. Most of the powerloss file creation concerns are
  covered in the dtree tests, and we should eventually add powerloss
  tests tailored to recovering files after powerloss instead of just
  truncating.

- Avoided tiny fragment sizes with large file sizes.

  Tiny fragments are a degenerate case and end up with excessive
  overhead (1 byte fragment => 41x overhead!). But they are useful for
  revealing subtle bugs. Still, it just doesn't make sense time-wise to
  test with tiny fragments once the file size exceeds ~1 block.

- Limited fuzz tests to cover fewer random seeds.

  We can increase these if performance improves, but even if not, we can
  run these individually with a high number of seeds in CI.

Also fixed a number of bugs found by the extended testing, which is
always a good sign:

- Yet another `lfsr_data_size(&data)` vs `data.u.disk.size` typo.

  This is the first time I've seen a real world argument for private
  struct/class fields, but I am still against the concept.

- Fixed delta/weight miscalculation when tree-carving a left sibling.

- Fixed missing offset in hole writing during block writes.

- Worked around lfsr_file_readnext's reliance on file->size when we are
  using it to write to a block. This may be more a hack than a good
  long term solution though.

- Checkpointed the allocator in both lfsr_file_write and lfsr_file_sync.

  Otherwise calling lfsr_file_write repeatedly can easily trigger an
  incorrect ENOSPC.

- Correctly reverted both shrubs and btrees in truncate/fruncate

  This gets a bit more complicated in fruncate, since either one of the
  two, or both, can revert.

  truncate/fruncate probably deserve a bit more work around reversions
  to simpler data structures, as is.

- Added handling of shrub overflows during fruncate.

  Notably not possible with truncate, shrub overflows require that we
  1. flush the shrub, 2. fruncate the tree, 3. and make sure any side
  effects to the buffer are handled correctly.
This commit is contained in:
Christopher Haster
2023-10-24 02:25:55 -05:00
parent 6d8eb948d1
commit b1bf650328
2 changed files with 454 additions and 173 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+96 -24
View File
@@ -10227,7 +10227,7 @@ static int lfsr_file_carvetree(lfs_t *lfs, lfsr_file_t *file,
lfsr_bptr_t bptr_ = {
.block = slice_.u.disk.block,
.off = slice_.u.disk.off,
.size = slice_.u.disk.size,
.size = lfsr_data_size(&slice_),
};
uint8_t bptr_buf[LFSR_BPTR_DSIZE];
@@ -10285,7 +10285,7 @@ static int lfsr_file_carvetree(lfs_t *lfs, lfsr_file_t *file,
lfsr_bptr_t bptr_ = {
.block = slice_.u.disk.block,
.off = slice_.u.disk.off,
.size = slice_.u.disk.size,
.size = lfsr_data_size(&slice_),
};
uint8_t bptr_buf[LFSR_BPTR_DSIZE];
@@ -10342,7 +10342,7 @@ static int lfsr_file_carvetree(lfs_t *lfs, lfsr_file_t *file,
lfsr_bptr_t bptr_ = {
.block = slice_.u.disk.block,
.off = slice_.u.disk.off,
.size = slice_.u.disk.size,
.size = lfsr_data_size(&slice_),
};
uint8_t bptr_buf[LFSR_BPTR_DSIZE];
@@ -10377,8 +10377,8 @@ static int lfsr_file_carvetree(lfs_t *lfs, lfsr_file_t *file,
}
}
delta += lfs_min32(weight, weight_);
weight -= lfs_min32(weight, weight_);
delta += lfs_min32(weight, bid_+1 - pos);
weight -= lfs_min32(weight, bid_+1 - pos);
}
// need a hole?
@@ -10635,9 +10635,9 @@ static int lfsr_file_flushshrub(lfs_t *lfs, lfsr_file_t *file) {
// use this via lfsr_data_t hole representation?
//
// found a hole? fill with zeros
} else if (lfsr_data_size(&data) == 0) {
} else {
for (lfs_size_t j = 0; j < weight; j++) {
err = lfsr_bd_prog(lfs, block, pos_ - left_align,
err = lfsr_bd_prog(lfs, block, pos_ - left_align + j,
&(uint8_t){0}, 1,
NULL);
if (err) {
@@ -11855,6 +11855,10 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
file->pos = file->size;
}
// TODO is this a good design? how do we abort?
// proactively update our file->size, we rely on this internally
file->size = lfs_max32(file->size, file->pos + size);
lfs_off_t pos = file->pos;
const uint8_t *buffer_ = buffer;
int err;
@@ -11884,6 +11888,10 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
continue;
}
// TODO is this the right place for this?
// checkpoint the allocator
lfs_alloc_ack(lfs);
// flush our buffer so the above can't fail
err = lfsr_file_flushbuffer(lfs, file);
if (err) {
@@ -11893,7 +11901,6 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
lfs_size_t written = pos - file->pos;
file->pos = pos;
file->size = lfs_max32(file->size, pos);
return written;
failed:;
@@ -11923,6 +11930,10 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// TODO should we also update file to be unbuffered after syncing
// inlined data?
// TODO is this the right place for this?
// checkpoint the allocator
lfs_alloc_ack(lfs);
// does buffer contain the entire file? we can create a simple
// inlined file in that case
if (file->buffer_size >= file->size) {
@@ -12063,17 +12074,22 @@ int lfsr_file_truncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size) {
// mark as unsynced before we commit anything
file->flags |= LFS_F_UNSYNCED;
// TODO we should also revert to sprout even if data is not already
// in buffer
//
// if our truncated file is contained entirely in our buffer,
// revert to a sprout
lfs_size_t buffer_size = lfs_min32(
file->buffer_size,
size - lfs_min32(file->buffer_pos, size));
if (buffer_size >= size) {
// TODO LFSR_SHRUB_NULL/LFSR_TREE_NULL?
file->shrub.u.data = LFSR_DATA_DISK(0, 0, 0);
file->tree.u.btree = LFSR_BTREE_NULL;
// TODO, wait, could we just update file->size and leave it to
// lfsr_file_sync to update the shrub?
// otherwise, we need to modify our sprout/shrub
// otherwise, we need to modify our sprout/shrub/bptr/btree
} else {
int err = lfsr_file_carveshrub(lfs, file,
lfs_min32(file->size, size),
@@ -12081,13 +12097,26 @@ int lfsr_file_truncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size) {
+size - file->size,
LFSR_TAG_SHRUB(DATA),
LFSR_DATA_NULL);
if (err) {
// note, unlike fruncate, truncate will never overflow a shrub
LFS_ASSERT(err != LFS_ERR_RANGE);
return err;
}
// TODO avoid transforming into trees all the time?
err = lfsr_file_carvetree(lfs, file,
lfs_min32(file->size, size),
file->size - lfs_min32(file->size, size),
+size - file->size,
LFSR_TAG_DATA,
LFSR_DATA_NULL);
if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE);
return err;
}
}
// TODO update btree?
LFS_ASSERT(!lfsr_shrub_hasshrub(&file->shrub)
|| lfsr_shrub_size(&file->shrub) > 0);
// update our buffer
file->buffer_size = buffer_size;
@@ -12114,29 +12143,72 @@ int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size) {
// mark as unsynced before we commit anything
file->flags |= LFS_F_UNSYNCED;
// TODO we should also revert to sprout even if data is not already
// in buffer
//
// if our truncated file is contained entirely in our buffer,
// revert to a sprout
lfs_size_t buffer_size = file->buffer_size - lfs_min32(
lfs_smax32(file->size - size - file->buffer_pos, 0),
file->buffer_size);
if (size < file->size
&& file->size - size >= lfsr_shrub_size(&file->shrub)) {
if (buffer_size >= size) {
// TODO LFSR_SHRUB_NULL/LFSR_TREE_NULL?
file->shrub.u.data = LFSR_DATA_DISK(0, 0, 0);
file->tree.u.btree = LFSR_BTREE_NULL;
// otherwise, we need to modify our sprout/shrub and btree
// otherwise, we need to modify our sprout/shrub/bptr/btree
} else {
int err = lfsr_file_carveshrub(lfs, file,
0,
lfs_smax32(file->size - size, 0),
+size - file->size,
LFSR_TAG_SHRUB(DATA),
LFSR_DATA_NULL);
if (err) {
return err;
// should should this logic and the above sprout logic be
// merged somehow?
//
// revert shrubs if they go to zero
if ((lfs_soff_t)(file->size - size)
>= (lfs_soff_t)lfsr_shrub_size(&file->shrub)) {
file->shrub.u.data = LFSR_DATA_DISK(0, 0, 0);
} else {
int err = lfsr_file_carveshrub(lfs, file,
0,
lfs_smax32(file->size - size, 0),
+size - file->size,
LFSR_TAG_SHRUB(DATA),
LFSR_DATA_NULL);
if (err && err != LFS_ERR_RANGE) {
return err;
}
// if a fruncate would push our shrub out of range, flush, and
// then take care of fruncate in carvetree
if (err == LFS_ERR_RANGE) {
err = lfsr_file_flushshrub(lfs, file);
if (err) {
return err;
}
// note! this zeros our buffer
buffer_size = 0;
}
}
// revert btrees if they go to zero
if ((lfs_soff_t)(file->size - size)
>= (lfs_soff_t)lfsr_tree_size(&file->tree)) {
file->tree.u.btree = LFSR_BTREE_NULL;
} else {
// TODO avoid transforming into trees all the time?
int err = lfsr_file_carvetree(lfs, file,
0,
lfs_smax32(file->size - size, 0),
+size - file->size,
LFSR_TAG_DATA,
LFSR_DATA_NULL);
if (err) {
LFS_ASSERT(err != LFS_ERR_RANGE);
return err;
}
}
}
// TODO update btree
LFS_ASSERT(!lfsr_shrub_hasshrub(&file->shrub)
|| lfsr_shrub_size(&file->shrub) > 0);
// update our buffer
file->buffer_pos -= lfs_smin32(file->size - size, file->buffer_pos);