Tweaked lfsr_file_open to sync when LFS_O_SYNC

So now the following creates a reg file (instead of just a stickynote):

  lfsr_file_open(&lfs, &file, "test.txt",
          LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL | LFS_O_SYNC) => 0;
  // powerloss!!

  struct lfsr_info info;
  lfsr_stat(&lfs, "test.txt",  &info) => 0; // LFS_ERR_NOENT before
  assert(info.type == LFS_TYPE_REG);

This hopefully results in more intuitive behavior around lfsr_file_open
with LFS_O_SYNC. Which is important as LFS_O_SYNC is often used as an
escape hatch to avoid needing to reason about syncing things when
performance is not a big concern.

Unfortunately this does come with a surprisingly big code/stack cost,
but I'm thinking of putting these flags (LFS_O_FLUSH/LFS_O_SYNC) behind
ifdefs anyways (LFS_MAYBE_SYNC?):

           code          stack          ctx
  before: 35780           2440          640
  after:  35836 (+0.2%)   2488 (+2.0%)  640 (+0.0%)

Also added some more tests to make sure these open+LFS_O_SYNC cases are
explicitly covered:

- test_fsync_sync_o_wrr
- test_fsync_sync_o_wwrr
- test_fsync_desync_o_wdwrr
- test_fsync_resync_o_wdwyrr

This does make a bit of a mess when you combined LFS_O_SYNC +
LFS_O_DESYNC. What exactly should a SYNC + DESYNC file look like?

For now I've just made LFS_O_SYNC + LFS_O_DESYNC behave as if you opened
a file with LFS_O_SYNC and then immediately called lfsr_file_desync on
it. So it doesn't receive broadcasts, but _does_ create the reg file,
and _does_ sync on first write, clearing the desync flag.

But this may be worth revisiting. Maybe LFS_O_DESYNC files shouldn't
have their desync flags cleared unless lfsr_file_sync is explicitly
called? Or maybe LFS_O_SYNC + LFS_O_DESYNC should just be an error?
Unsure...
This commit is contained in:
Christopher Haster
2025-04-25 20:51:17 -05:00
parent b5e503ca85
commit 0f4ad6d842
2 changed files with 611 additions and 4 deletions
+21 -4
View File
@@ -11339,6 +11339,9 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file,
|| !lfsr_o_isexcl(cfg->attrs[i].flags));
}
// mounted with LFS_M_FLUSH/SYNC? implies LFS_O_FLUSH/SYNC
flags |= lfs->flags & (LFS_M_FLUSH | LFS_M_SYNC);
if (!lfsr_o_isrdonly(flags)) {
// prepare our filesystem for writing
int err = lfsr_fs_mkconsistent(lfs);
@@ -11350,8 +11353,6 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file,
// setup file state
file->cfg = cfg;
file->b.o.flags = lfsr_o_settype(flags, LFS_TYPE_REG)
// mounted with LFS_M_FLUSH/SYNC? implies LFS_O_FLUSH/SYNC
| (lfs->flags & (LFS_M_FLUSH | LFS_M_SYNC))
// default to unflushed for orphans/truncated files
| LFS_o_UNFLUSH;
file->pos = 0;
@@ -11441,8 +11442,7 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file,
file->cache.size = 0;
// fetch the file struct and custom attrs
err = lfsr_file_fetch(lfs, file,
lfsr_o_istrunc(file->b.o.flags));
err = lfsr_file_fetch(lfs, file, lfsr_o_istrunc(flags));
if (err) {
goto failed;
}
@@ -11457,6 +11457,23 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file,
// add to tracked mdirs
lfsr_omdir_open(lfs, &file->b.o);
// sync if requested
if (lfsr_o_issync(flags) && !lfsr_o_isrdonly(flags)) {
err = lfsr_file_sync(lfs, file);
if (err) {
lfsr_omdir_close(lfs, &file->b.o);
goto failed;
}
// TODO should we do this for all LFS_O_SYNC operations?
// sync clears the desync flag, so reset it if we're desync
//
// note this matches the behavior of calling lfsr_file_sync and
// then lfsr_file_desync after opening the file
file->b.o.flags |= flags & LFS_O_DESYNC;
}
return 0;
failed:;