From e0d9472ced9a0bfbdc41623340c0e0937bda6825 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 3 Feb 2022 01:39:49 +0100 Subject: [PATCH] Kernel: Protect Inode's list of watchers with spinlock instead of mutex --- Kernel/FileSystem/Inode.cpp | 51 +++++++++++++++++++------------------ Kernel/FileSystem/Inode.h | 4 +-- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp index 69945ba8002..078518b8114 100644 --- a/Kernel/FileSystem/Inode.cpp +++ b/Kernel/FileSystem/Inode.cpp @@ -93,9 +93,9 @@ Inode::Inode(FileSystem& fs, InodeIndex index) Inode::~Inode() { - for (auto& watcher : m_watchers) { + m_watchers.for_each([&](auto& watcher) { watcher->unregister_by_inode({}, identifier()); - } + }); } void Inode::will_be_destroyed() @@ -156,17 +156,19 @@ bool Inode::unbind_socket() ErrorOr Inode::register_watcher(Badge, InodeWatcher& watcher) { - MutexLocker locker(m_inode_lock); - VERIFY(!m_watchers.contains(&watcher)); - TRY(m_watchers.try_set(&watcher)); - return {}; + return m_watchers.with([&](auto& watchers) -> ErrorOr { + VERIFY(!watchers.contains(&watcher)); + TRY(watchers.try_set(&watcher)); + return {}; + }); } void Inode::unregister_watcher(Badge, InodeWatcher& watcher) { - MutexLocker locker(m_inode_lock); - VERIFY(m_watchers.contains(&watcher)); - m_watchers.remove(&watcher); + m_watchers.with([&](auto& watchers) { + VERIFY(watchers.contains(&watcher)); + watchers.remove(&watcher); + }); } ErrorOr> Inode::fifo() @@ -197,49 +199,43 @@ void Inode::set_metadata_dirty(bool metadata_dirty) if (m_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) { + m_watchers.for_each([&](auto& watcher) { watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::MetadataModified); - } + }); } } void Inode::did_add_child(InodeIdentifier, StringView name) { - MutexLocker locker(m_inode_lock); - - for (auto& watcher : m_watchers) { + m_watchers.for_each([&](auto& watcher) { watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::ChildCreated, name); - } + }); } void Inode::did_remove_child(InodeIdentifier, StringView name) { - MutexLocker locker(m_inode_lock); - if (name == "." || name == "..") { // These are just aliases and are not interesting to userspace. return; } - for (auto& watcher : m_watchers) { + m_watchers.for_each([&](auto& watcher) { watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::ChildDeleted, name); - } + }); } void Inode::did_modify_contents() { - MutexLocker locker(m_inode_lock); - for (auto& watcher : m_watchers) { + m_watchers.for_each([&](auto& watcher) { watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::ContentModified); - } + }); } void Inode::did_delete_self() { - MutexLocker locker(m_inode_lock); - for (auto& watcher : m_watchers) { + m_watchers.for_each([&](auto& watcher) { watcher->notify_inode_event({}, identifier(), InodeWatcherEvent::Type::Deleted); - } + }); } ErrorOr Inode::prepare_to_write_data() @@ -370,4 +366,9 @@ void Inode::remove_flocks_for_description(OpenFileDescription const& description m_flocks.remove(i--); } } +bool Inode::has_watchers() const +{ + return !m_watchers.with([&](auto& watchers) { return watchers.is_empty(); }); +} + } diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h index 1a074c3591b..871f81af972 100644 --- a/Kernel/FileSystem/Inode.h +++ b/Kernel/FileSystem/Inode.h @@ -89,7 +89,7 @@ public: static void sync_all(); void sync(); - bool has_watchers() const { return !m_watchers.is_empty(); } + bool has_watchers() const; ErrorOr register_watcher(Badge, InodeWatcher&); void unregister_watcher(Badge, InodeWatcher&); @@ -118,7 +118,7 @@ private: InodeIndex m_index { 0 }; WeakPtr m_shared_vmobject; RefPtr m_socket; - HashTable m_watchers; + SpinlockProtected> m_watchers; bool m_metadata_dirty { false }; RefPtr m_fifo; IntrusiveListNode m_inode_list_node;