From 9be954bc8780ffec5f9d3eeea7ada623ca6447f6 Mon Sep 17 00:00:00 2001 From: Theodore Dubois Date: Sat, 30 Mar 2019 20:07:53 -0700 Subject: [PATCH] Unbreak fstat on deleted files --- fs/fake-migrate.c | 8 ++++++-- fs/fake.c | 28 +++++++++++++++++++++++++++- fs/inode.c | 44 +++++++++++++++++++++++++++++--------------- fs/inode.h | 3 +++ kernel/fs.h | 5 +++++ 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/fs/fake-migrate.c b/fs/fake-migrate.c index 27bdce85..3a387b43 100644 --- a/fs/fake-migrate.c +++ b/fs/fake-migrate.c @@ -9,11 +9,11 @@ static struct migration { const char *sql; void (*migrate)(struct mount *mount); } migrations[] = { - // version 0 to version 1 + // version 1: add another index { "create index inode_to_path on paths (inode, path);" }, - // version 1 to version 2 + // version 2: add foreign key constraint on paths, create trigger to automatically cleanup stats { "create table paths_new (path blob primary key, inode integer references stats(inode));" "insert into paths_new select * from paths where exists (select 1 from stats where inode = paths.inode);" @@ -26,6 +26,10 @@ static struct migration { "delete from stats where not exists (select 1 from paths where inode = old.inode) and inode = old.inode; " "end;" }, + // version 3: the trigger was a mistake + { + "drop trigger delete_path" + }, }; int fakefs_migrate(struct mount *mount) { diff --git a/fs/fake.c b/fs/fake.c index cbbeda3f..3c768f96 100644 --- a/fs/fake.c +++ b/fs/fake.c @@ -11,6 +11,7 @@ #include "kernel/task.h" #include "fs/fd.h" #include "fs/dev.h" +#include "fs/inode.h" #define ISH_INTERNAL #include "fs/fake.h" @@ -67,6 +68,11 @@ static void bind_path(sqlite3_stmt *stmt, int i, const char *path) { sqlite3_bind_blob(stmt, i, path, strlen(path), SQLITE_TRANSIENT); } +static void try_cleanup_inode(struct mount *mount, ino_t inode) { + sqlite3_bind_int64(mount->stmt.try_cleanup_inode, 1, inode); + db_exec_reset(mount, mount->stmt.try_cleanup_inode); +} + ino_t path_get_inode(struct mount *mount, const char *path) { // select inode from paths where path = ? bind_path(mount->stmt.path_get_inode, 1, path); @@ -123,9 +129,14 @@ static void path_link(struct mount *mount, const char *src, const char *dst) { db_exec_reset(mount, mount->stmt.path_link); } static void path_unlink(struct mount *mount, const char *path) { + ino_t inode = path_get_inode(mount, path); + if (inode == 0) + die("path_unlink(%s): nonexistent path", path); // delete from paths where path = ? bind_path(mount->stmt.path_unlink, 1, path); db_exec_reset(mount, mount->stmt.path_unlink); + if (inode_is_orphaned(mount, inode)) + try_cleanup_inode(mount, inode); } static void path_rename(struct mount *mount, const char *src, const char *dst) { // update or replace paths set path = change_prefix(path, ? [len(src)], ? [dst]) @@ -576,6 +587,13 @@ static int fakefs_mount(struct mount *mount) { db_check_error(mount); sqlite3_finalize(statement); + // delete orphaned stats + statement = db_prepare(mount, "delete from stats where not exists (select 1 from paths where inode = stats.inode)"); + db_check_error(mount); + sqlite3_step(statement); + db_check_error(mount); + sqlite3_finalize(statement); + lock_init(&mount->lock); mount->stmt.begin = db_prepare(mount, "begin"); mount->stmt.commit = db_prepare(mount, "commit"); @@ -591,6 +609,7 @@ static int fakefs_mount(struct mount *mount) { mount->stmt.path_rename = db_prepare(mount, "update or replace paths set path = change_prefix(path, ?, ?) " "where (path >= ? and path < ?) or path = ?"); mount->stmt.path_from_inode = db_prepare(mount, "select path from paths where inode = ?"); + mount->stmt.try_cleanup_inode = db_prepare(mount, "delete from stats where inode = ? and not exists (select 1 from paths where inode = stats.inode)"); return 0; } @@ -602,6 +621,12 @@ static int fakefs_umount(struct mount *mount) { return 0; } +static void fakefs_inode_orphaned(struct mount *mount, struct inode_data *inode) { + db_begin(mount); + try_cleanup_inode(mount, inode->number); + db_commit(mount); +} + const struct fs_ops fakefs = { .magic = 0x66616b65, .mount = fakefs_mount, @@ -626,5 +651,6 @@ const struct fs_ops fakefs = { .mkdir = fakefs_mkdir, .rmdir = fakefs_rmdir, -}; + .inode_orphaned = fakefs_inode_orphaned, +}; diff --git a/fs/inode.c b/fs/inode.c index 4634a54b..3234b674 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -7,29 +7,34 @@ static lock_t inodes_lock = LOCK_INITIALIZER; #define INODES_HASH_SIZE (1 << 10) static struct list inodes_hash[INODES_HASH_SIZE]; -struct inode_data *inode_get(struct mount *mount, ino_t ino) { - lock(&inodes_lock); +static struct inode_data *inode_get_data(struct mount *mount, ino_t ino) { int index = ino % INODES_HASH_SIZE; if (list_null(&inodes_hash[index])) list_init(&inodes_hash[index]); struct inode_data *inode; list_for_each_entry(&inodes_hash[index], inode, chain) { if (inode->mount == mount && inode->number == ino) - goto out; + return inode; + } + return NULL; +} + +struct inode_data *inode_get(struct mount *mount, ino_t ino) { + lock(&inodes_lock); + struct inode_data *inode = inode_get_data(mount, ino); + if (inode == NULL) { + inode = malloc(sizeof(struct inode_data)); + inode->refcount = 0; + inode->number = ino; + mount_retain(mount); + inode->mount = mount; + cond_init(&inode->posix_unlock); + list_init(&inode->posix_locks); + list_init(&inode->chain); + lock_init(&inode->lock); + list_add(&inodes_hash[ino % INODES_HASH_SIZE], &inode->chain); } - inode = malloc(sizeof(struct inode_data)); - inode->refcount = 0; - inode->number = ino; - mount_retain(mount); - inode->mount = mount; - cond_init(&inode->posix_unlock); - list_init(&inode->posix_locks); - list_init(&inode->chain); - lock_init(&inode->lock); - list_add(&inodes_hash[index], &inode->chain); - -out: lock(&inode->lock); inode->refcount++; unlock(&inode->lock); @@ -37,6 +42,13 @@ out: return inode; } +bool inode_is_orphaned(struct mount *mount, ino_t ino) { + lock(&inodes_lock); + struct inode_data *inode = inode_get_data(mount, ino); + unlock(&inodes_lock); + return inode == NULL; +} + void inode_retain(struct inode_data *inode) { lock(&inode->lock); inode->refcount++; @@ -47,6 +59,8 @@ void inode_release(struct inode_data *inode) { lock(&inodes_lock); lock(&inode->lock); if (--inode->refcount == 0) { + if (inode->mount->fs->inode_orphaned) + inode->mount->fs->inode_orphaned(inode->mount, inode); unlock(&inode->lock); list_remove(&inode->chain); mount_release(inode->mount); diff --git a/fs/inode.h b/fs/inode.h index 9730c7e9..c4ba0cb7 100644 --- a/fs/inode.h +++ b/fs/inode.h @@ -22,6 +22,9 @@ struct inode_data *inode_get(struct mount *mount, ino_t inode); void inode_retain(struct inode_data *inode); void inode_release(struct inode_data *inode); +// does this inode have any references? specifically to tell fakefs if it's safe to try and delete its inode data +bool inode_is_orphaned(struct mount *mount, ino_t inode); + // file locking stuff (maybe should go in kernel/calls.h?) #define F_RDLCK_ 0 diff --git a/kernel/fs.h b/kernel/fs.h index 7f7e1f30..6a7fd284 100644 --- a/kernel/fs.h +++ b/kernel/fs.h @@ -90,6 +90,7 @@ struct mount { sqlite3_stmt *path_unlink; sqlite3_stmt *path_rename; sqlite3_stmt *path_from_inode; + sqlite3_stmt *try_cleanup_inode; } stmt; lock_t lock; }; @@ -158,6 +159,10 @@ struct fs_ops { int (*getpath)(struct fd *fd, char *buf); int (*flock)(struct fd *fd, int operation); + + // If present, called when all references to an inode_data for this + // filesystem go away. + void (*inode_orphaned)(struct mount *mount, struct inode_data *inode); }; struct mount *find_mount_and_trim_path(char *path);