Kernel: Implement multi-watch InodeWatcher :^)

This patch modifies InodeWatcher to switch to a one watcher, multiple
watches architecture.  The following changes have been made:

- The watch_file syscall is removed, and in its place the
  create_iwatcher, iwatcher_add_watch and iwatcher_remove_watch calls
  have been added.
- InodeWatcher now holds multiple WatchDescriptions for each file that
  is being watched.
- The InodeWatcher file descriptor can be read from to receive events on
  all watched files.

Co-authored-by: Gunnar Beutner <gunnar@beutner.name>

Kernel/FileSystem/Ext2FileSystem.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -1033,6 +1034,8 @@ KResultOr<ssize_t> Ext2FSInode::write_bytes(off_t offset, ssize_t count, const U
         nwritten += num_bytes_to_copy;
     }
 
+    did_modify_contents();
+
     dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes(): After write, i_size={}, i_blocks={} ({} blocks in list)", identifier(), size(), m_raw_inode.i_blocks, m_block_list.size());
     return nwritten;
 }
@@ -1208,7 +1211,7 @@ KResult Ext2FSInode::add_child(Inode& child, const StringView& name, mode_t mode
         return result;
 
     m_lookup_cache.set(name, child.index());
-    did_add_child(child.identifier());
+    did_add_child(child.identifier(), name);
     return KSuccess;
 }
 
@@ -1245,7 +1248,7 @@ KResult Ext2FSInode::remove_child(const StringView& name)
     if (result.is_error())
         return result;
 
-    did_remove_child(child_id);
+    did_remove_child(child_id, name);
     return KSuccess;
 }
 
@@ -1674,10 +1677,15 @@ KResult Ext2FSInode::decrement_link_count()
     if (fs().is_readonly())
         return EROFS;
     VERIFY(m_raw_inode.i_links_count);
+
     --m_raw_inode.i_links_count;
+    set_metadata_dirty(true);
+    if (m_raw_inode.i_links_count == 0)
+        did_delete_self();
+
     if (ref_count() == 1 && m_raw_inode.i_links_count == 0)
         fs().uncache_inode(index());
-    set_metadata_dirty(true);
+
     return KSuccess;
 }
 

Kernel/FileSystem/File.h

@@ -109,6 +109,7 @@ public:
     virtual bool is_block_device() const { return false; }
     virtual bool is_character_device() const { return false; }
     virtual bool is_socket() const { return false; }
+    virtual bool is_inode_watcher() const { return false; }
 
     virtual FileBlockCondition& block_condition() { return m_block_condition; }
 

Kernel/FileSystem/FileDescription.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -13,6 +14,7 @@
 #include <Kernel/FileSystem/FileDescription.h>
 #include <Kernel/FileSystem/FileSystem.h>
 #include <Kernel/FileSystem/InodeFile.h>
+#include <Kernel/FileSystem/InodeWatcher.h>
 #include <Kernel/Net/Socket.h>
 #include <Kernel/Process.h>
 #include <Kernel/TTY/MasterPTY.h>
@@ -295,6 +297,25 @@ TTY* FileDescription::tty()
     return static_cast<TTY*>(m_file.ptr());
 }
 
+bool FileDescription::is_inode_watcher() const
+{
+    return m_file->is_inode_watcher();
+}
+
+const InodeWatcher* FileDescription::inode_watcher() const
+{
+    if (!is_inode_watcher())
+        return nullptr;
+    return static_cast<const InodeWatcher*>(m_file.ptr());
+}
+
+InodeWatcher* FileDescription::inode_watcher()
+{
+    if (!is_inode_watcher())
+        return nullptr;
+    return static_cast<InodeWatcher*>(m_file.ptr());
+}
+
 bool FileDescription::is_master_pty() const
 {
     return m_file->is_master_pty();

Kernel/FileSystem/FileDescription.h

@@ -77,6 +77,10 @@ public:
     const TTY* tty() const;
     TTY* tty();
 
+    bool is_inode_watcher() const;
+    const InodeWatcher* inode_watcher() const;
+    InodeWatcher* inode_watcher();
+
     bool is_master_pty() const;
     const MasterPTY* master_pty() const;
     MasterPTY* master_pty();

Kernel/FileSystem/Inode.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -109,6 +110,10 @@ Inode::~Inode()
 {
     ScopedSpinLock all_inodes_lock(s_all_inodes_lock);
     all_with_lock().remove(this);
+
+    for (auto& watcher : m_watchers) {
+        watcher->unregister_by_inode({}, identifier());
+    }
 }
 
 void Inode::will_be_destroyed()
@@ -211,24 +216,47 @@ void Inode::set_metadata_dirty(bool metadata_dirty)
         // FIXME: Maybe we should hook into modification events somewhere else, I'm not sure where.
         //        We don't always end up on this particular code path, for instance when writing to an ext2fs file.
         for (auto& watcher : m_watchers) {
-            watcher->notify_inode_event({}, InodeWatcherEvent::Type::Modified);
+            watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::MetadataModified);
         }
     }
 }
 
-void Inode::did_add_child(const InodeIdentifier& child_id)
+void Inode::did_add_child(InodeIdentifier const&, String const& name)
 {
     Locker locker(m_lock);
+
     for (auto& watcher : m_watchers) {
-        watcher->notify_child_added({}, child_id);
+        watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::ChildCreated, name);
     }
 }
 
-void Inode::did_remove_child(const InodeIdentifier& child_id)
+void Inode::did_remove_child(InodeIdentifier const&, String const& name)
+{
+    Locker locker(m_lock);
+
+    if (name == "." || name == "..") {
+        // These are just aliases and are not interesting to userspace.
+        return;
+    }
+
+    for (auto& watcher : m_watchers) {
+        watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::ChildDeleted, name);
+    }
+}
+
+void Inode::did_modify_contents()
 {
     Locker locker(m_lock);
     for (auto& watcher : m_watchers) {
-        watcher->notify_child_removed({}, child_id);
+        watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::ContentModified);
+    }
+}
+
+void Inode::did_delete_self()
+{
+    Locker locker(m_lock);
+    for (auto& watcher : m_watchers) {
+        watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::Deleted);
     }
 }
 

Kernel/FileSystem/Inode.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -111,8 +112,10 @@ protected:
     void set_metadata_dirty(bool);
     KResult prepare_to_write_data();
 
-    void did_add_child(const InodeIdentifier&);
-    void did_remove_child(const InodeIdentifier&);
+    void did_add_child(InodeIdentifier const& child_id, String const& name);
+    void did_remove_child(InodeIdentifier const& child_id, String const& name);
+    void did_modify_contents();
+    void did_delete_self();
 
     mutable Lock m_lock { "Inode" };
 

Kernel/FileSystem/InodeWatcher.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -7,53 +8,59 @@
 #include <AK/Memory.h>
 #include <Kernel/FileSystem/Inode.h>
 #include <Kernel/FileSystem/InodeWatcher.h>
+#include <Kernel/Process.h>
 
 namespace Kernel {
 
-NonnullRefPtr<InodeWatcher> InodeWatcher::create(Inode& inode)
+NonnullRefPtr<InodeWatcher> InodeWatcher::create()
 {
-    return adopt_ref(*new InodeWatcher(inode));
-}
-
-InodeWatcher::InodeWatcher(Inode& inode)
-    : m_inode(inode)
-{
-    inode.register_watcher({}, *this);
+    return adopt_ref(*new InodeWatcher);
 }
 
 InodeWatcher::~InodeWatcher()
 {
-    if (auto inode = m_inode.strong_ref())
-        inode->unregister_watcher({}, *this);
+    (void)close();
 }
 
 bool InodeWatcher::can_read(const FileDescription&, size_t) const
 {
-    return !m_queue.is_empty() || !m_inode;
-}
-
-bool InodeWatcher::can_write(const FileDescription&, size_t) const
-{
-    return true;
+    Locker locker(m_lock);
+    return !m_queue.is_empty();
 }
 
 KResultOr<size_t> InodeWatcher::read(FileDescription&, u64, UserOrKernelBuffer& buffer, size_t buffer_size)
 {
     Locker locker(m_lock);
-    VERIFY(!m_queue.is_empty() || !m_inode);
-
-    if (!m_inode)
-        return 0;
+    if (m_queue.is_empty())
+        // can_read will catch the blocking case.
+        return EAGAIN;
 
     auto event = m_queue.dequeue();
 
-    if (buffer_size < sizeof(InodeWatcherEvent))
-        return buffer_size;
+    size_t name_length = event.path.length() + 1;
+    size_t bytes_to_write = sizeof(InodeWatcherEvent);
+    if (!event.path.is_null())
+        bytes_to_write += name_length;
 
-    size_t bytes_to_write = min(buffer_size, sizeof(event));
+    if (buffer_size < bytes_to_write)
+        return EINVAL;
+
+    ssize_t nwritten = buffer.write_buffered<MAXIMUM_EVENT_SIZE>(bytes_to_write, [&](u8* data, size_t data_bytes) {
+        size_t offset = 0;
+
+        memcpy(data + offset, &event.wd, sizeof(InodeWatcherEvent::watch_descriptor));
+        offset += sizeof(InodeWatcherEvent::watch_descriptor);
+        memcpy(data + offset, &event.type, sizeof(InodeWatcherEvent::type));
+        offset += sizeof(InodeWatcherEvent::type);
+
+        if (!event.path.is_null()) {
+            memcpy(data + offset, &name_length, sizeof(InodeWatcherEvent::name_length));
+            offset += sizeof(InodeWatcherEvent::name_length);
+            memcpy(data + offset, event.path.characters(), name_length);
+        } else {
+            memset(data + offset, 0, sizeof(InodeWatcherEvent::name_length));
+        }
 
-    ssize_t nwritten = buffer.write_buffered<sizeof(event)>(bytes_to_write, [&](u8* data, size_t data_bytes) {
-        memcpy(data, &event, bytes_to_write);
         return (ssize_t)data_bytes;
     });
     if (nwritten < 0)
@@ -62,37 +69,94 @@ KResultOr<size_t> InodeWatcher::read(FileDescription&, u64, UserOrKernelBuffer&
     return bytes_to_write;
 }
 
-KResultOr<size_t> InodeWatcher::write(FileDescription&, u64, const UserOrKernelBuffer&, size_t)
+KResult InodeWatcher::close()
 {
-    return EIO;
+    Locker locker(m_lock);
+
+    for (auto& entry : m_wd_to_watches) {
+        auto& inode = const_cast<Inode&>(entry.value->inode);
+        inode.unregister_watcher({}, *this);
+    }
+
+    m_wd_to_watches.clear();
+    m_inode_to_watches.clear();
+    return KSuccess;
 }
 
 String InodeWatcher::absolute_path(const FileDescription&) const
 {
-    if (auto inode = m_inode.strong_ref())
-        return String::formatted("InodeWatcher:{}", inode->identifier().to_string());
-    return "InodeWatcher:(gone)";
+    return String::formatted("InodeWatcher:({})", m_wd_to_watches.size());
 }
 
-void InodeWatcher::notify_inode_event(Badge<Inode>, InodeWatcherEvent::Type event_type)
+void InodeWatcher::notify_inode_event(Badge<Inode>, InodeIdentifier inode_id, InodeWatcherEvent::Type event_type, String const& name)
 {
     Locker locker(m_lock);
-    m_queue.enqueue({ event_type });
+
+    auto it = m_inode_to_watches.find(inode_id);
+    if (it == m_inode_to_watches.end())
+        return;
+
+    auto& watcher = *it->value;
+    if (!(watcher.event_mask & static_cast<unsigned>(event_type)))
+        return;
+
+    m_queue.enqueue({ watcher.wd, event_type, name });
     evaluate_block_conditions();
 }
 
-void InodeWatcher::notify_child_added(Badge<Inode>, const InodeIdentifier& child_id)
+KResultOr<int> InodeWatcher::register_inode(Inode& inode, unsigned event_mask)
 {
     Locker locker(m_lock);
-    m_queue.enqueue({ InodeWatcherEvent::Type::ChildAdded, child_id.index().value() });
-    evaluate_block_conditions();
+
+    if (m_inode_to_watches.find(inode.identifier()) != m_inode_to_watches.end())
+        return EEXIST;
+
+    int wd;
+    do {
+        wd = m_wd_counter.value();
+
+        m_wd_counter++;
+        if (m_wd_counter.has_overflow())
+            m_wd_counter = 1;
+    } while (m_wd_to_watches.find(wd) != m_wd_to_watches.end());
+
+    auto description = WatchDescription::create(wd, inode, event_mask);
+    m_inode_to_watches.set(inode.identifier(), description.ptr());
+    m_wd_to_watches.set(wd, move(description));
+
+    inode.register_watcher({}, *this);
+    return wd;
 }
 
-void InodeWatcher::notify_child_removed(Badge<Inode>, const InodeIdentifier& child_id)
+KResult InodeWatcher::unregister_by_wd(int wd)
 {
     Locker locker(m_lock);
-    m_queue.enqueue({ InodeWatcherEvent::Type::ChildRemoved, child_id.index().value() });
-    evaluate_block_conditions();
+
+    auto it = m_wd_to_watches.find(wd);
+    if (it == m_wd_to_watches.end())
+        return ENOENT;
+
+    auto& inode = it->value->inode;
+    inode.unregister_watcher({}, *this);
+
+    m_inode_to_watches.remove(inode.identifier());
+    m_wd_to_watches.remove(it);
+
+    return KSuccess;
+}
+
+void InodeWatcher::unregister_by_inode(Badge<Inode>, InodeIdentifier identifier)
+{
+    Locker locker(m_lock);
+
+    auto it = m_inode_to_watches.find(identifier);
+    if (it == m_inode_to_watches.end())
+        return;
+
+    // NOTE: no need to call unregister_watcher here, the Inode calls us.
+
+    m_inode_to_watches.remove(identifier);
+    m_wd_to_watches.remove(it->value->wd);
 }
 
 }

Kernel/FileSystem/InodeWatcher.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, sin-ack <sin-ack@protonmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -7,38 +8,76 @@
 #pragma once
 
 #include <AK/Badge.h>
+#include <AK/Checked.h>
 #include <AK/CircularQueue.h>
-#include <AK/WeakPtr.h>
+#include <AK/HashMap.h>
+#include <AK/NonnullOwnPtr.h>
 #include <Kernel/API/InodeWatcherEvent.h>
 #include <Kernel/FileSystem/File.h>
-#include <Kernel/Lock.h>
 
 namespace Kernel {
 
 class Inode;
 
+// A specific description of a watch.
+struct WatchDescription {
+    int wd;
+    Inode& inode;
+    unsigned event_mask;
+
+    static NonnullOwnPtr<WatchDescription> create(int wd, Inode& inode, unsigned event_mask)
+    {
+        return adopt_own(*new WatchDescription(wd, inode, event_mask));
+    }
+
+private:
+    WatchDescription(int wd, Inode& inode, unsigned event_mask)
+        : wd(wd)
+        , inode(inode)
+        , event_mask(event_mask)
+    {
+    }
+};
+
 class InodeWatcher final : public File {
 public:
-    static NonnullRefPtr<InodeWatcher> create(Inode&);
+    static NonnullRefPtr<InodeWatcher> create();
     virtual ~InodeWatcher() override;
 
     virtual bool can_read(const FileDescription&, size_t) const override;
-    virtual bool can_write(const FileDescription&, size_t) const override;
     virtual KResultOr<size_t> read(FileDescription&, u64, UserOrKernelBuffer&, size_t) override;
-    virtual KResultOr<size_t> write(FileDescription&, u64, const UserOrKernelBuffer&, size_t) override;
+    // Can't write to an inode watcher.
+    virtual bool can_write(const FileDescription&, size_t) const override { return true; }
+    virtual KResultOr<size_t> write(FileDescription&, u64, const UserOrKernelBuffer&, size_t) override { return EIO; }
+    virtual KResult close() override;
+
     virtual String absolute_path(const FileDescription&) const override;
     virtual const char* class_name() const override { return "InodeWatcher"; };
+    virtual bool is_inode_watcher() const override { return true; }
 
-    void notify_inode_event(Badge<Inode>, InodeWatcherEvent::Type);
-    void notify_child_added(Badge<Inode>, const InodeIdentifier& child_id);
-    void notify_child_removed(Badge<Inode>, const InodeIdentifier& child_id);
+    void notify_inode_event(Badge<Inode>, InodeIdentifier, InodeWatcherEvent::Type, String const& name = {});
+
+    KResultOr<int> register_inode(Inode&, unsigned event_mask);
+    KResult unregister_by_wd(int);
+    void unregister_by_inode(Badge<Inode>, InodeIdentifier);
 
 private:
-    explicit InodeWatcher(Inode&);
+    explicit InodeWatcher() { }
 
-    Lock m_lock;
-    WeakPtr<Inode> m_inode;
-    CircularQueue<InodeWatcherEvent, 32> m_queue;
+    mutable Lock m_lock;
+
+    struct Event {
+        int wd { 0 };
+        InodeWatcherEvent::Type type { InodeWatcherEvent::Type::Invalid };
+        String path;
+    };
+    CircularQueue<Event, 32> m_queue;
+    Checked<int> m_wd_counter { 1 };
+
+    // NOTE: These two hashmaps provide two different ways of reaching the same
+    // watch description, so they will overlap.
+    HashMap<int, NonnullOwnPtr<WatchDescription>> m_wd_to_watches;
+    HashMap<InodeIdentifier, WatchDescription*> m_inode_to_watches;
 };
 
 }

Kernel/FileSystem/TmpFS.cpp

@@ -188,6 +188,8 @@ KResultOr<ssize_t> TmpFSInode::write_bytes(off_t offset, ssize_t size, const Use
 
     if (!buffer.read(m_content->data() + offset, size)) // TODO: partial reads?
         return EFAULT;
+
+    did_modify_contents();
     return size;
 }
 
@@ -284,7 +286,7 @@ KResult TmpFSInode::add_child(Inode& child, const StringView& name, mode_t)
         return ENAMETOOLONG;
 
     m_children.set(name, { name, static_cast<TmpFSInode&>(child) });
-    did_add_child(child.identifier());
+    did_add_child(child.identifier(), name);
     return KSuccess;
 }
 
@@ -300,8 +302,9 @@ KResult TmpFSInode::remove_child(const StringView& name)
     if (it == m_children.end())
         return ENOENT;
     auto child_id = it->value.inode->identifier();
+    it->value.inode->did_delete_self();
     m_children.remove(it);
-    did_remove_child(child_id);
+    did_remove_child(child_id, name);
     return KSuccess;
 }