Github/littlefs
1
Fork 0
Code Actions Activity

Implemented file-level error recover in theory

Browse Source 
Not yet tested, thus the "in theory". Testing this is going to be a bit
tricky. Fortunately on-stack copies are a pretty resilient way to
recover from errors.

This comes with an unfortunate, but necessary, code/stack increase:

            code          stack
  before:  31280           2736
  after:   31584 (+1.0%)   2824 (+3.2%)
This commit is contained in:
Christopher Haster
2023-12-04 15:41:48 -06:00
parent 5636895eee
commit 6bfbbae341
1 changed files with 144 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+144 -70
View File
@@ -10280,17 +10280,20 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
return 0; return 0;
} }
// create a copy and track it so our shrub gets updates
lfsr_file_t file_ = *file;
lfsr_mdir_addopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
int err;
// checkpoint the allocator // checkpoint the allocator
lfs_alloc_ack(lfs); lfs_alloc_ack(lfs);
// update pos if we are appending // update pos if we are appending
if (lfsr_file_isappend(file)) { if (lfsr_file_isappend(&file_)) {
file->pos = file->size; file_.pos = file_.size;
} }
lfs_off_t pos = file->pos;
const uint8_t *buffer_ = buffer; const uint8_t *buffer_ = buffer;
int err;
while (size > 0) { while (size > 0) {
// bypass write buffer? // bypass write buffer?
// //
@@ -10298,64 +10301,83 @@ lfs_ssize_t lfsr_file_write(lfs_t *lfs, lfsr_file_t *file,
// strictly necessary, but enforces a more intuitive write order // strictly necessary, but enforces a more intuitive write order
// and avoids weird cases with low-level write strategies // and avoids weird cases with low-level write strategies
// //
if (file->buffer_size == 0 if (file_.buffer_size == 0
&& size >= lfs->cfg->cache_size) { && size >= lfs->cfg->cache_size) {
// TODO can we avoid needing F_UNSYNCED before lfsr_file_flush // TODO can we avoid needing F_UNSYNCED before lfsr_file_flush
file->flags |= LFS_F_UNSYNCED; file_.flags |= LFS_F_UNSYNCED;
err = lfsr_file_flush(lfs, file, err = lfsr_file_flush(lfs, &file_,
pos, buffer_, size); file_.pos, buffer_, size);
if (err) { if (err) {
goto failed; goto failed;
} }
pos += size; file_.pos += size;
buffer_ += size; buffer_ += size;
size -= size; size -= size;
continue; continue;
} }
// try to fill our write buffer // try to fill our write buffer
if (file->buffer_size == 0 //
|| (pos >= file->buffer_pos // This is a bit delicate, since our buffer is shared between our
&& pos <= file->buffer_pos + file->buffer_size // backup and staging file copies, but note:
&& pos < file->buffer_pos + lfs->cfg->cache_size)) { //
// 1. We only write to yet unused buffer memory.
//
// 2. Bypassing the buffer above means we only write to the
// buffer once, and flush at most twice.
//
if (file_.buffer_size == 0
|| (file_.pos >= file_.buffer_pos
&& file_.pos <= file_.buffer_pos + file_.buffer_size
&& file_.pos < file_.buffer_pos + lfs->cfg->cache_size)) {
// unused buffer? we can move this where we need it // unused buffer? we can move this where we need it
if (file->buffer_size == 0) { if (file_.buffer_size == 0) {
file->buffer_pos = pos; file_.buffer_pos = file_.pos;
} }
lfs_size_t d = lfs_min32( lfs_size_t d = lfs_min32(
size, size,
lfs->cfg->cache_size - (pos - file->buffer_pos)); lfs->cfg->cache_size - (file_.pos - file_.buffer_pos));
memcpy(&file->buffer[pos - file->buffer_pos], buffer_, d); memcpy(&file_.buffer[file_.pos - file_.buffer_pos], buffer_, d);
file->buffer_size = lfs_max32( file_.buffer_size = lfs_max32(
file->buffer_size, file_.buffer_size,
pos+d - file->buffer_pos); file_.pos+d - file_.buffer_pos);
pos += d; file_.pos += d;
buffer_ += d; buffer_ += d;
size -= d; size -= d;
file->flags |= LFS_F_UNSYNCED; file_.flags |= LFS_F_UNSYNCED;
continue; continue;
} }
// flush our buffer so the above can't fail // flush our buffer so the above can't fail
err = lfsr_file_flush(lfs, file, err = lfsr_file_flush(lfs, &file_,
file->buffer_pos, file->buffer, file->buffer_size); file_.buffer_pos, file_.buffer, file_.buffer_size);
if (err) { if (err) {
goto failed; goto failed;
} }
file->buffer_pos = 0; file_.buffer_pos = 0;
file->buffer_size = 0; file_.buffer_size = 0;
} }
lfs_size_t written = pos - file->pos; // update size
file->pos = pos; file_.size = lfs_max32(file_.size, file_.pos);
file->size = lfs_max32(file->size, file->pos);
// untrack temporary copy
lfsr_mdir_removeopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
// update file and return amount written
lfs_size_t written = file_.pos - (
(lfsr_file_isappend(&file_)) ? file->size : file->pos);
file_.next = file->next;
*file = file_;
return written; return written;
failed:; failed:;
// untrack temporary copy
lfsr_mdir_removeopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
// mark as errored so lfsr_file_close doesn't write to disk
file->flags |= LFS_F_ERRORED; file->flags |= LFS_F_ERRORED;
return err; return err;
} }
@@ -10427,17 +10449,39 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) {
} else { } else {
// first make sure to flush our buffer // first make sure to flush our buffer
// //
// note that flush does not change the actual file data, so if
// flush succeeds but mdir commit fails it's ok to fall back to
// our flushed state
//
// TODO can we avoid an extra commit here? this may be too complex // TODO can we avoid an extra commit here? this may be too complex
// to be worth doing... // to be worth doing...
// //
if (file->buffer_size > 0) { if (file->buffer_size > 0) {
err = lfsr_file_flush(lfs, file, // TODO dedup into lfsr_file_flush?
file->buffer_pos, file->buffer, file->buffer_size);
// create a copy and track it so our shrub gets updates
lfsr_file_t file_ = *file;
lfsr_mdir_addopened(lfs, LFS_TYPE_REG,
(lfsr_openedmdir_t*)&file_);
// flush
err = lfsr_file_flush(lfs, &file_,
file_.buffer_pos, file_.buffer, file_.buffer_size);
if (err) { if (err) {
// make sure to untrack temporary copy
lfsr_mdir_removeopened(lfs, LFS_TYPE_REG,
(lfsr_openedmdir_t*)&file_);
goto failed; goto failed;
} }
file->buffer_pos = 0; file_.buffer_pos = 0;
file->buffer_size = 0; file_.buffer_size = 0;
// untrack temporary copy
lfsr_mdir_removeopened(lfs, LFS_TYPE_REG,
(lfsr_openedmdir_t*)&file_);
// update file
file_.next = file->next;
*file = file_;
} }
// now commit our file's metadata // now commit our file's metadata
@@ -10527,47 +10571,62 @@ int lfsr_file_truncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size) {
return 0; return 0;
} }
// TODO make this recoverable on failure // create a copy and track it so our shrub gets updates
lfsr_file_t file_ = *file;
lfsr_mdir_addopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
int err;
// mark as unsynced before we commit anything // mark as unsynced before we commit anything
file->flags |= LFS_F_UNSYNCED; file_.flags |= LFS_F_UNSYNCED;
// TODO we should also revert to sprout even if data is not already // TODO we should also revert to sprout even if data is not already
// in buffer // in buffer
// //
// if our truncated file is contained entirely in our buffer, // if our truncated file is contained entirely in our buffer,
// revert to a sprout // revert to a sprout
lfs_off_t buffer_pos = lfs_min32(file->buffer_pos, size); lfs_off_t buffer_pos = lfs_min32(file_.buffer_pos, size);
lfs_size_t buffer_size = lfs_min32( lfs_size_t buffer_size = lfs_min32(
file->buffer_size, file_.buffer_size,
size - lfs_min32(file->buffer_pos, size)); size - lfs_min32(file_.buffer_pos, size));
if (buffer_size >= size) { if (buffer_size >= size) {
file->u.bsprout = LFSR_FILE_BNULL(); file_.u.bsprout = LFSR_FILE_BNULL();
// TODO, wait, could we just update file->size and leave it to // TODO, wait, could we just update file_.size and leave it to
// lfsr_file_sync to update the shrub? // lfsr_file_sync to update the shrub?
// otherwise, we need to modify our sprout/bptr/bshrub/btree // otherwise, we need to modify our sprout/bptr/bshrub/btree
} else { } else {
int err = lfsr_file_carve(lfs, file, err = lfsr_file_carve(lfs, &file_,
lfs_min32(file->size, size), lfs_min32(file_.size, size),
file->size - lfs_min32(file->size, size), file_.size - lfs_min32(file_.size, size),
+size - file->size, +size - file_.size,
LFSR_TAG_DATA, LFSR_DATA_NULL()); LFSR_TAG_DATA, LFSR_DATA_NULL());
if (err) { if (err) {
return err; goto failed;
} }
} }
LFS_ASSERT(!lfsr_file_isbshruborbtree(file) LFS_ASSERT(!lfsr_file_isbshruborbtree(&file_)
|| lfsr_file_uweight(file) > 0); || lfsr_file_uweight(&file_) > 0);
// update our buffer // update our buffer
file->buffer_pos = buffer_pos; file_.buffer_pos = buffer_pos;
file->buffer_size = buffer_size; file_.buffer_size = buffer_size;
// update our internal file size // update our internal file size
file->size = size; file_.size = size;
// untrack temporary copy
lfsr_mdir_removeopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
// update file
file_.next = file->next;
*file = file_;
return 0; return 0;
failed:;
// untrack temporary copy
lfsr_mdir_removeopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
// mark as errored so lfsr_file_close doesn't write to disk
file->flags |= LFS_F_ERRORED;
return err;
} }
int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size) { int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size) {
@@ -10581,21 +10640,24 @@ int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size) {
return 0; return 0;
} }
// TODO make this recoverable on failure // create a copy and track it so our shrub gets updates
lfsr_file_t file_ = *file;
lfsr_mdir_addopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
int err;
// mark as unsynced before we commit anything // mark as unsynced before we commit anything
file->flags |= LFS_F_UNSYNCED; file_.flags |= LFS_F_UNSYNCED;
// TODO we should also revert to sprout even if data is not already // TODO we should also revert to sprout even if data is not already
// in buffer // in buffer
// //
// if our truncated file is contained entirely in our buffer, // if our truncated file is contained entirely in our buffer,
// revert to a sprout // revert to a sprout
lfs_size_t buffer_size = file->buffer_size - lfs_min32( lfs_size_t buffer_size = file_.buffer_size - lfs_min32(
lfs_smax32(file->size - size - file->buffer_pos, 0), lfs_smax32(file_.size - size - file_.buffer_pos, 0),
file->buffer_size); file_.buffer_size);
if (buffer_size >= size) { if (buffer_size >= size) {
file->u.bsprout = LFSR_FILE_BNULL(); file_.u.bsprout = LFSR_FILE_BNULL();
// otherwise, we need to modify our sprout/bptr/bshrub/btree // otherwise, we need to modify our sprout/bptr/bshrub/btree
} else { } else {
@@ -10603,34 +10665,46 @@ int lfsr_file_fruncate(lfs_t *lfs, lfsr_file_t *file, lfs_off_t size) {
// merged somehow? // merged somehow?
// //
// revert shrubs if they go to zero // revert shrubs if they go to zero
if ((lfs_soff_t)(file->size - size) if ((lfs_soff_t)(file_.size - size)
>= (lfs_soff_t)lfsr_file_uweight(file)) { >= (lfs_soff_t)lfsr_file_uweight(&file_)) {
file->u.bsprout = LFSR_FILE_BNULL(); file_.u.bsprout = LFSR_FILE_BNULL();
} else { } else {
int err = lfsr_file_carve(lfs, file, err = lfsr_file_carve(lfs, &file_,
0, 0,
lfs_smax32(file->size - size, 0), lfs_smax32(file_.size - size, 0),
+size - file->size, +size - file_.size,
LFSR_TAG_DATA, LFSR_DATA_NULL()); LFSR_TAG_DATA, LFSR_DATA_NULL());
if (err) { if (err) {
return err; goto failed;
} }
} }
} }
LFS_ASSERT(!lfsr_file_isbshruborbtree(file) LFS_ASSERT(!lfsr_file_isbshruborbtree(&file_)
|| lfsr_file_uweight(file) > 0); || lfsr_file_uweight(&file_) > 0);
// update our buffer // update our buffer
file->buffer_pos -= lfs_smin32(file->size - size, file->buffer_pos); file_.buffer_pos -= lfs_smin32(file_.size - size, file_.buffer_pos);
memmove(file->buffer, memmove(file_.buffer,
file->buffer + (file->buffer_size - buffer_size), file_.buffer + (file_.buffer_size - buffer_size),
buffer_size); buffer_size);
file->buffer_size = buffer_size; file_.buffer_size = buffer_size;
// update our internal file size // update our internal file size
file->size = size; file_.size = size;
// untrack temporary copy
lfsr_mdir_removeopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
// update file
file_.next = file->next;
*file = file_;
return 0; return 0;
failed:;
// untrack temporary copy
lfsr_mdir_removeopened(lfs, LFS_TYPE_REG, (lfsr_openedmdir_t*)&file_);
// mark as errored so lfsr_file_close doesn't write to disk
file->flags |= LFS_F_ERRORED;
return err;
} }