Reworked Inode to have a dirty bit and subclass-implemented flush_metadata().

This way we can defer disk writes as long as we like. There's no automatic
flushing happening just yet.

Kernel/Process.cpp

@@ -1090,6 +1090,9 @@ int Process::sys$utime(const char* pathname, const Unix::utimbuf* buf)
     auto descriptor = VFS::the().open(move(path), error, 0, cwd_inode()->identifier());
     if (!descriptor)
         return error;
+    auto& inode = *descriptor->inode();
+    if (inode.fs().is_readonly())
+        return -EROFS;
     Unix::time_t atime;
     Unix::time_t mtime;
     if (buf) {
@@ -1100,7 +1103,10 @@ int Process::sys$utime(const char* pathname, const Unix::utimbuf* buf)
         mtime = now;
         atime = now;
     }
-    return descriptor->set_atime_and_mtime(atime, mtime);
+    inode.set_atime(atime);
+    inode.set_mtime(atime);
+    inode.flush_metadata();
+    return 0;
 }
 
 int Process::sys$access(const char* pathname, int mode)

VirtualFileSystem/Ext2FileSystem.cpp

@@ -304,6 +304,21 @@ void Ext2FSInode::populate_metadata() const
     }
 }
 
+void Ext2FSInode::flush_metadata()
+{
+    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);
+}
+
 RetainPtr<Inode> Ext2FS::get_inode(InodeIdentifier inode) const
 {
     ASSERT(inode.fsid() == id());
@@ -716,22 +731,6 @@ bool Ext2FS::modify_link_count(InodeIndex inode, int delta)
     return write_ext2_inode(inode, *e2inode);
 }
 
-int Ext2FS::set_atime_and_mtime(InodeIdentifier inode, dword atime, dword mtime)
-{
-    ASSERT(inode.fsid() == id());
-
-    auto e2inode = lookup_ext2_inode(inode.index());
-    if (!e2inode)
-        return -EIO;
-
-    dbgprintf("changing inode %u atime from %u to %u\n", inode.index(), e2inode->i_atime, atime);
-    dbgprintf("changing inode %u mtime from %u to %u\n", inode.index(), e2inode->i_mtime, mtime);
-    e2inode->i_mtime = mtime;
-    e2inode->i_atime = atime;
-
-    return write_ext2_inode(inode.index(), *e2inode);
-}
-
 bool Ext2FS::write_ext2_inode(unsigned inode, const ext2_inode& e2inode)
 {
     unsigned blockIndex;

VirtualFileSystem/Ext2FileSystem.h

@@ -21,12 +21,13 @@ public:
     bool is_symlink() const { return isSymbolicLink(m_raw_inode.i_mode); }
 
 private:
-    // ^CoreInode
+    // ^Inode
     virtual ssize_t read_bytes(Unix::off_t, size_t, byte* buffer, FileDescriptor*) override;
     virtual void populate_metadata() const override;
     virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) override;
     virtual InodeIdentifier lookup(const String& name) override;
     virtual String reverse_lookup(InodeIdentifier) override;
+    virtual void flush_metadata() override;
 
     void populate_lookup_cache();
 
@@ -72,7 +73,6 @@ private:
     virtual InodeIdentifier root_inode() const override;
     virtual bool write_inode(InodeIdentifier, const ByteBuffer&) override;
     virtual InodeMetadata inode_metadata(InodeIdentifier) const override;
-    virtual int set_atime_and_mtime(InodeIdentifier, dword atime, dword mtime) override;
     virtual InodeIdentifier create_inode(InodeIdentifier parentInode, const String& name, Unix::mode_t, unsigned size, int& error) override;
     virtual ssize_t read_inode_bytes(InodeIdentifier, Unix::off_t offset, size_t count, byte* buffer, FileDescriptor*) const override;
     virtual InodeIdentifier create_directory(InodeIdentifier parentInode, const String& name, Unix::mode_t, int& error) override;

VirtualFileSystem/FileDescriptor.cpp

@@ -143,7 +143,7 @@ ssize_t FileDescriptor::read(byte* buffer, size_t count)
         // FIXME: What should happen to m_currentOffset?
         return m_vnode->characterDevice()->read(buffer, count);
     }
-    ssize_t nread = m_vnode->fileSystem()->read_inode_bytes(m_vnode->inode, m_current_offset, count, buffer, this);
+    ssize_t nread = m_vnode->fs()->read_inode_bytes(m_vnode->inode, m_current_offset, count, buffer, this);
     m_current_offset += nread;
     return nread;
 }
@@ -196,7 +196,7 @@ ByteBuffer FileDescriptor::read_entire_file()
 
     if (m_vnode->core_inode())
         return m_vnode->core_inode()->read_entire(this);
-    return m_vnode->fileSystem()->read_entire_inode(m_vnode->inode, this);
+    return m_vnode->fs()->read_entire_inode(m_vnode->inode, this);
 }
 
 bool FileDescriptor::is_directory() const
@@ -288,17 +288,3 @@ FileDescriptor::FileDescriptor(FIFO& fifo, FIFO::Direction direction)
 {
     m_fifo->open(direction);
 }
-
-int FileDescriptor::set_atime_and_mtime(time_t atime, time_t mtime)
-{
-    if (!m_vnode || !m_vnode->core_inode())
-        return -EBADF;
-    return m_vnode->core_inode()->set_atime_and_mtime(atime, mtime);
-}
-
-int FileDescriptor::set_ctime(time_t ctime)
-{
-    (void) ctime;
-    // FIXME: Implement.
-    ASSERT_NOT_REACHED();
-}

VirtualFileSystem/FileDescriptor.h

@@ -50,6 +50,7 @@ public:
     InodeMetadata metadata() const { return m_vnode->metadata(); }
 
     Vnode* vnode() { return m_vnode.ptr(); }
+    Inode* inode() { return m_vnode ? m_vnode->core_inode() : nullptr; }
 
 #ifdef SERENITY
     bool is_blocking() const { return m_is_blocking; }
@@ -64,9 +65,6 @@ public:
 
     ByteBuffer& generator_cache() { return m_generator_cache; }
 
-    int set_atime_and_mtime(time_t, time_t);
-    int set_ctime(time_t);
-
 private:
     friend class VFS;
     explicit FileDescriptor(RetainPtr<Vnode>&&);

VirtualFileSystem/FileSystem.cpp

@@ -1,5 +1,6 @@
 #include <AK/Assertions.h>
 #include <AK/HashMap.h>
+#include <LibC/errno_numbers.h>
 #include "FileSystem.h"
 
 static dword s_lastFileSystemID;
@@ -60,7 +61,7 @@ ByteBuffer Inode::read_entire(FileDescriptor* descriptor)
         ASSERT(offset <= (ssize_t)initial_size); // FIXME: Support dynamically growing the buffer.
     }
     if (nread < 0) {
-        kprintf("CoreInode::read_entire: ERROR: %d\n", nread);
+        kprintf("Inode::read_entire: ERROR: %d\n", nread);
         return nullptr;
     }
 
@@ -128,7 +129,35 @@ Inode::~Inode()
 {
 }
 
-int Inode::set_atime_and_mtime(Unix::time_t atime, Unix::time_t mtime)
+int Inode::set_atime(Unix::time_t ts)
 {
-    return fs().set_atime_and_mtime(identifier(), atime, mtime);
+    if (fs().is_readonly())
+        return -EROFS;
+    if (m_metadata.atime == ts)
+        return 0;
+    m_metadata.atime = ts;
+    m_dirty = true;
+    return 0;
+}
+
+int Inode::set_ctime(Unix::time_t ts)
+{
+    if (fs().is_readonly())
+        return -EROFS;
+    if (m_metadata.ctime == ts)
+        return 0;
+    m_metadata.ctime = ts;
+    m_dirty = true;
+    return 0;
+}
+
+int Inode::set_mtime(Unix::time_t ts)
+{
+    if (fs().is_readonly())
+        return -EROFS;
+    if (m_metadata.mtime == ts)
+        return 0;
+    m_metadata.mtime = ts;
+    m_dirty = true;
+    return 0;
 }

VirtualFileSystem/FileSystem.h

@@ -35,6 +35,8 @@ public:
 
     virtual ssize_t read_inode_bytes(InodeIdentifier, Unix::off_t offset, size_t count, byte* buffer, FileDescriptor*) const = 0;
 
+    bool is_readonly() const { return m_readonly; }
+
     struct DirectoryEntry {
         DirectoryEntry(const char* name, InodeIdentifier, byte fileType);
         DirectoryEntry(const char* name, size_t name_length, InodeIdentifier, byte fileType);
@@ -44,7 +46,6 @@ public:
         byte fileType { 0 };
     };
 
-    virtual int set_atime_and_mtime(InodeIdentifier, dword atime, dword mtime) = 0;
     virtual InodeIdentifier create_inode(InodeIdentifier parentInode, const String& name, Unix::mode_t, unsigned size, int& error) = 0;
     virtual InodeIdentifier create_directory(InodeIdentifier parentInode, const String& name, Unix::mode_t, int& error) = 0;
 
@@ -59,6 +60,7 @@ protected:
 
 private:
     dword m_fsid { 0 };
+    bool m_readonly { false };
 };
 
 class Inode : public Retainable<Inode> {
@@ -85,7 +87,13 @@ public:
     virtual InodeIdentifier lookup(const String& name) = 0;
     virtual String reverse_lookup(InodeIdentifier) = 0;
 
-    int set_atime_and_mtime(Unix::time_t atime, Unix::time_t mtime);
+    bool is_dirty() const { return m_dirty; }
+
+    int set_atime(Unix::time_t);
+    int set_ctime(Unix::time_t);
+    int set_mtime(Unix::time_t);
+
+    virtual void flush_metadata() = 0;
 
 protected:
     Inode(FS& fs, unsigned index)
@@ -95,11 +103,13 @@ protected:
     }
 
     virtual void populate_metadata() const = 0;
+    void set_dirty(bool b) { m_dirty = b; }
 
     mutable InodeMetadata m_metadata;
 private:
     FS& m_fs;
     unsigned m_index { 0 };
+    bool m_dirty { false };
 };
 
 inline FS* InodeIdentifier::fs()

VirtualFileSystem/SyntheticFileSystem.cpp

@@ -146,14 +146,6 @@ InodeMetadata SynthFS::inode_metadata(InodeIdentifier inode) const
     return (*it).value->m_metadata;
 }
 
-int SynthFS::set_atime_and_mtime(InodeIdentifier, dword atime, dword mtime)
-{
-    (void) atime;
-    (void) mtime;
-    kprintf("FIXME: Implement SyntheticFileSystem::setModificationTime().\n");
-    return -ENOTIMPL;
-}
-
 InodeIdentifier SynthFS::create_inode(InodeIdentifier parentInode, const String& name, Unix::mode_t mode, unsigned size, int& error)
 {
     (void) parentInode;
@@ -318,3 +310,7 @@ String SynthFSInode::reverse_lookup(InodeIdentifier child_id)
     }
     return { };
 }
+
+void SynthFSInode::flush_metadata()
+{
+}

VirtualFileSystem/SyntheticFileSystem.h

@@ -16,7 +16,6 @@ public:
     virtual InodeIdentifier root_inode() const override;
     virtual bool write_inode(InodeIdentifier, const ByteBuffer&) override;
     virtual InodeMetadata inode_metadata(InodeIdentifier) const override;
-    virtual int set_atime_and_mtime(InodeIdentifier, dword atime, dword mtime) override;
     virtual InodeIdentifier create_inode(InodeIdentifier parentInode, const String& name, Unix::mode_t, unsigned size, int& error) override;
     virtual ssize_t read_inode_bytes(InodeIdentifier, Unix::off_t offset, size_t count, byte* buffer, FileDescriptor*) const override;
     virtual InodeIdentifier create_directory(InodeIdentifier parentInode, const String& name, Unix::mode_t, int& error) override;
@@ -49,12 +48,13 @@ public:
     virtual ~SynthFSInode() override;
 
 private:
-    // ^CoreInode
+    // ^Inode
     virtual ssize_t read_bytes(Unix::off_t, size_t, byte* buffer, FileDescriptor*) override;
     virtual void populate_metadata() const override;
     virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) override;
     virtual InodeIdentifier lookup(const String& name) override;
     virtual String reverse_lookup(InodeIdentifier) override;
+    virtual void flush_metadata() override;
 
     SynthFS& fs();
     const SynthFS& fs() const;

VirtualFileSystem/VirtualFileSystem.cpp

@@ -170,8 +170,8 @@ bool VFS::mount_root(RetainPtr<FS>&& fileSystem)
     m_root_vnode = move(node);
 
     kprintf("VFS: mounted root on %s{%p}\n",
-        m_root_vnode->fileSystem()->class_name(),
-        m_root_vnode->fileSystem());
+        m_root_vnode->fs()->class_name(),
+        m_root_vnode->fs());
 
     m_mounts.append(move(mount));
     return true;
@@ -278,7 +278,7 @@ RetainPtr<FileDescriptor> VFS::create(const String& path, InodeIdentifier base,
     // FIXME: Do the real thing, not just this fake thing!
     (void) path;
     (void) base;
-    m_root_vnode->fileSystem()->create_inode(m_root_vnode->fileSystem()->root_inode(), "empty", 0100644, 0, error);
+    m_root_vnode->fs()->create_inode(m_root_vnode->fs()->root_inode(), "empty", 0100644, 0, error);
     return nullptr;
 }
 

VirtualFileSystem/VirtualFileSystem.h

@@ -49,8 +49,8 @@ public:
     void retain();
     void release();
 
-    FS* fileSystem() { return inode.fs(); }
-    const FS* fileSystem() const { return inode.fs(); }
+    FS* fs() { return inode.fs(); }
+    const FS* fs() const { return inode.fs(); }
 
     VFS* vfs() { return m_vfs; }
     const VFS* vfs() const { return m_vfs; }