Fixed another subtle corner cases with noop sync broadcasting

This is an extension of the noop-sync after unrelated write-sync after
desync corner case:

  op                 a state         b state
                     in-sync         in-sync
  desync(b)          in-sync         desync
  write(a)           unsync          desync
  sync(a)            in-sync'        desync
  sync(b)            in-sync         in-sync

But instead of explicitly calling lfsr_file_sync, what if you implicitly
triggered sync through something like a write on a file with the
LFS_O_SYNC flag, but not a normal write, a noop write, write(0)?

If the definition of LFS_O_SYNC is taken literally as "lfsr_file_write
and friends implicitly call lfsr_file_sync after every call", then this
should behave just as if lfsr_file_sync had been called, and
unconditionally broadcast the sync. Since this is the simplest
interpretation, I think this is what we should implement.

Added tests, and adopted this behavior. Fortunately this just involves
some small gotos (https://xkcd.com/292):

            code          stack
  before:  33020           2976
  after:   33026 (+0.0%)   2976 (+0.0%)
This commit is contained in:
Christopher Haster
2024-01-10 19:45:57 -06:00
parent fdc8c8caf1
commit 4e7a68ff08
2 changed files with 125 additions and 4 deletions
+7 -4
View File
@@ -10520,8 +10520,9 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
// underlying file, this means no updating file pos or file size
//
// since we need to test for this, just return early
lfs_size_t written = 0;
if (size == 0) {
return 0;
goto noop;
}
// checkpoint the allocator
@@ -10548,7 +10549,6 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
}
const uint8_t *buffer_ = buffer;
lfs_size_t written = 0;
while (size > 0) {
// bypass buffer?
//
@@ -10631,6 +10631,7 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
// update our pos
file->pos = pos;
noop:;
// flush if requested
//
// this seems unreachable, but it's possible if we transition from
@@ -10873,7 +10874,7 @@ int lfsr_file_truncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
// do nothing if our size does not change
lfs_off_t size = lfsr_file_size_(file);
if (lfsr_file_size_(file) == size_) {
return 0;
goto noop;
}
// checkpoint the allocator
@@ -10942,6 +10943,7 @@ int lfsr_file_truncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
// mark as unsynced
file->flags |= LFS_F_UNSYNCED;
noop:;
// flush if requested
//
// this seems unreachable, but it's possible if we transition from
@@ -10978,7 +10980,7 @@ int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
// do nothing if our size does not change
lfs_off_t size = lfsr_file_size_(file);
if (size == size_) {
return 0;
goto noop;
}
// checkpoint the allocator
@@ -11075,6 +11077,7 @@ int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size_) {
// mark as unsynced
file->flags |= LFS_F_UNSYNCED;
noop:;
// flush if requested
//
// this seems unreachable, but it's possible if we transition from
+118
View File
@@ -2085,6 +2085,124 @@ code = '''
lfsr_unmount(&lfs) => 0;
'''
[cases.test_fsync_desync_wdwdrr_noop_creative]
# there's several clever ways we can noop...
# TODO flush()?
# 0 => sync()
# 1 => write(0)
# 2 => truncate(size)
# 3 => fruncate(size)
defines.NOOP = [0, 1, 2, 3]
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
lfsr_mount(&lfs, CFG) => 0;
// a - writer
// b - syncing writer
// c - reader kept open, recvs updates from a
// d - reader kept closed, checks disk state
lfsr_file_t a;
lfsr_file_t b;
lfsr_file_t c;
lfsr_file_t d;
uint8_t rbuf[256];
lfsr_file_open(&lfs, &a, "jello",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_open(&lfs, &b, "jello", LFS_O_WRONLY | LFS_O_SYNC) => 0;
lfsr_file_open(&lfs, &c, "jello", LFS_O_RDONLY) => 0;
// write to a and sync
lfsr_file_write(&lfs, &a, "hello!", strlen("hello!"))
=> strlen("hello!");
lfsr_file_sync(&lfs, &a) => 0;
// our write should show up in c
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
// and on disk
lfsr_file_open(&lfs, &d, "jello", LFS_O_RDONLY) => 0;
lfsr_file_read(&lfs, &d, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
lfsr_file_close(&lfs, &d) => 0;
// mark b as desync, this should freeze its contents
lfsr_file_desync(&lfs, &b) => 0;
// rewrite a, sync
lfsr_file_rewind(&lfs, &a) => 0;
lfsr_file_write(&lfs, &a, "bonjour!", strlen("bonjour!"))
=> strlen("bonjour!");
lfsr_file_sync(&lfs, &a) => 0;
// a should show up in c
lfsr_file_rewind(&lfs, &c) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("bonjour!");
assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0);
// and on disk
lfsr_file_open(&lfs, &d, "jello", LFS_O_RDONLY) => 0;
lfsr_file_read(&lfs, &d, rbuf, sizeof(rbuf)) => strlen("bonjour!");
assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0);
lfsr_file_close(&lfs, &d) => 0;
// sync b, _creatively_, dragons be here
if (NOOP == 0) {
lfsr_file_sync(&lfs, &b) => 0;
} else if (NOOP == 1) {
lfsr_file_write(&lfs, &b, NULL, 0) => 0;
} else if (NOOP == 2) {
lfsr_file_size(&lfs, &b) => strlen("hello!");
lfsr_file_truncate(&lfs, &b, strlen("hello!")) => 0;
} else if (NOOP == 3) {
lfsr_file_size(&lfs, &b) => strlen("hello!");
lfsr_file_fruncate(&lfs, &b, strlen("hello!")) => 0;
}
// b should show up in c, without a's changes
lfsr_file_rewind(&lfs, &c) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
// and on disk
lfsr_file_open(&lfs, &d, "jello", LFS_O_RDONLY) => 0;
lfsr_file_read(&lfs, &d, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
lfsr_file_close(&lfs, &d) => 0;
// rewrite b, close, desync flag should have been cleared
lfsr_file_rewind(&lfs, &b) => 0;
lfsr_file_write(&lfs, &b, "ohayo!", strlen("ohayo!"))
=> strlen("ohayo!");
lfsr_file_close(&lfs, &b) => 0;
// our write should show up in c
lfsr_file_rewind(&lfs, &c) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("ohayo!");
assert(memcmp(rbuf, "ohayo!", strlen("ohayo!")) == 0);
// and on disk
lfsr_file_open(&lfs, &d, "jello", LFS_O_RDONLY) => 0;
lfsr_file_read(&lfs, &d, rbuf, sizeof(rbuf)) => strlen("ohayo!");
assert(memcmp(rbuf, "ohayo!", strlen("ohayo!")) == 0);
lfsr_file_close(&lfs, &d) => 0;
// rewrite a, close
lfsr_file_rewind(&lfs, &a) => 0;
lfsr_file_write(&lfs, &a, "zdrasti!", strlen("zdrasti!"))
=> strlen("zdrasti!");
lfsr_file_close(&lfs, &a) => 0;
// our write should show up in c
lfsr_file_rewind(&lfs, &c) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("zdrasti!");
assert(memcmp(rbuf, "zdrasti!", strlen("zdrasti!")) == 0);
// and on disk
lfsr_file_open(&lfs, &d, "jello", LFS_O_RDONLY) => 0;
lfsr_file_read(&lfs, &d, rbuf, sizeof(rbuf)) => strlen("zdrasti!");
assert(memcmp(rbuf, "zdrasti!", strlen("zdrasti!")) == 0);
lfsr_file_close(&lfs, &d) => 0;
lfsr_file_close(&lfs, &c) => 0;
lfsr_unmount(&lfs) => 0;
'''
[cases.test_fsync_desync_wdwdrr_append]
code = '''
lfs_t lfs;