Github/littlefs
1
Fork 0
Code Actions Activity

Relaxed asserts, allow syncing rdonly files, error on unsync

Browse Source 
This is a compromise on consistency and not breaking expected
invariants.

The problem: rdonly files can become unsynced:

1. file is opened rdonly + desync
2. the same file is opened and written to
3. we try to sync our original file handle

What we want:

1. sync should ensure disk + files are in-sync
2. rdonly implies sync should not write to disk

Without desync, and in other systems, this is not a problem, because
rdonly files can never become unsynced.

But with desync, a state (albiet a roundabout one) can be reached where
we can't satisfy both of these invariants.

I wanted to just assert on syncing a rdonly file, but this is supported
on POSIX and other systems, and it makes sense that you would want to
unconditionally call sync in certain circumstances (ensuring close can't
write to disk for example).

So adopts the approach of allowing flush and sync on rdonly files when
possible, and when not possible, sync simply returns LFS_ERR_INVAL and
makes it the user's problem.

For the above example, this has the side effect of making the rdonly file
desync again, so close can complete without touching disk.

As a plus, a desynced rdonly file can now be used to test if a file has
been written to. Though I'm not sure when this would be useful... Or
if it's a good idea to suggest this use of the API...
This commit is contained in:
Christopher Haster
2024-01-10 22:50:45 -06:00
parent 122864f4b6
commit 1344d416d2
2 changed files with 122 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+21 -11
View File
@@ -10659,11 +10659,12 @@ failed:;
}
int lfsr_file_flush(lfs_t *lfs, lfsr_file_t *file) {
// flushing readonly files is not supported
//
// in theory we could make this a noop, but that would be inconsistent
// with lfsr_file_sync
LFS_ASSERT(lfsr_o_iswriteable(file->flags));
// readonly files should do nothing
LFS_ASSERT(lfsr_o_iswriteable(file->flags)
|| !lfsr_f_isunflushed(file->flags)
|| (lfsr_file_size_(file) <= lfs->cfg->cache_size
&& lfsr_file_size_(file) <= lfs->cfg->inline_size
&& lfsr_file_size_(file) <= lfs->cfg->fragment_size));
// do nothing if our file is already flushed
if (!lfsr_f_isunflushed(file->flags)) {
@@ -10707,12 +10708,6 @@ failed:;
}
int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// syncing readonly files is not supported
//
// in theory we could make this a noop, but then syncing desynced
// readonly files would require disk writes
LFS_ASSERT(lfsr_o_iswriteable(file->flags));
// do nothing if our file has been removed
if (file->mdir.mid == -1) {
return 0;
@@ -10751,6 +10746,21 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// don't write to disk if our disk is already in-sync
if (lfsr_f_isunsynced(file->flags)) {
// readonly files should do nothing
//
// but readonly files _can_ end up unsynced, in the roundabout
// case where:
//
// 1. a file is opened rdonly + desync
// 2. the same file is opened and written to
// 3. we try to sync our original file handle
//
// the best thing we can do in this case is return an error
if (!lfsr_o_iswriteable(file->flags)) {
err = LFS_ERR_INVAL;
goto failed;
}
// checkpoint the allocator again
lfs_alloc_ckpoint(lfs);
tests/test_fsync.toml
+101
View File
@@ -2085,6 +2085,107 @@ code = '''
lfsr_unmount(&lfs) => 0;
'''
[cases.test_fsync_desync_wrrd_noop]
code = '''
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
lfsr_mount(&lfs, CFG) => 0;
// a - writer
// b - reader kept open, recvs updates from a
// c - desynced reader
lfsr_file_t a;
lfsr_file_t b;
lfsr_file_t c;
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_RDONLY) => 0;
lfsr_file_open(&lfs, &c, "jello", LFS_O_RDONLY | LFS_O_DESYNC) => 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 b
lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
// but not in c
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => 0;
// reopen c, should now be up to date
lfsr_file_close(&lfs, &c) => 0;
lfsr_file_open(&lfs, &c, "jello", LFS_O_RDONLY | LFS_O_DESYNC) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
// rewrite a and sync
lfsr_file_rewind(&lfs, &a) => 0;
lfsr_file_write(&lfs, &a, "bonjour!", strlen("bonjour!"))
=> strlen("bonjour!");
lfsr_file_sync(&lfs, &a) => 0;
// our write should show up in b
lfsr_file_rewind(&lfs, &b) => 0;
lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("bonjour!");
assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0);
// but not in c
lfsr_file_rewind(&lfs, &c) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
// sync b, this should be a noop
lfsr_file_sync(&lfs, &b) => 0;
// our write should show up in b
lfsr_file_rewind(&lfs, &b) => 0;
lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("bonjour!");
assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0);
// but not in c
lfsr_file_rewind(&lfs, &c) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
// as if things couldn't get weirder
//
// what do you think should happend if we sync c?
//
// in theory, this should update a + b + disk, but that would require
// writing to disk... instead we just error
lfsr_file_sync(&lfs, &c) => LFS_ERR_INVAL;
// our write should show up in b
lfsr_file_rewind(&lfs, &b) => 0;
lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("bonjour!");
assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0);
// but not in c
lfsr_file_rewind(&lfs, &c) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!");
assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0);
// reopen c, should now be up to date
lfsr_file_close(&lfs, &c) => 0;
lfsr_file_open(&lfs, &c, "jello", LFS_O_RDONLY | LFS_O_DESYNC) => 0;
lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("bonjour!");
assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0);
// we _are_ allowed to sync c if it results in a noop
lfsr_file_sync(&lfs, &c) => 0;
// our write should show up in b
lfsr_file_rewind(&lfs, &b) => 0;
lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("bonjour!");
assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0);
// and 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);
lfsr_file_close(&lfs, &a) => 0;
lfsr_file_close(&lfs, &b) => 0;
lfsr_file_close(&lfs, &c) => 0;
lfsr_unmount(&lfs) => 0;
'''
[cases.test_fsync_desync_wdwdrr_append]
code = '''
lfs_t lfs;