From d07b08a28783b36855705eb563c798b07ca86b7c Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 1 Jan 2019 03:16:36 +0100 Subject: [PATCH] FS: Don't default to having a full InodeMetadata in every Inode. This allows Ext2FS to keep its own ext2_inode around instead. --- VirtualFileSystem/Ext2FileSystem.cpp | 91 ++++++++++++++++------- VirtualFileSystem/Ext2FileSystem.h | 7 +- VirtualFileSystem/FileSystem.cpp | 43 ++--------- VirtualFileSystem/FileSystem.h | 15 ++-- VirtualFileSystem/SyntheticFileSystem.cpp | 4 +- VirtualFileSystem/SyntheticFileSystem.h | 3 +- VirtualFileSystem/VirtualFileSystem.cpp | 8 +- VirtualFileSystem/VirtualFileSystem.h | 2 +- 8 files changed, 93 insertions(+), 80 deletions(-) diff --git a/VirtualFileSystem/Ext2FileSystem.cpp b/VirtualFileSystem/Ext2FileSystem.cpp index 4eeb566344c..e5506d30c56 100644 --- a/VirtualFileSystem/Ext2FileSystem.cpp +++ b/VirtualFileSystem/Ext2FileSystem.cpp @@ -221,41 +221,33 @@ Ext2FSInode::~Ext2FSInode() { } -void Ext2FSInode::populate_metadata() const +InodeMetadata Ext2FSInode::metadata() const { - m_metadata.inode = identifier(); - m_metadata.size = m_raw_inode.i_size; - m_metadata.mode = m_raw_inode.i_mode; - m_metadata.uid = m_raw_inode.i_uid; - m_metadata.gid = m_raw_inode.i_gid; - m_metadata.linkCount = m_raw_inode.i_links_count; - m_metadata.atime = m_raw_inode.i_atime; - m_metadata.ctime = m_raw_inode.i_ctime; - m_metadata.mtime = m_raw_inode.i_mtime; - m_metadata.dtime = m_raw_inode.i_dtime; - m_metadata.blockSize = fs().blockSize(); - m_metadata.blockCount = m_raw_inode.i_blocks; + InodeMetadata metadata; + metadata.inode = identifier(); + metadata.size = m_raw_inode.i_size; + metadata.mode = m_raw_inode.i_mode; + metadata.uid = m_raw_inode.i_uid; + metadata.gid = m_raw_inode.i_gid; + metadata.linkCount = m_raw_inode.i_links_count; + metadata.atime = m_raw_inode.i_atime; + metadata.ctime = m_raw_inode.i_ctime; + metadata.mtime = m_raw_inode.i_mtime; + metadata.dtime = m_raw_inode.i_dtime; + metadata.blockSize = fs().blockSize(); + metadata.blockCount = m_raw_inode.i_blocks; if (isBlockDevice(m_raw_inode.i_mode) || isCharacterDevice(m_raw_inode.i_mode)) { unsigned dev = m_raw_inode.i_block[0]; - m_metadata.majorDevice = (dev & 0xfff00) >> 8; - m_metadata.minorDevice= (dev & 0xff) | ((dev >> 12) & 0xfff00); + metadata.majorDevice = (dev & 0xfff00) >> 8; + metadata.minorDevice= (dev & 0xff) | ((dev >> 12) & 0xfff00); } + return metadata; } void Ext2FSInode::flush_metadata() { dbgprintf("Ext2FSInode: flush_metadata for inode %u\n", index()); - m_raw_inode.i_size = m_metadata.size; - m_raw_inode.i_mode = m_metadata.mode; - m_raw_inode.i_uid = m_metadata.uid; - m_raw_inode.i_gid = m_metadata.gid; - m_raw_inode.i_links_count = m_metadata.linkCount; - m_raw_inode.i_atime = m_metadata.atime; - m_raw_inode.i_ctime = m_metadata.ctime; - m_raw_inode.i_mtime = m_metadata.mtime; - m_raw_inode.i_dtime = m_metadata.dtime; - m_raw_inode.i_blocks = m_metadata.blockCount; fs().write_ext2_inode(index(), m_raw_inode); set_metadata_dirty(false); } @@ -358,7 +350,7 @@ ssize_t Ext2FSInode::read_bytes(Unix::off_t offset, size_t count, byte* buffer, bool Ext2FSInode::write(const ByteBuffer& data) { // FIXME: Support writing to symlink inodes. - ASSERT(!m_metadata.isSymbolicLink()); + ASSERT(!is_symlink()); unsigned blocksNeededBefore = ceilDiv(size(), fs().blockSize()); unsigned blocksNeededAfter = ceilDiv((unsigned)data.size(), fs().blockSize()); @@ -595,7 +587,6 @@ bool Ext2FS::write_ext2_inode(unsigned inode, const ext2_inode& e2inode) auto& cached_inode = *(*it).value; LOCKER(cached_inode.m_lock); cached_inode.m_raw_inode = e2inode; - cached_inode.populate_metadata(); if (cached_inode.is_directory()) cached_inode.m_lookup_cache.clear(); } @@ -924,6 +915,7 @@ InodeIdentifier Ext2FS::find_parent_of_inode(InodeIdentifier inode_id) const Vector> directories_in_group; for (unsigned i = 0; i < inodes_per_group(); ++i) { + // FIXME: Consult the inode bitmap to see which inodes to look into. auto group_member = get_inode({ id(), firstInodeInGroup + i }); if (!group_member) continue; @@ -990,3 +982,48 @@ void Ext2FSInode::one_retain_left() { // FIXME: I would like to not live forever, but uncached Ext2FS is fucking painful right now. } + +int Ext2FSInode::set_atime(Unix::time_t t) +{ + if (fs().is_readonly()) + return -EROFS; + m_raw_inode.i_atime = t; + set_metadata_dirty(true); + return 0; +} + +int Ext2FSInode::set_ctime(Unix::time_t t) +{ + if (fs().is_readonly()) + return -EROFS; + m_raw_inode.i_ctime = t; + set_metadata_dirty(true); + return 0; +} + +int Ext2FSInode::set_mtime(Unix::time_t t) +{ + if (fs().is_readonly()) + return -EROFS; + m_raw_inode.i_mtime = t; + set_metadata_dirty(true); + return 0; +} + +int Ext2FSInode::increment_link_count() +{ + if (fs().is_readonly()) + return -EROFS; + ++m_raw_inode.i_links_count; + set_metadata_dirty(true); + return 0; +} + +int Ext2FSInode::decrement_link_count() +{ + if (fs().is_readonly()) + return -EROFS; + --m_raw_inode.i_links_count; + set_metadata_dirty(true); + return 0; +} diff --git a/VirtualFileSystem/Ext2FileSystem.h b/VirtualFileSystem/Ext2FileSystem.h index 9ab407267b6..ecdca6486be 100644 --- a/VirtualFileSystem/Ext2FileSystem.h +++ b/VirtualFileSystem/Ext2FileSystem.h @@ -27,13 +27,18 @@ public: private: // ^Inode virtual ssize_t read_bytes(Unix::off_t, size_t, byte* buffer, FileDescriptor*) override; - virtual void populate_metadata() const override; + virtual InodeMetadata metadata() const override; virtual bool traverse_as_directory(Function) override; virtual InodeIdentifier lookup(const String& name) override; virtual String reverse_lookup(InodeIdentifier) override; virtual void flush_metadata() override; virtual bool write(const ByteBuffer&) override; virtual bool add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) override; + virtual int set_atime(Unix::time_t) override; + virtual int set_ctime(Unix::time_t) override; + virtual int set_mtime(Unix::time_t) override; + virtual int increment_link_count() override; + virtual int decrement_link_count() override; void populate_lookup_cache(); diff --git a/VirtualFileSystem/FileSystem.cpp b/VirtualFileSystem/FileSystem.cpp index b5429d09cd7..59633ab2297 100644 --- a/VirtualFileSystem/FileSystem.cpp +++ b/VirtualFileSystem/FileSystem.cpp @@ -111,56 +111,29 @@ void Inode::will_be_destroyed() flush_metadata(); } -int Inode::set_atime(Unix::time_t ts) +int Inode::set_atime(Unix::time_t) { - if (fs().is_readonly()) - return -EROFS; - if (m_metadata.atime == ts) - return 0; - m_metadata.atime = ts; - m_metadata_dirty = true; - return 0; + return -ENOTIMPL; } -int Inode::set_ctime(Unix::time_t ts) +int Inode::set_ctime(Unix::time_t) { - if (fs().is_readonly()) - return -EROFS; - if (m_metadata.ctime == ts) - return 0; - m_metadata.ctime = ts; - m_metadata_dirty = true; - return 0; + return -ENOTIMPL; } -int Inode::set_mtime(Unix::time_t ts) +int Inode::set_mtime(Unix::time_t) { - if (fs().is_readonly()) - return -EROFS; - if (m_metadata.mtime == ts) - return 0; - m_metadata.mtime = ts; - m_metadata_dirty = true; - return 0; + return -ENOTIMPL; } int Inode::increment_link_count() { - if (fs().is_readonly()) - return -EROFS; - ++m_metadata.linkCount; - m_metadata_dirty = true; - return 0; + return -ENOTIMPL; } int Inode::decrement_link_count() { - if (fs().is_readonly()) - return -EROFS; - ASSERT(m_metadata.linkCount); - --m_metadata.linkCount; - m_metadata_dirty = true; - return 0; + return -ENOTIMPL; } void FS::sync() diff --git a/VirtualFileSystem/FileSystem.h b/VirtualFileSystem/FileSystem.h index 551c7b4b133..9a5a81e49d5 100644 --- a/VirtualFileSystem/FileSystem.h +++ b/VirtualFileSystem/FileSystem.h @@ -75,7 +75,7 @@ public: bool is_directory() const { return metadata().isDirectory(); } InodeIdentifier identifier() const { return { fsid(), index() }; } - const InodeMetadata& metadata() const { if (!m_metadata.isValid()) { populate_metadata(); } return m_metadata; } + virtual InodeMetadata metadata() const = 0; ByteBuffer read_entire(FileDescriptor* = nullptr); @@ -88,11 +88,11 @@ public: bool is_metadata_dirty() const { return m_metadata_dirty; } - int set_atime(Unix::time_t); - int set_ctime(Unix::time_t); - int set_mtime(Unix::time_t); - int increment_link_count(); - int decrement_link_count(); + virtual int set_atime(Unix::time_t); + virtual int set_ctime(Unix::time_t); + virtual int set_mtime(Unix::time_t); + virtual int increment_link_count(); + virtual int decrement_link_count(); virtual void flush_metadata() = 0; @@ -101,10 +101,7 @@ public: protected: Inode(FS& fs, unsigned index); - virtual void populate_metadata() const = 0; void set_metadata_dirty(bool b) { m_metadata_dirty = b; } - - mutable InodeMetadata m_metadata; private: FS& m_fs; unsigned m_index { 0 }; diff --git a/VirtualFileSystem/SyntheticFileSystem.cpp b/VirtualFileSystem/SyntheticFileSystem.cpp index 3a353eeb586..aaa1abf16c8 100644 --- a/VirtualFileSystem/SyntheticFileSystem.cpp +++ b/VirtualFileSystem/SyntheticFileSystem.cpp @@ -184,9 +184,9 @@ SynthFSInode::~SynthFSInode() { } -void SynthFSInode::populate_metadata() const +InodeMetadata SynthFSInode::metadata() const { - // Already done when SynthFS created the file. + return m_metadata; } ssize_t SynthFSInode::read_bytes(Unix::off_t offset, size_t count, byte* buffer, FileDescriptor* descriptor) diff --git a/VirtualFileSystem/SyntheticFileSystem.h b/VirtualFileSystem/SyntheticFileSystem.h index 27062b31cac..fa6bf9eb7a7 100644 --- a/VirtualFileSystem/SyntheticFileSystem.h +++ b/VirtualFileSystem/SyntheticFileSystem.h @@ -47,7 +47,7 @@ public: private: // ^Inode virtual ssize_t read_bytes(Unix::off_t, size_t, byte* buffer, FileDescriptor*) override; - virtual void populate_metadata() const override; + virtual InodeMetadata metadata() const override; virtual bool traverse_as_directory(Function) override; virtual InodeIdentifier lookup(const String& name) override; virtual String reverse_lookup(InodeIdentifier) override; @@ -64,4 +64,5 @@ private: ByteBuffer m_data; Function m_generator; Vector m_children; + InodeMetadata m_metadata; }; diff --git a/VirtualFileSystem/VirtualFileSystem.cpp b/VirtualFileSystem/VirtualFileSystem.cpp index a96ac920dcf..dea9972dc1b 100644 --- a/VirtualFileSystem/VirtualFileSystem.cpp +++ b/VirtualFileSystem/VirtualFileSystem.cpp @@ -51,7 +51,7 @@ auto VFS::makeNode(InodeIdentifier inode) -> RetainPtr if (!core_inode) return nullptr; - auto& metadata = core_inode->metadata(); + auto metadata = core_inode->metadata(); InterruptDisabler disabler; @@ -74,7 +74,6 @@ auto VFS::makeNode(InodeIdentifier inode) -> RetainPtr vnode->inode = inode; vnode->m_core_inode = move(core_inode); - vnode->m_cachedMetadata = metadata; #ifdef VFS_DEBUG kprintf("makeNode: inode=%u, size=%u, mode=%o, uid=%u, gid=%u\n", inode.index(), metadata.size, metadata.mode, metadata.uid, metadata.gid); @@ -474,11 +473,12 @@ void Vnode::release() } } -const InodeMetadata& Vnode::metadata() const +InodeMetadata Vnode::metadata() const { if (m_core_inode) return m_core_inode->metadata(); - return m_cachedMetadata; + ASSERT_NOT_REACHED(); + return { }; } VFS::Mount::Mount(InodeIdentifier host, RetainPtr&& guest_fs) diff --git a/VirtualFileSystem/VirtualFileSystem.h b/VirtualFileSystem/VirtualFileSystem.h index 8ed1c66f612..e53c1712ae9 100644 --- a/VirtualFileSystem/VirtualFileSystem.h +++ b/VirtualFileSystem/VirtualFileSystem.h @@ -38,7 +38,7 @@ class VFS; class Vnode { public: InodeIdentifier inode; - const InodeMetadata& metadata() const; + InodeMetadata metadata() const; bool inUse() const { return inode.is_valid() || m_characterDevice; }