Github/littlefs
1
Fork 0
Code Actions Activity

Revisited zero-length file sync path

Browse Source 
This needed a second pass. Changes:

- Small file flushes are no longer limited to LFS3_o_UNFLUSH, which
  should avoid bshrubs/btrees being written for small files with
  complicated seek+writes. Now, any file small enough is converted
  to a small file when we would need to flush.

  This does _not_ flush small unsync files that don't need to be
  flushed, though I'm not exactly sure how that would happen (broadcast
  from file with a different cache size?)

  I think this was a regression from previous logic.

- discardbshrub/discardbleaf moved into lfs3_file_sync_, otherwise
  we risk discarding the bshrub/bleaf without setting UNSYNC.

  This keeps all the state changing logic together.

- We now use lfs3_file_size_ == 0 as the decision for committing bnulls.

  size_ == 0 implies bnull, and this avoids the extra headache of
  checking for pending small file flush.

Note the ultimate decision on if the file is small is still left up to
lfs3_file_sync. lfs3_file_sync_ just relies on the UNFLUSH + UNCRYST +
UNGRAFT checks to do the last minute small file flush (aside from
asserts).

The UNFLUSH + UNCRYST + UNGRAFT checks look a bit messy, but keep in
mind these optimize to a single bitmask.

Saves a tiny bit of code:

           code          stack          ctx
  before: 37856           2416          636
  after:  37836 (-0.1%)   2408 (-0.3%)  636 (+0.0%)
This commit is contained in:
Christopher Haster
2025-06-21 19:36:22 -05:00
parent 7a6aad3cc8
commit e7c7a81cfe
1 changed files with 25 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs3.c
+25 -30
View File
@@ -13220,6 +13220,10 @@ static int lfs3_file_sync_(lfs3_t *lfs3, lfs3_file_t *file,
// small unflushed files must be unsync
LFS3_ASSERT(!lfs3_o_isunflush(file->b.o.flags)
|| lfs3_o_isunsync(file->b.o.flags));
LFS3_ASSERT(!lfs3_o_isuncryst(file->b.o.flags)
|| lfs3_o_isunsync(file->b.o.flags));
LFS3_ASSERT(!lfs3_o_isungraft(file->b.o.flags)
|| lfs3_o_isunsync(file->b.o.flags));
// pending metadata changes?
if (lfs3_o_isunsync(file->b.o.flags)) {
@@ -13247,12 +13251,16 @@ static int lfs3_file_sync_(lfs3_t *lfs3, lfs3_file_t *file,
}
// pending small file flush?
if (lfs3_o_isunflush(file->b.o.flags)) {
if (lfs3_o_isunflush(file->b.o.flags)
|| lfs3_o_isuncryst(file->b.o.flags)
|| lfs3_o_isungraft(file->b.o.flags)) {
// this only works if the file is entirely in our cache
LFS3_ASSERT(file->cache.pos == 0);
LFS3_ASSERT(file->cache.size == lfs3_file_size_(file));
// bshrub should be discarded here
LFS3_ASSERT(lfs3_file_weight_(file) == 0);
// discard any lingering bshrub state
lfs3_file_discardleaf(file);
lfs3_file_discardbshrub(file);
// build a small shrub commit
if (file->cache.size > 0) {
@@ -13270,30 +13278,30 @@ static int lfs3_file_sync_(lfs3_t *lfs3, lfs3_file_t *file,
}
}
// zero size files should have no bshrub/btree
LFS3_ASSERT(lfs3_file_size_(file) > 0
|| lfs3_bshrub_isbnull(&file->b));
// TODO is all this logic optimal for zero-length files?
// make sure data is on-disk before committing metadata
if (!lfs3_bshrub_isbnull(&file->b)
&& !lfs3_o_isunflush(file->b.o.flags)) {
if (lfs3_file_size_(file) > 0
&& !lfs3_o_isunflush(file->b.o.flags)
&& !lfs3_o_isuncryst(file->b.o.flags)
&& !lfs3_o_isungraft(file->b.o.flags)) {
int err = lfs3_bd_sync(lfs3);
if (err) {
return err;
}
}
// zero size files should have no bshrub/btree
LFS3_ASSERT(lfs3_file_size_(file) > 0
|| lfs3_bshrub_isbnull(&file->b));
// no bshrub/btree?
if (lfs3_bshrub_isbnull(&file->b)
&& !(lfs3_o_isunflush(file->b.o.flags)
&& file->cache.size > 0)) {
if (lfs3_file_size_(file) == 0) {
rattrs[rattr_count++] = LFS3_RATTR(
LFS3_TAG_RM | LFS3_TAG_MASK8 | LFS3_TAG_STRUCT, 0);
// bshrub?
} else if (lfs3_bshrub_isbshrub(&file->b)
|| (lfs3_o_isunflush(file->b.o.flags)
&& file->cache.size > 0)) {
|| lfs3_o_isunflush(file->b.o.flags)
|| lfs3_o_isuncryst(file->b.o.flags)
|| lfs3_o_isungraft(file->b.o.flags)) {
rattrs[rattr_count++] = LFS3_RATTR_SHRUB(
LFS3_TAG_MASK8 | LFS3_TAG_BSHRUB, 0,
// note we use the staged trunk here
@@ -13471,23 +13479,10 @@ int lfs3_file_sync(lfs3_t *lfs3, lfs3_file_t *file) {
// though don't flush quite yet if our file is small and can be
// combined with sync in a single commit
int err;
if (lfs3_o_isunflush(file->b.o.flags)
&& file->cache.size == lfs3_file_size_(file)
if (!(file->cache.size == lfs3_file_size_(file)
&& file->cache.size <= lfs3->cfg->inline_size
&& file->cache.size <= lfs3->cfg->fragment_size
&& file->cache.size < lfs3->cfg->crystal_thresh) {
// size == size implies pos == 0
LFS3_ASSERT(file->cache.pos == 0);
// map any weird bshrub/btree states to unflush
if (lfs3_o_isuncryst(file->b.o.flags)
|| lfs3_o_isungraft(file->b.o.flags)) {
file->b.o.flags |= LFS3_o_UNFLUSH;
}
// discard any lingering bshrub state
lfs3_file_discardleaf(file);
lfs3_file_discardbshrub(file);
} else {
&& file->cache.size < lfs3->cfg->crystal_thresh)) {
err = lfs3_file_flush(lfs3, file);
if (err) {
goto failed;