From 2936514b5e0e4df519fc853127bb0adde4065249 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 7 Oct 2017 09:19:08 -0500 Subject: [PATCH] Added atomic move using dirty tag in entry type The "move problem" has been present in littlefs for a while, but I haven't come across a solution worth implementing for various reasons. The problem is simple: how do we move directory entries across directories atomically? Since multiple directory entries are involved, we can't rely entirely on the atomic block updates. It ends up being a bit of a puzzle. To make the problem more complicated, any directory block update can fail due to wear, and cause the directory block to need to be relocated. This happens rarely, but brings a large number of corner cases. --- The solution in this patch is simple: 1. Mark source as "moving" 2. Copy source to destination 3. Remove source If littlefs ever runs into a "moving" entry, that means a power loss occured during a move. Either the destination entry exists or it doesn't. In this case we just search the entire filesystem for the destination entry. This is expensive, however the chance of a power loss during a move is relatively low. --- Makefile | 2 +- lfs.c | 232 +++++++++++++++++++++++++++++++++++++------- lfs.h | 6 +- tests/test_move.sh | 236 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 438 insertions(+), 38 deletions(-) create mode 100755 tests/test_move.sh diff --git a/Makefile b/Makefile index 2c9e8bf..bd5bd90 100644 --- a/Makefile +++ b/Makefile @@ -32,7 +32,7 @@ size: $(OBJ) .SUFFIXES: test: test_format test_dirs test_files test_seek test_parallel \ - test_alloc test_paths test_orphan test_corrupt + test_alloc test_paths test_orphan test_move test_corrupt test_%: tests/test_%.sh ./$< diff --git a/lfs.c b/lfs.c index dc4a955..222d15a 100644 --- a/lfs.c +++ b/lfs.c @@ -253,9 +253,11 @@ int lfs_traverse(lfs_t *lfs, int (*cb)(void*, lfs_block_t), void *data); static int lfs_pred(lfs_t *lfs, const lfs_block_t dir[2], lfs_dir_t *pdir); static int lfs_parent(lfs_t *lfs, const lfs_block_t dir[2], lfs_dir_t *parent, lfs_entry_t *entry); +static int lfs_moved(lfs_t *lfs, const void *e); static int lfs_relocate(lfs_t *lfs, const lfs_block_t oldpair[2], const lfs_block_t newpair[2]); int lfs_deorphan(lfs_t *lfs); +int lfs_deduplicate(lfs_t *lfs); /// Block allocator /// @@ -722,8 +724,8 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir, return err; } - if ((entry->d.type != LFS_TYPE_REG && - entry->d.type != LFS_TYPE_DIR) || + if (((0x7f & entry->d.type) != LFS_TYPE_REG && + (0x7f & entry->d.type) != LFS_TYPE_DIR) || entry->d.nlen != pathlen) { continue; } @@ -741,6 +743,16 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir, } } + // check that entry has not been moved + if (entry->d.type & 0x80) { + int moved = lfs_moved(lfs, &entry->d.u); + if (moved < 0 || moved) { + return (moved < 0) ? moved : LFS_ERR_NOENT; + } + + entry->d.type &= ~0x80; + } + pathname += pathlen; pathname += strspn(pathname, "/"); if (pathname[0] == '\0') { @@ -764,6 +776,14 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir, /// Top level directory operations /// int lfs_mkdir(lfs_t *lfs, const char *path) { + // make sure directories are clean + if (!lfs->deduplicated) { + int err = lfs_deduplicate(lfs); + if (err) { + return err; + } + } + // fetch parent directory lfs_dir_t cwd; int err = lfs_dir_fetch(lfs, &cwd, lfs->root); @@ -880,10 +900,26 @@ int lfs_dir_read(lfs_t *lfs, lfs_dir_t *dir, struct lfs_info *info) { return (err == LFS_ERR_NOENT) ? 0 : err; } - if (entry.d.type == LFS_TYPE_REG || - entry.d.type == LFS_TYPE_DIR) { - break; + if ((0x7f & entry.d.type) != LFS_TYPE_REG && + (0x7f & entry.d.type) != LFS_TYPE_DIR) { + continue; } + + // check that entry has not been moved + if (entry.d.type & 0x80) { + int moved = lfs_moved(lfs, &entry.d.u); + if (moved < 0) { + return moved; + } + + if (moved) { + continue; + } + + entry.d.type &= ~0x80; + } + + break; } info->type = entry.d.type; @@ -1113,6 +1149,14 @@ static int lfs_index_traverse(lfs_t *lfs, /// Top level file operations /// int lfs_file_open(lfs_t *lfs, lfs_file_t *file, const char *path, int flags) { + // make sure directories are clean + if ((flags & 3) != LFS_O_RDONLY && !lfs->deduplicated) { + int err = lfs_deduplicate(lfs); + if (err) { + return err; + } + } + // allocate entry for file if it doesn't exist lfs_dir_t cwd; int err = lfs_dir_fetch(lfs, &cwd, lfs->root); @@ -1598,6 +1642,14 @@ int lfs_stat(lfs_t *lfs, const char *path, struct lfs_info *info) { } int lfs_remove(lfs_t *lfs, const char *path) { + // make sure directories are clean + if (!lfs->deduplicated) { + int err = lfs_deduplicate(lfs); + if (err) { + return err; + } + } + lfs_dir_t cwd; int err = lfs_dir_fetch(lfs, &cwd, lfs->root); if (err) { @@ -1654,6 +1706,14 @@ int lfs_remove(lfs_t *lfs, const char *path) { } int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { + // make sure directories are clean + if (!lfs->deduplicated) { + int err = lfs_deduplicate(lfs); + if (err) { + return err; + } + } + // find old entry lfs_dir_t oldcwd; int err = lfs_dir_fetch(lfs, &oldcwd, lfs->root); @@ -1667,6 +1727,14 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { return err; } + // mark as moving + oldentry.d.type |= 0x80; + err = lfs_dir_update(lfs, &oldcwd, &oldentry, NULL); + if (err) { + return err; + } + oldentry.d.type &= ~0x80; + // allocate new entry lfs_dir_t newcwd; err = lfs_dir_fetch(lfs, &newcwd, lfs->root); @@ -1716,35 +1784,6 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { } } - // fetch again in case newcwd == oldcwd - err = lfs_dir_fetch(lfs, &oldcwd, oldcwd.pair); - if (err) { - return err; - } - - err = lfs_dir_find(lfs, &oldcwd, &oldentry, &oldpath); - if (err) { - return err; - } - - // remove from old location - err = lfs_dir_remove(lfs, &oldcwd, &oldentry); - if (err) { - return err; - } - - // shift over any files that are affected - for (lfs_file_t *f = lfs->files; f; f = f->next) { - if (lfs_paircmp(f->pair, oldcwd.pair) == 0) { - if (f->poff == oldentry.off) { - f->pair[0] = 0xffffffff; - f->pair[1] = 0xffffffff; - } else if (f->poff > oldentry.off) { - f->poff -= lfs_entry_size(&oldentry); - } - } - } - // if we were a directory, just run a deorphan step, this should // collect us, although is expensive if (prevexists && preventry.d.type == LFS_TYPE_DIR) { @@ -1754,6 +1793,12 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { } } + // just deduplicate + err = lfs_deduplicate(lfs); + if (err) { + return err; + } + return 0; } @@ -1802,6 +1847,7 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) { lfs->root[1] = 0xffffffff; lfs->files = NULL; lfs->deorphaned = false; + lfs->deduplicated = false; return 0; } @@ -1979,7 +2025,7 @@ int lfs_traverse(lfs_t *lfs, int (*cb)(void*, lfs_block_t), void *data) { } dir.off += lfs_entry_size(&entry); - if ((0xf & entry.d.type) == (0xf & LFS_TYPE_REG)) { + if ((0x70 & entry.d.type) == (0x70 & LFS_TYPE_REG)) { int err = lfs_index_traverse(lfs, &lfs->rcache, NULL, entry.d.u.file.head, entry.d.u.file.size, cb, data); if (err) { @@ -2069,7 +2115,7 @@ static int lfs_parent(lfs_t *lfs, const lfs_block_t dir[2], break; } - if (((0xf & entry->d.type) == (0xf & LFS_TYPE_DIR)) && + if (((0x70 & entry->d.type) == (0x70 & LFS_TYPE_DIR)) && lfs_paircmp(entry->d.u.dir, dir) == 0) { return true; } @@ -2079,6 +2125,46 @@ static int lfs_parent(lfs_t *lfs, const lfs_block_t dir[2], return false; } +static int lfs_moved(lfs_t *lfs, const void *e) { + if (lfs_pairisnull(lfs->root)) { + return 0; + } + + // skip superblock + lfs_dir_t cwd; + int err = lfs_dir_fetch(lfs, &cwd, (const lfs_block_t[2]){0, 1}); + if (err) { + return err; + } + + // iterate over all directory directory entries + lfs_entry_t entry; + while (!lfs_pairisnull(cwd.d.tail)) { + int err = lfs_dir_fetch(lfs, &cwd, cwd.d.tail); + if (err) { + return err; + } + + while (true) { + int err = lfs_dir_next(lfs, &cwd, &entry); + if (err && err != LFS_ERR_NOENT) { + return err; + } + + if (err == LFS_ERR_NOENT) { + break; + } + + if (!(0x80 & entry.d.type) && + memcmp(&entry.d.u, e, sizeof(entry.d.u)) == 0) { + return true; + } + } + } + + return false; +} + static int lfs_relocate(lfs_t *lfs, const lfs_block_t oldpair[2], const lfs_block_t newpair[2]) { // find parent @@ -2197,3 +2283,77 @@ int lfs_deorphan(lfs_t *lfs) { return 0; } +int lfs_deduplicate(lfs_t *lfs) { + lfs->deduplicated = true; + + if (lfs_pairisnull(lfs->root)) { + return 0; + } + + // skip superblock + lfs_dir_t cwd; + int err = lfs_dir_fetch(lfs, &cwd, (const lfs_block_t[2]){0, 1}); + if (err) { + return err; + } + + // iterate over all directory directory entries + lfs_entry_t entry; + while (!lfs_pairisnull(cwd.d.tail)) { + int err = lfs_dir_fetch(lfs, &cwd, cwd.d.tail); + if (err) { + return err; + } + + while (true) { + int err = lfs_dir_next(lfs, &cwd, &entry); + if (err && err != LFS_ERR_NOENT) { + return err; + } + + if (err == LFS_ERR_NOENT) { + break; + } + + // found moved entry + if (entry.d.type & 0x80) { + int moved = lfs_moved(lfs, &entry.d.u); + if (moved < 0) { + return moved; + } + + if (moved) { + LFS_DEBUG("Found move %d %d", + entry.d.u.dir[0], entry.d.u.dir[1]); + int err = lfs_dir_remove(lfs, &cwd, &entry); + if (err) { + return err; + } + + // shift over any files that are affected + for (lfs_file_t *f = lfs->files; f; f = f->next) { + if (lfs_paircmp(f->pair, cwd.pair) == 0) { + if (f->poff == entry.off) { + f->pair[0] = 0xffffffff; + f->pair[1] = 0xffffffff; + } else if (f->poff > entry.off) { + f->poff -= lfs_entry_size(&entry); + } + } + } + } else { + LFS_DEBUG("Found partial move %d %d", + entry.d.u.dir[0], entry.d.u.dir[1]); + entry.d.type &= ~0x80; + int err = lfs_dir_update(lfs, &cwd, &entry, NULL); + if (err) { + return err; + } + } + } + } + } + + return 0; +} + diff --git a/lfs.h b/lfs.h index ed232f6..31d19ed 100644 --- a/lfs.h +++ b/lfs.h @@ -46,7 +46,7 @@ enum lfs_error { enum lfs_type { LFS_TYPE_REG = 0x11, LFS_TYPE_DIR = 0x22, - LFS_TYPE_SUPERBLOCK = 0xe2, + LFS_TYPE_SUPERBLOCK = 0x2e, }; // File open flags @@ -244,6 +244,7 @@ typedef struct lfs { lfs_free_t free; bool deorphaned; + bool deduplicated; } lfs_t; @@ -434,5 +435,8 @@ int lfs_traverse(lfs_t *lfs, int (*cb)(void*, lfs_block_t), void *data); // Returns a negative error code on failure. int lfs_deorphan(lfs_t *lfs); +// TODO doc +int lfs_deduplicate(lfs_t *lfs); + #endif diff --git a/tests/test_move.sh b/tests/test_move.sh new file mode 100755 index 0000000..9e5abab --- /dev/null +++ b/tests/test_move.sh @@ -0,0 +1,236 @@ +#!/bin/bash +set -eu + +echo "=== Move tests ===" +rm -rf blocks +tests/test.py << TEST + lfs_format(&lfs, &cfg) => 0; + + lfs_mount(&lfs, &cfg) => 0; + lfs_mkdir(&lfs, "a") => 0; + lfs_mkdir(&lfs, "b") => 0; + lfs_mkdir(&lfs, "c") => 0; + lfs_mkdir(&lfs, "d") => 0; + + lfs_mkdir(&lfs, "a/hi") => 0; + lfs_mkdir(&lfs, "a/hi/hola") => 0; + lfs_mkdir(&lfs, "a/hi/bonjour") => 0; + lfs_mkdir(&lfs, "a/hi/ohayo") => 0; + + lfs_file_open(&lfs, &file[0], "a/hello", LFS_O_CREAT | LFS_O_WRONLY) => 0; + lfs_file_write(&lfs, &file[0], "hola\n", 5) => 5; + lfs_file_write(&lfs, &file[0], "bonjour\n", 8) => 8; + lfs_file_write(&lfs, &file[0], "ohayo\n", 6) => 6; + lfs_file_close(&lfs, &file[0]) => 0; + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Move file ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_rename(&lfs, "a/hello", "b/hello") => 0; + lfs_unmount(&lfs) => 0; +TEST +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "a") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hi") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_dir_close(&lfs, &dir[0]) => 0; + lfs_dir_open(&lfs, &dir[0], "b") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hello") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Move file corrupt source ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_rename(&lfs, "b/hello", "c/hello") => 0; + lfs_unmount(&lfs) => 0; +TEST +rm -v blocks/7 +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "b") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_dir_close(&lfs, &dir[0]) => 0; + lfs_dir_open(&lfs, &dir[0], "c") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hello") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Move file corrupt source and dest ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_rename(&lfs, "c/hello", "d/hello") => 0; + lfs_unmount(&lfs) => 0; +TEST +rm -v blocks/8 +rm -v blocks/a +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "c") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hello") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_dir_close(&lfs, &dir[0]) => 0; + lfs_dir_open(&lfs, &dir[0], "d") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Move dir ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_rename(&lfs, "a/hi", "b/hi") => 0; + lfs_unmount(&lfs) => 0; +TEST +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "a") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_dir_close(&lfs, &dir[0]) => 0; + lfs_dir_open(&lfs, &dir[0], "b") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hi") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Move dir corrupt source ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_rename(&lfs, "b/hi", "c/hi") => 0; + lfs_unmount(&lfs) => 0; +TEST +rm -v blocks/7 +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "b") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_dir_close(&lfs, &dir[0]) => 0; + lfs_dir_open(&lfs, &dir[0], "c") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hello") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hi") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Move dir corrupt source and dest ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_rename(&lfs, "c/hi", "d/hi") => 0; + lfs_unmount(&lfs) => 0; +TEST +rm -v blocks/9 +rm -v blocks/a +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "c") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hello") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hi") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_dir_close(&lfs, &dir[0]) => 0; + lfs_dir_open(&lfs, &dir[0], "d") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + +echo "--- Move check ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + + lfs_dir_open(&lfs, &dir[0], "a/hi") => LFS_ERR_NOENT; + lfs_dir_open(&lfs, &dir[0], "b/hi") => LFS_ERR_NOENT; + lfs_dir_open(&lfs, &dir[0], "d/hi") => LFS_ERR_NOENT; + + lfs_dir_open(&lfs, &dir[0], "c/hi") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "hola") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "bonjour") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "ohayo") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_dir_close(&lfs, &dir[0]) => 0; + + lfs_dir_open(&lfs, &dir[0], "a/hello") => LFS_ERR_NOENT; + lfs_dir_open(&lfs, &dir[0], "b/hello") => LFS_ERR_NOENT; + lfs_dir_open(&lfs, &dir[0], "d/hello") => LFS_ERR_NOENT; + + lfs_file_open(&lfs, &file[0], "c/hello", LFS_O_RDONLY) => 0; + lfs_file_read(&lfs, &file[0], buffer, 5) => 5; + memcmp(buffer, "hola\n", 5) => 0; + lfs_file_read(&lfs, &file[0], buffer, 8) => 8; + memcmp(buffer, "bonjour\n", 8) => 0; + lfs_file_read(&lfs, &file[0], buffer, 6) => 6; + memcmp(buffer, "ohayo\n", 6) => 0; + lfs_file_close(&lfs, &file[0]) => 0; + + lfs_unmount(&lfs) => 0; +TEST + + +echo "--- Results ---" +tests/stats.py