From f9c2fd93f289a3d8f23ffebc6f4089260c417258 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 29 Jan 2020 22:05:58 -0600 Subject: [PATCH] Removed file outlining on ENOSPC in lfs_file_sync This was initially added as protection against the case where a file grew to no longer fit in a metadata-pair. While in most cases this should be caught by the math in lfs_file_write, it doesn't handle a problem that can happen if the files metadata is large enough that even small inline files can't fit. This can happen if you combine a small block size with large file names and many custom attributes. But trying to outline on ENOSPC creates creates a lot of problems. If we are actually low on space, this is one of the worst things we can do. Inline files take up less space than CTZ skip-lists, but inline files are rendered useless if we outline inline files as soon as we run low on space. On top of this, the outlining logic tries multiple mdir commits if it gets ENOSPC, which can hide errors if ENOSPC is returned for other reasons. In a perfect world, we would be using a different error code for no-room-in-metadata-pair, and no-blocks-on-disk. For now I've removed the outlining logic and we will need to figure out how to handle this situation more robustly. --- lfs.c | 47 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/lfs.c b/lfs.c index 9785092..be6612f 100644 --- a/lfs.c +++ b/lfs.c @@ -1631,33 +1631,17 @@ static int lfs_dir_compact(lfs_t *lfs, // TODO huh? LFS_ASSERT(commit.off % lfs->cfg->prog_size == 0); - // update gstate - lfs->gdelta = (lfs_gstate_t){0}; - if (!relocated) { - lfs->gdisk = lfs->gstate; - } - - // TODO here?? - if (relocated) { - // update references if we relocated - LFS_DEBUG("Relocating %"PRIx32" %"PRIx32" -> %"PRIx32" %"PRIx32, - oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]); - err = lfs_fs_relocate(lfs, oldpair, dir->pair); - if (err) { - // TODO make better - dir->pair[1] = oldpair[1]; // - return err; - } - LFS_DEBUG("Relocated %"PRIx32" %"PRIx32" -> %"PRIx32" %"PRIx32, - oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]); - } - // successful compaction, swap dir pair to indicate most recent lfs_pair_swap(dir->pair); dir->rev = nrev; dir->count = end - begin; dir->off = commit.off; dir->etag = commit.ptag; + // update gstate + lfs->gdelta = (lfs_gstate_t){0}; + if (!relocated) { + lfs->gdisk = lfs->gstate; + } } break; @@ -1685,6 +1669,21 @@ relocate: continue; } + // TODO here?? + if (relocated) { + // update references if we relocated + LFS_DEBUG("Relocating %"PRIx32" %"PRIx32" -> %"PRIx32" %"PRIx32, + oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]); + int err = lfs_fs_relocate(lfs, oldpair, dir->pair); + if (err) { + // TODO make better + //dir->pair[1] = oldpair[1]; // + return err; + } + LFS_DEBUG("Relocated %"PRIx32" %"PRIx32" -> %"PRIx32" %"PRIx32, + oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]); + } + return 0; } @@ -2739,9 +2738,9 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) { {LFS_MKTAG(LFS_FROM_USERATTRS, file->id, file->cfg->attr_count), file->cfg->attrs})); if (err) { - if (err == LFS_ERR_NOSPC && (file->flags & LFS_F_INLINE)) { - goto relocate; - } +// if (err == LFS_ERR_NOSPC && (file->flags & LFS_F_INLINE)) { +// goto relocate; +// } file->flags |= LFS_F_ERRED; LFS_TRACE("lfs_file_sync -> %d", err); return err;