Github/littlefs
1
Fork 0
Code Actions Activity

Implemented zombied file handles

Browse Source 
A "zombie file" is a term I just made up to describe what happens when
you remove a file that is currently open.

To match POSIX, the opened file handle should still be available for
reading/writing, even though the file doesn't really exist in the
filesystem anymore.

We don't have inodes, which makes this a bit more complicated, but this
is where scratch files are handy again. By creating a scratch file when
we remove an opened file, we preserve the mid slot for the file's
sprout/shrub. We also mark the opened file as desync, so the existing
orphan reclaimation circuitry kicks in when the last file handle is
closed.

Really the only difference between zombie files and desync files is what
happens when you call lfsr_file_sync:

- Desynced lfsr_file_sync => Become synced, broadcast file state.
- Zombied lfsr_file_sync => Return ENOENT, you can't sync a zombie.

This _is_ a bit different from POSIX, where sync on a removed file
returns 0. I considered returning 0 in this case, but with all the extra
behavior around sync/desync state, I figured returning ENOENT was
clearer at indicating to the user sync is no longer possible.

Worst case, ENOENT is not returned from sync for any other reason, so
users can always treat ENOENT and 0 as the same in higher layers. The
zombie file is already desynced, so close will never error.

---

Implementation wise, zombies get a bit crazy.

Fortunately they add little extra code, but they make up for it by
adding extra subtlety. Zombie files introduce a ton of corner cases, now
even directories can have zombied shrubs.

This means more tests.

- Seemingly unrelated operations need to be able to remove scratch files
  (mkdir, rename, etc).

- UNCREAT state needs to be broadcasted in seemingly unrelated
  operations (mkdir, rename, etc).

- Zombied files need to be copied over during seemingly unrelated rename
  operations.

- And I'm sure more corner cases I'm already forgetting.

One interesting tweak that simplifies things that's worth mentioning is
the change to the implicitly file mid updates on rm in lfsr_mdir_commit.

For non-reg files, an rm attr causes lfsr_mdir_commit to increment the
mid to the next mid in the mtree. This is the correct behavior for dirs,
traversals, etc.

Previously, reg files were a special case that marks the mid as -1. But
by changing this to also increment the mid, as well as set the zombie
flag, upper layers can broadcast zombie changes by simply creating a new
file and then deleting the old file in the same commit.

This seems to Just Work^TM, and avoids needing to do additional state
broadcasting in upper layers, which gets tricky since we may not know
exactly what the new mid is post-mdir-commit.

Downside: The order matters, we need to create the new file first. This
violates the normal delete-then-insert order we use elsewhere to avoid
overflow issues. This isn't that bad here, since we increment by at
most 1. But it is something to be wary of...

Still, this is much better than any other option I can think of right
now.

---

Uh, ignore the test_fscratch_rename* tests for now. I somehow forgot
file renaming was not yet implemented...
This commit is contained in:
Christopher Haster
2024-01-15 15:08:12 -06:00
parent 99156b5573
commit f51dc5c5af
3 changed files with 2672 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+120 -29
View File
@@ -6589,15 +6589,10 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
&& opened->mdir.mid >= attrs[i].rid) {
// removed?
if (opened->mdir.mid < attrs[i].rid - attrs[i].delta) {
// for dir's second mdir (the position mdir), move
// on to the next rid
if (opened->type == LFS_TYPE_DIR) {
opened->mdir.mid = attrs[i].rid;
// for normal mdirs mark as dropped
} else {
opened->mdir.mid = -1;
goto next;
}
// mark as zombied and move onto the next rid, upper
// layers should handle the repercussions
opened->flags |= LFS_F_ZOMBIE;
opened->mdir.mid = attrs[i].rid;
} else {
opened->mdir.mid += attrs[i].delta;
// adjust dir position?
@@ -8286,6 +8281,9 @@ static int lfsr_fs_preparemutation(lfs_t *lfs) {
/// Directory operations ///
// needed in lfsr_mkdir
static inline bool lfsr_f_iszombie(uint32_t flags);
int lfsr_mkdir(lfs_t *lfs, const char *path) {
// prepare our filesystem for writing
int err = lfsr_fs_preparemutation(lfs);
@@ -8295,18 +8293,20 @@ int lfsr_mkdir(lfs_t *lfs, const char *path) {
// lookup our parent
lfsr_mdir_t mdir;
lfsr_tag_t tag;
lfsr_did_t did;
const char *name;
lfs_size_t name_size;
err = lfsr_mtree_pathlookup(lfs, path,
&mdir, NULL,
&mdir, &tag,
&did, &name, &name_size);
if (err && (err != LFS_ERR_NOENT || mdir.mid == -1)) {
return err;
}
// already exists?
if (err != LFS_ERR_NOENT) {
// already exists? note scratch files don't really exist
bool exists = (err != LFS_ERR_NOENT);
if (exists && tag != LFSR_TAG_SCRATCH) {
return LFS_ERR_EXIST;
}
@@ -8386,12 +8386,18 @@ int lfsr_mkdir(lfs_t *lfs, const char *path) {
// commit our new directory into our parent, creating a grm to self-remove
// in case of powerloss
err = lfsr_mdir_commit(lfs, &mdir, LFSR_ATTRS(
LFSR_ATTR(mdir.mid,
LFSR_ATTR(mdir.mid + ((exists) ? 1 : 0),
DIR, +1, CAT(
LFSR_DATA_LEB128(did),
LFSR_DATA_BUF(name, name_size))),
LFSR_ATTR(mdir.mid, DID, 0, LEB128(did_)),
LFSR_ATTR(-1, GRM, 0, GRM(&((lfsr_grm_t){{mdir.mid, -1}})))));
LFSR_ATTR(mdir.mid + ((exists) ? 1 : 0),
DID, 0, LEB128(did_)),
(exists)
? LFSR_ATTR(mdir.mid, RM, -1, NULL())
: LFSR_ATTR_NOOP(),
LFSR_ATTR(-1, GRM, 0, GRM(&((lfsr_grm_t){{
mdir.mid + ((exists) ? 1 : 0),
-1}})))));
if (err) {
goto failed_with_bookmark;
}
@@ -8407,6 +8413,18 @@ int lfsr_mkdir(lfs_t *lfs, const char *path) {
return err;
}
// mark any zombied files as created to avoid a remove from beyond
// the grave
for (lfsr_opened_t *opened = lfs->opened;
opened;
opened = opened->next) {
if (opened->type == LFS_TYPE_REG
&& opened->mdir.mid == mdir.mid) {
LFS_ASSERT(lfsr_f_iszombie(opened->flags));
opened->flags &= ~LFS_F_UNCREAT;
}
}
return 0;
failed_with_bookmark:
@@ -8424,13 +8442,22 @@ int lfsr_remove(lfs_t *lfs, const char *path) {
// lookup our entry
lfsr_mdir_t mdir;
lfsr_tag_t tag;
lfsr_did_t did;
const char *name;
lfs_size_t name_size;
err = lfsr_mtree_pathlookup(lfs, path,
&mdir, &tag,
NULL, NULL, NULL);
&did, &name, &name_size);
if (err) {
return err;
}
// found a zombie?
if (tag == LFSR_TAG_SCRATCH) {
// don't worry, zombies aren't real and cannot hurt you
return LFS_ERR_NOENT;
}
// as funny as it would be, you can't remove the root
if (lfsr_mdir_isroot(&mdir)) {
return LFS_ERR_INVAL;
@@ -8493,14 +8520,47 @@ int lfsr_remove(lfs_t *lfs, const char *path) {
}
}
// are we removing an opened file?
bool zombie = false;
for (lfsr_opened_t *opened = lfs->opened;
opened;
opened = opened->next) {
if (opened->type == LFS_TYPE_REG
&& opened->mdir.mid == mdir.mid) {
zombie = true;
break;
}
}
// remove the metadata entry
err = lfsr_mdir_commit(lfs, &mdir, LFSR_ATTRS(
// create a scratch file if zombied
//
// we use a create+delete here to also clear any attrs
// and trim the entry size
(zombie)
? LFSR_ATTR(mdir.mid+1, SCRATCH, +1, CAT(
LFSR_DATA_LEB128(did),
LFSR_DATA_BUF(name, name_size)))
: LFSR_ATTR_NOOP(),
LFSR_ATTR(mdir.mid, RM, -1, NULL()),
LFSR_ATTR(-1, GRM, 0, GRM(&grm))));
if (err) {
return err;
}
// lfsr_mdir_commit implicitly marks removed files as zombied, but
// we also need to mark them as uncreate to indicate that the mid
// needs to be cleaned up on close
for (lfsr_opened_t *opened = lfs->opened;
opened;
opened = opened->next) {
if (opened->type == LFS_TYPE_REG
&& opened->mdir.mid == mdir.mid) {
opened->flags |= LFS_F_UNCREAT;
}
}
// if we were a directory, we need to clean up, fortunately we can leave
// this up to lfsr_fs_fixgrm
return lfsr_fs_fixgrm(lfs);
@@ -8523,6 +8583,12 @@ int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) {
return err;
}
// found a zombie?
if (old_tag == LFSR_TAG_SCRATCH) {
// don't worry, zombies aren't real and cannot hurt you
return LFS_ERR_NOENT;
}
// as funny as it would be, you can't rename the root
if (lfsr_mdir_isroot(&old_mdir)) {
return LFS_ERR_INVAL;
@@ -8544,6 +8610,7 @@ int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) {
if (err && (err != LFS_ERR_NOENT || new_mdir.mid == -1)) {
return err;
}
// already exists?
bool exists = (err != LFS_ERR_NOENT);
// there are a few cases we need to watch out for
@@ -8561,7 +8628,9 @@ int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) {
} else {
// renaming different types is an error
if (old_tag != new_tag) {
//
// unless we found a scratch file, these don't really exist
if (old_tag != new_tag && new_tag != LFSR_TAG_SCRATCH) {
return (new_tag == LFSR_TAG_DIR)
? LFS_ERR_ISDIR
: LFS_ERR_NOTDIR;
@@ -8629,19 +8698,32 @@ int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) {
// rename our entry, copying all tags associated with the old rid to the
// new rid, while also marking the old rid for removal
err = lfsr_mdir_commit(lfs, &new_mdir, LFSR_ATTRS(
(exists
? LFSR_ATTR(new_mdir.mid, RM, -1, NULL())
: LFSR_ATTR_NOOP()),
LFSR_ATTR(new_mdir.mid,
LFSR_ATTR(new_mdir.mid + ((exists) ? 1 : 0),
TAG(old_tag), +1, CAT(
LFSR_DATA_LEB128(new_did),
LFSR_DATA_BUF(new_name, new_name_size))),
LFSR_ATTR(new_mdir.mid, MOVE, 0, MOVE(&old_mdir)),
LFSR_ATTR(new_mdir.mid + ((exists) ? 1 : 0),
MOVE, 0, MOVE(&old_mdir)),
(exists)
? LFSR_ATTR(new_mdir.mid, RM, -1, NULL())
: LFSR_ATTR_NOOP(),
LFSR_ATTR(-1, GRM, 0, GRM(&grm))));
if (err) {
return err;
}
// mark any zombied files as created to avoid a remove from beyond
// the grave
for (lfsr_opened_t *opened = lfs->opened;
opened;
opened = opened->next) {
if (opened->type == LFS_TYPE_REG
&& opened->mdir.mid == new_mdir.mid) {
LFS_ASSERT(lfsr_f_iszombie(opened->flags));
opened->flags &= ~LFS_F_UNCREAT;
}
}
// we need to clean up any pending grms, fortunately we can leave
// this up to lfsr_fs_fixgrm
return lfsr_fs_fixgrm(lfs);
@@ -9022,6 +9104,10 @@ static inline bool lfsr_f_isuncreat(uint32_t flags) {
return flags & LFS_F_UNCREAT;
}
static inline bool lfsr_f_iszombie(uint32_t flags) {
return flags & LFS_F_ZOMBIE;
}
static inline lfs_off_t lfsr_file_size_(const lfsr_file_t *file) {
return lfs_max32(
file->buffer_pos + file->buffer_size,
@@ -9222,10 +9308,11 @@ int lfsr_file_open(lfs_t *lfs, lfsr_file_t *file,
int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file);
int lfsr_file_close(lfs_t *lfs, lfsr_file_t *file) {
// don't call lfsr_file_sync if we're readonly or desynced
// don't call lfsr_file_sync if we're readonly, desynced, or zombied
int err = 0;
if (!lfsr_o_isrdonly(file->flags)
&& !lfsr_o_isdesync(file->flags)) {
&& !lfsr_o_isdesync(file->flags)
&& !lfsr_f_iszombie(file->flags)) {
err = lfsr_file_sync(lfs, file);
}
@@ -10817,9 +10904,9 @@ failed:;
}
int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// do nothing if our file has been removed
if (file->mdir.mid == -1) {
return 0;
// removed? we can't sync
if (lfsr_f_iszombie(file->flags)) {
return LFS_ERR_NOENT;
}
// first flush any data in our buffer, this is a noop if already
@@ -10852,6 +10939,9 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
|| (file->buffer_size <= lfs->cfg->cache_size
&& file->buffer_size <= lfs->cfg->inline_size
&& file->buffer_size <= lfs->cfg->fragment_size));
// uncreat files must be unsync
LFS_ASSERT(!lfsr_f_isuncreat(file->flags)
|| lfsr_f_isunsync(file->flags));
// don't write to disk if our disk is already in-sync
if (lfsr_f_isunsync(file->flags)) {
@@ -10944,8 +11034,9 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
// notify all files of creation
file_->flags &= ~LFS_F_UNCREAT;
// mark desynced files an unsynced
if (lfsr_o_isdesync(file_->flags)) {
// mark desynced/zombied files an unsynced
if (lfsr_o_isdesync(file_->flags)
|| lfsr_f_iszombie(file_->flags)) {
file_->flags |= LFS_F_UNSYNC;
// update synced files
lfs.h
+1
View File
@@ -140,6 +140,7 @@ enum lfs_open_flags {
LFS_F_UNFLUSH = 0x1000, // File's data does not match storage
LFS_F_UNSYNC = 0x2000, // File's metadata does not match storage
LFS_F_UNCREAT = 0x4000, // File does not exist yet
LFS_F_ZOMBIE = 0x8000, // File has been removed
};
// File seek flags
tests/test_fscratch.toml
+2551 -30
View File
File diff suppressed because it is too large Load Diff