Attempted better allocator checkpoints
This tries to call lfs3_alloc_ckpoint in more correct positions, and
fixes a bug where we _never_ called lfs3_alloc_ckpoint before
finishing crystallization in lfs3_file_readnext and
lfs3_file_truncate/fruncate:
- lfs3_file_crystallize now implicitly calls lfs3_alloc_ckpoint before
both finishing crystallization and grafting.
- lfs3_file_flush_ and lfs3_file_flushonce_ now call lfs3_alloc_ckpoint
at the beginning of each loop iteration.
This may be redundant on some iterations but that's ok.
- lfs3_file_write does _not_ call lfs3_alloc_ckpoint, this is all
handled in lfs3_file_flush_ now.
- lfs3_file_truncate/fruncate still call lfs3_alloc_ckpoint, but just
before lfs3_file_graft.
This matches the lfs3_alloc_ckpoint pattern used for most
lfs3_mdir_commit calls, i.e. checkpoint just before to make it easier
to audit the logic.
- Also moved the pre-fragment crystallization out of the fragment loop,
we should only crystallize once and this makes the code a bit more
readable.
I think this is the source of the extra 8 bytes of stack, but that's
small enough to consider compiler noise.
It's not the biggest problem to not call lfs3_alloc_ckpoint everytime
all blocks are at rest, but it does risk a premature ENOSPC error when
it's still possible to make progress.
This gets more complicated with lazy crystallization/grafting, as block
allocations can end up deferred to operations you might not expect
(lfs3_file_read for example).
Adds a bit of code, but is in theory more correct:
code stack ctx
before: 37888 2416 636
after: 37920 (+0.1%) 2424 (+0.3%) 636 (+0.0%)
This commit is contained in:
@@ -12775,7 +12775,8 @@ static int lfs3_file_crystallize_(lfs3_t *lfs3, lfs3_file_t *file,
|
|||||||
|
|
||||||
#if !defined(LFS3_RDONLY) && !defined(LFS3_KVONLY) && !defined(LFS3_2BONLY)
|
#if !defined(LFS3_RDONLY) && !defined(LFS3_KVONLY) && !defined(LFS3_2BONLY)
|
||||||
static int lfs3_file_crystallize(lfs3_t *lfs3, lfs3_file_t *file) {
|
static int lfs3_file_crystallize(lfs3_t *lfs3, lfs3_file_t *file) {
|
||||||
bool flushable = (
|
// is it possible for this to flush the cache?
|
||||||
|
bool flushing = (
|
||||||
file->cache.pos
|
file->cache.pos
|
||||||
>= file->leaf.pos + lfs3_bptr_size(&file->leaf.bptr));
|
>= file->leaf.pos + lfs3_bptr_size(&file->leaf.bptr));
|
||||||
|
|
||||||
@@ -12787,6 +12788,8 @@ static int lfs3_file_crystallize(lfs3_t *lfs3, lfs3_file_t *file) {
|
|||||||
LFS3_ASSERT(lfs3_bptr_isbptr(&file->leaf.bptr));
|
LFS3_ASSERT(lfs3_bptr_isbptr(&file->leaf.bptr));
|
||||||
LFS3_ASSERT(lfs3_bptr_iserased(&file->leaf.bptr));
|
LFS3_ASSERT(lfs3_bptr_iserased(&file->leaf.bptr));
|
||||||
|
|
||||||
|
// checkpoint the allocator
|
||||||
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
// finish crystallizing the block
|
// finish crystallizing the block
|
||||||
int err = lfs3_file_crystallize_(lfs3, file,
|
int err = lfs3_file_crystallize_(lfs3, file,
|
||||||
file->leaf.pos - lfs3_bptr_off(&file->leaf.bptr), -1, -1,
|
file->leaf.pos - lfs3_bptr_off(&file->leaf.bptr), -1, -1,
|
||||||
@@ -12801,6 +12804,9 @@ static int lfs3_file_crystallize(lfs3_t *lfs3, lfs3_file_t *file) {
|
|||||||
|
|
||||||
// and graft into tree
|
// and graft into tree
|
||||||
if (lfs3_o_isungraft(file->b.o.flags)) {
|
if (lfs3_o_isungraft(file->b.o.flags)) {
|
||||||
|
// checkpoint the allocator
|
||||||
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
|
// and graft
|
||||||
int err = lfs3_file_graft(lfs3, file,
|
int err = lfs3_file_graft(lfs3, file,
|
||||||
file->leaf.pos, file->leaf.weight, 0,
|
file->leaf.pos, file->leaf.weight, 0,
|
||||||
&file->leaf.bptr.data, -1);
|
&file->leaf.bptr.data, -1);
|
||||||
@@ -12813,7 +12819,7 @@ static int lfs3_file_crystallize(lfs3_t *lfs3, lfs3_file_t *file) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// eagerly mark as flushed if this included all of our cache
|
// eagerly mark as flushed if this included all of our cache
|
||||||
if (flushable
|
if (flushing
|
||||||
&& file->leaf.pos + lfs3_bptr_size(&file->leaf.bptr)
|
&& file->leaf.pos + lfs3_bptr_size(&file->leaf.bptr)
|
||||||
>= file->cache.pos + file->cache.size) {
|
>= file->cache.pos + file->cache.size) {
|
||||||
file->b.o.flags &= ~LFS3_o_UNFLUSH;
|
file->b.o.flags &= ~LFS3_o_UNFLUSH;
|
||||||
@@ -12829,6 +12835,9 @@ static int lfs3_file_flushonce_(lfs3_t *lfs3, lfs3_file_t *file,
|
|||||||
const uint8_t *buffer, lfs3_size_t size) {
|
const uint8_t *buffer, lfs3_size_t size) {
|
||||||
lfs3_off_t pos = 0;
|
lfs3_off_t pos = 0;
|
||||||
while (size > 0) {
|
while (size > 0) {
|
||||||
|
// checkpoint the allocator
|
||||||
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
|
|
||||||
// enough data for a block?
|
// enough data for a block?
|
||||||
#ifndef LFS3_2BONLY
|
#ifndef LFS3_2BONLY
|
||||||
if (size > lfs3->cfg->crystal_thresh) {
|
if (size > lfs3->cfg->crystal_thresh) {
|
||||||
@@ -12926,6 +12935,9 @@ static int lfs3_file_flush_(lfs3_t *lfs3, lfs3_file_t *file,
|
|||||||
// iteratively write blocks
|
// iteratively write blocks
|
||||||
#ifndef LFS3_2BONLY
|
#ifndef LFS3_2BONLY
|
||||||
while (size > 0) {
|
while (size > 0) {
|
||||||
|
// checkpoint the allocator
|
||||||
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
|
|
||||||
// mid-crystallization? can we just resume crystallizing?
|
// mid-crystallization? can we just resume crystallizing?
|
||||||
//
|
//
|
||||||
// note that the threshold to resume crystallization (prog_size),
|
// note that the threshold to resume crystallization (prog_size),
|
||||||
@@ -12942,6 +12954,7 @@ static int lfs3_file_flush_(lfs3_t *lfs3, lfs3_file_t *file,
|
|||||||
&& pos - block_end < lfs3->cfg->crystal_thresh
|
&& pos - block_end < lfs3->cfg->crystal_thresh
|
||||||
// need to bail if we can't meet prog alignment
|
// need to bail if we can't meet prog alignment
|
||||||
&& (pos + size) - block_end >= lfs3->cfg->prog_size) {
|
&& (pos + size) - block_end >= lfs3->cfg->prog_size) {
|
||||||
|
// crystallize
|
||||||
int err = lfs3_file_crystallize_(lfs3, file,
|
int err = lfs3_file_crystallize_(lfs3, file,
|
||||||
block_start,
|
block_start,
|
||||||
(pos + size) - block_start,
|
(pos + size) - block_start,
|
||||||
@@ -13070,6 +13083,7 @@ static int lfs3_file_flush_(lfs3_t *lfs3, lfs3_file_t *file,
|
|||||||
&& lfs3_bptr_iserased(&file->leaf.bptr)
|
&& lfs3_bptr_iserased(&file->leaf.bptr)
|
||||||
&& crystal_start >= block_end
|
&& crystal_start >= block_end
|
||||||
&& crystal_start < block_start + lfs3->cfg->block_size) {
|
&& crystal_start < block_start + lfs3->cfg->block_size) {
|
||||||
|
// crystallize
|
||||||
int err = lfs3_file_crystallize_(lfs3, file,
|
int err = lfs3_file_crystallize_(lfs3, file,
|
||||||
block_start,
|
block_start,
|
||||||
crystal_end - block_start,
|
crystal_end - block_start,
|
||||||
@@ -13163,9 +13177,9 @@ static int lfs3_file_flush_(lfs3_t *lfs3, lfs3_file_t *file,
|
|||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
|
return 0;
|
||||||
|
|
||||||
fragment:;
|
fragment:;
|
||||||
// iteratively write fragments (inlined leaves)
|
|
||||||
while (size > 0) {
|
|
||||||
// before we write fragments, we need to make sure our crystal
|
// before we write fragments, we need to make sure our crystal
|
||||||
// is grafted into the tree
|
// is grafted into the tree
|
||||||
//
|
//
|
||||||
@@ -13173,6 +13187,7 @@ fragment:;
|
|||||||
// writes!
|
// writes!
|
||||||
if (lfs3_o_isungraft(file->b.o.flags)) {
|
if (lfs3_o_isungraft(file->b.o.flags)) {
|
||||||
// graft our crystal
|
// graft our crystal
|
||||||
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
int err = lfs3_file_graft(lfs3, file,
|
int err = lfs3_file_graft(lfs3, file,
|
||||||
file->leaf.pos, file->leaf.weight, 0,
|
file->leaf.pos, file->leaf.weight, 0,
|
||||||
&file->leaf.bptr.data, -1);
|
&file->leaf.bptr.data, -1);
|
||||||
@@ -13184,6 +13199,11 @@ fragment:;
|
|||||||
file->b.o.flags &= ~LFS3_o_UNGRAFT;
|
file->b.o.flags &= ~LFS3_o_UNGRAFT;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// iteratively write fragments (inlined leaves)
|
||||||
|
while (size > 0) {
|
||||||
|
// checkpoint the allocator
|
||||||
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
|
|
||||||
// do we need to discard our leaf? we need to discard fragments
|
// do we need to discard our leaf? we need to discard fragments
|
||||||
// in case the underlying rbyd compacts, and we need to discard
|
// in case the underlying rbyd compacts, and we need to discard
|
||||||
// overwritten blocks
|
// overwritten blocks
|
||||||
@@ -13354,8 +13374,6 @@ lfs3_ssize_t lfs3_file_write(lfs3_t *lfs3, lfs3_file_t *file,
|
|||||||
|
|
||||||
// clobber entangled traversals
|
// clobber entangled traversals
|
||||||
lfs3_omdir_clobber(lfs3, &file->b.o, LFS3_t_DIRTY);
|
lfs3_omdir_clobber(lfs3, &file->b.o, LFS3_t_DIRTY);
|
||||||
// checkpoint the allocator
|
|
||||||
lfs3_alloc_ckpoint(lfs3);
|
|
||||||
// mark as unsynced in case we fail
|
// mark as unsynced in case we fail
|
||||||
file->b.o.flags |= LFS3_o_UNSYNC;
|
file->b.o.flags |= LFS3_o_UNSYNC;
|
||||||
|
|
||||||
@@ -13503,8 +13521,6 @@ int lfs3_file_flush(lfs3_t *lfs3, lfs3_file_t *file) {
|
|||||||
#ifndef LFS3_RDONLY
|
#ifndef LFS3_RDONLY
|
||||||
// clobber entangled traversals
|
// clobber entangled traversals
|
||||||
lfs3_omdir_clobber(lfs3, &file->b.o, LFS3_t_DIRTY);
|
lfs3_omdir_clobber(lfs3, &file->b.o, LFS3_t_DIRTY);
|
||||||
// checkpoint the allocator
|
|
||||||
lfs3_alloc_ckpoint(lfs3);
|
|
||||||
int err;
|
int err;
|
||||||
|
|
||||||
// flush our cache
|
// flush our cache
|
||||||
@@ -13711,9 +13727,11 @@ static int lfs3_file_sync_(lfs3_t *lfs3, lfs3_file_t *file,
|
|||||||
|
|
||||||
// pending metadata? looks like we need to write to disk
|
// pending metadata? looks like we need to write to disk
|
||||||
if (rattr_count > 0) {
|
if (rattr_count > 0) {
|
||||||
// commit!
|
// make sure we don't overflow our rattr buffer
|
||||||
LFS3_ASSERT(rattr_count <= sizeof(rattrs)/sizeof(lfs3_rattr_t));
|
LFS3_ASSERT(rattr_count <= sizeof(rattrs)/sizeof(lfs3_rattr_t));
|
||||||
|
// checkpoint the allocator
|
||||||
lfs3_alloc_ckpoint(lfs3);
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
|
// and commit!
|
||||||
int err = lfs3_mdir_commit(lfs3, &file->b.o.mdir,
|
int err = lfs3_mdir_commit(lfs3, &file->b.o.mdir,
|
||||||
rattrs, rattr_count);
|
rattrs, rattr_count);
|
||||||
if (err) {
|
if (err) {
|
||||||
@@ -14006,8 +14024,6 @@ int lfs3_file_truncate(lfs3_t *lfs3, lfs3_file_t *file, lfs3_off_t size_) {
|
|||||||
|
|
||||||
// clobber entangled traversals
|
// clobber entangled traversals
|
||||||
lfs3_omdir_clobber(lfs3, &file->b.o, LFS3_t_DIRTY);
|
lfs3_omdir_clobber(lfs3, &file->b.o, LFS3_t_DIRTY);
|
||||||
// checkpoint the allocator
|
|
||||||
lfs3_alloc_ckpoint(lfs3);
|
|
||||||
// mark as unsynced in case we fail
|
// mark as unsynced in case we fail
|
||||||
file->b.o.flags |= LFS3_o_UNSYNC;
|
file->b.o.flags |= LFS3_o_UNSYNC;
|
||||||
|
|
||||||
@@ -14032,6 +14048,8 @@ int lfs3_file_truncate(lfs3_t *lfs3, lfs3_file_t *file, lfs3_off_t size_) {
|
|||||||
lfs3_file_discardleaf(file);
|
lfs3_file_discardleaf(file);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// checkpoint the allocator
|
||||||
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
// truncate our btree
|
// truncate our btree
|
||||||
err = lfs3_file_graft(lfs3, file,
|
err = lfs3_file_graft(lfs3, file,
|
||||||
lfs3_min(size, size_), size - lfs3_min(size, size_),
|
lfs3_min(size, size_), size - lfs3_min(size, size_),
|
||||||
@@ -14092,8 +14110,6 @@ int lfs3_file_fruncate(lfs3_t *lfs3, lfs3_file_t *file, lfs3_off_t size_) {
|
|||||||
|
|
||||||
// clobber entangled traversals
|
// clobber entangled traversals
|
||||||
lfs3_omdir_clobber(lfs3, &file->b.o, LFS3_t_DIRTY);
|
lfs3_omdir_clobber(lfs3, &file->b.o, LFS3_t_DIRTY);
|
||||||
// checkpoint the allocator
|
|
||||||
lfs3_alloc_ckpoint(lfs3);
|
|
||||||
// mark as unsynced in case we fail
|
// mark as unsynced in case we fail
|
||||||
file->b.o.flags |= LFS3_o_UNSYNC;
|
file->b.o.flags |= LFS3_o_UNSYNC;
|
||||||
|
|
||||||
@@ -14123,6 +14139,8 @@ int lfs3_file_fruncate(lfs3_t *lfs3, lfs3_file_t *file, lfs3_off_t size_) {
|
|||||||
lfs3_file_discardleaf(file);
|
lfs3_file_discardleaf(file);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// checkpoint the allocator
|
||||||
|
lfs3_alloc_ckpoint(lfs3);
|
||||||
// fruncate our btree
|
// fruncate our btree
|
||||||
err = lfs3_file_graft(lfs3, file,
|
err = lfs3_file_graft(lfs3, file,
|
||||||
0, lfs3_smax(size - size_, 0),
|
0, lfs3_smax(size - size_, 0),
|
||||||
|
|||||||
Reference in New Issue
Block a user