From 673592dea83981bb0829ff14c521a78ebb543e69 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 2 Apr 2023 17:25:09 +0200 Subject: [PATCH] Kernel: Stop using *LockRefPtr for FileSystem pointers There was only one permanent storage location for these: as a member in the Mount class. That member is never modified after Mount initialization, so we don't need to worry about races there. --- Kernel/FileSystem/DevPtsFS/FileSystem.cpp | 4 ++-- Kernel/FileSystem/DevPtsFS/FileSystem.h | 2 +- Kernel/FileSystem/Ext2FS/FileSystem.cpp | 4 ++-- Kernel/FileSystem/Ext2FS/FileSystem.h | 2 +- Kernel/FileSystem/FATFS/FileSystem.cpp | 4 ++-- Kernel/FileSystem/FATFS/FileSystem.h | 2 +- Kernel/FileSystem/ISO9660FS/FileSystem.cpp | 4 ++-- Kernel/FileSystem/ISO9660FS/FileSystem.h | 2 +- Kernel/FileSystem/Mount.cpp | 4 ++-- Kernel/FileSystem/Mount.h | 4 ++-- Kernel/FileSystem/Plan9FS/FileSystem.cpp | 4 ++-- Kernel/FileSystem/Plan9FS/FileSystem.h | 2 +- Kernel/FileSystem/ProcFS/FileSystem.cpp | 4 ++-- Kernel/FileSystem/ProcFS/FileSystem.h | 2 +- Kernel/FileSystem/RAMFS/FileSystem.cpp | 4 ++-- Kernel/FileSystem/RAMFS/FileSystem.h | 2 +- Kernel/FileSystem/SysFS/FileSystem.cpp | 4 ++-- Kernel/FileSystem/SysFS/FileSystem.h | 2 +- Kernel/FileSystem/VirtualFileSystem.cpp | 8 ++++---- Kernel/FileSystem/VirtualFileSystem.h | 3 +-- Kernel/Storage/StorageManagement.cpp | 2 +- Kernel/Storage/StorageManagement.h | 2 +- Kernel/Syscalls/mount.cpp | 10 +++++----- 23 files changed, 40 insertions(+), 41 deletions(-) diff --git a/Kernel/FileSystem/DevPtsFS/FileSystem.cpp b/Kernel/FileSystem/DevPtsFS/FileSystem.cpp index c0d64b9440..304f2ae7c3 100644 --- a/Kernel/FileSystem/DevPtsFS/FileSystem.cpp +++ b/Kernel/FileSystem/DevPtsFS/FileSystem.cpp @@ -13,9 +13,9 @@ namespace Kernel { -ErrorOr> DevPtsFS::try_create() +ErrorOr> DevPtsFS::try_create() { - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) DevPtsFS)); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevPtsFS)); } DevPtsFS::DevPtsFS() = default; diff --git a/Kernel/FileSystem/DevPtsFS/FileSystem.h b/Kernel/FileSystem/DevPtsFS/FileSystem.h index db9d32cc39..0ee1dcf801 100644 --- a/Kernel/FileSystem/DevPtsFS/FileSystem.h +++ b/Kernel/FileSystem/DevPtsFS/FileSystem.h @@ -20,7 +20,7 @@ class DevPtsFS final : public FileSystem { public: virtual ~DevPtsFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "DevPtsFS"sv; } diff --git a/Kernel/FileSystem/Ext2FS/FileSystem.cpp b/Kernel/FileSystem/Ext2FS/FileSystem.cpp index e67ab51e7a..02e56ea5aa 100644 --- a/Kernel/FileSystem/Ext2FS/FileSystem.cpp +++ b/Kernel/FileSystem/Ext2FS/FileSystem.cpp @@ -13,9 +13,9 @@ namespace Kernel { -ErrorOr> Ext2FS::try_create(OpenFileDescription& file_description) +ErrorOr> Ext2FS::try_create(OpenFileDescription& file_description) { - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Ext2FS(file_description))); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Ext2FS(file_description))); } Ext2FS::Ext2FS(OpenFileDescription& file_description) diff --git a/Kernel/FileSystem/Ext2FS/FileSystem.h b/Kernel/FileSystem/Ext2FS/FileSystem.h index 89d0f10a9f..760c46bcdb 100644 --- a/Kernel/FileSystem/Ext2FS/FileSystem.h +++ b/Kernel/FileSystem/Ext2FS/FileSystem.h @@ -27,7 +27,7 @@ public: FileSize64bits = 1 << 1, }; - static ErrorOr> try_create(OpenFileDescription&); + static ErrorOr> try_create(OpenFileDescription&); virtual ~Ext2FS() override; diff --git a/Kernel/FileSystem/FATFS/FileSystem.cpp b/Kernel/FileSystem/FATFS/FileSystem.cpp index 6e35f6aede..728738866f 100644 --- a/Kernel/FileSystem/FATFS/FileSystem.cpp +++ b/Kernel/FileSystem/FATFS/FileSystem.cpp @@ -10,9 +10,9 @@ namespace Kernel { -ErrorOr> FATFS::try_create(OpenFileDescription& file_description) +ErrorOr> FATFS::try_create(OpenFileDescription& file_description) { - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) FATFS(file_description))); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) FATFS(file_description))); } FATFS::FATFS(OpenFileDescription& file_description) diff --git a/Kernel/FileSystem/FATFS/FileSystem.h b/Kernel/FileSystem/FATFS/FileSystem.h index d6972f0154..bedf8a7bc0 100644 --- a/Kernel/FileSystem/FATFS/FileSystem.h +++ b/Kernel/FileSystem/FATFS/FileSystem.h @@ -21,7 +21,7 @@ class FATFS final : public BlockBasedFileSystem { friend FATInode; public: - static ErrorOr> try_create(OpenFileDescription&); + static ErrorOr> try_create(OpenFileDescription&); virtual ~FATFS() override = default; virtual StringView class_name() const override { return "FATFS"sv; } diff --git a/Kernel/FileSystem/ISO9660FS/FileSystem.cpp b/Kernel/FileSystem/ISO9660FS/FileSystem.cpp index 450531b597..c95d35f64f 100644 --- a/Kernel/FileSystem/ISO9660FS/FileSystem.cpp +++ b/Kernel/FileSystem/ISO9660FS/FileSystem.cpp @@ -16,9 +16,9 @@ constexpr u32 first_data_area_block = 16; constexpr u32 logical_sector_size = 2048; constexpr u32 max_cached_directory_entries = 128; -ErrorOr> ISO9660FS::try_create(OpenFileDescription& description) +ErrorOr> ISO9660FS::try_create(OpenFileDescription& description) { - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) ISO9660FS(description))); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ISO9660FS(description))); } ISO9660FS::ISO9660FS(OpenFileDescription& description) diff --git a/Kernel/FileSystem/ISO9660FS/FileSystem.h b/Kernel/FileSystem/ISO9660FS/FileSystem.h index 2bb54c5bcb..6a16b4595c 100644 --- a/Kernel/FileSystem/ISO9660FS/FileSystem.h +++ b/Kernel/FileSystem/ISO9660FS/FileSystem.h @@ -29,7 +29,7 @@ class ISO9660FS final : public BlockBasedFileSystem { friend ISO9660DirectoryIterator; public: - static ErrorOr> try_create(OpenFileDescription&); + static ErrorOr> try_create(OpenFileDescription&); virtual ~ISO9660FS() override; virtual StringView class_name() const override { return "ISO9660FS"sv; } diff --git a/Kernel/FileSystem/Mount.cpp b/Kernel/FileSystem/Mount.cpp index a44aebee47..3dbb888477 100644 --- a/Kernel/FileSystem/Mount.cpp +++ b/Kernel/FileSystem/Mount.cpp @@ -11,8 +11,8 @@ namespace Kernel { -Mount::Mount(FileSystem& guest_fs, Custody* host_custody, int flags) - : m_guest(guest_fs.root_inode()) +Mount::Mount(NonnullRefPtr guest_fs, Custody* host_custody, int flags) + : m_guest(guest_fs->root_inode()) , m_guest_fs(guest_fs) , m_host_custody(host_custody) , m_flags(flags) diff --git a/Kernel/FileSystem/Mount.h b/Kernel/FileSystem/Mount.h index a7c8f45c86..25cb8c1abd 100644 --- a/Kernel/FileSystem/Mount.h +++ b/Kernel/FileSystem/Mount.h @@ -20,7 +20,7 @@ class Mount { friend class VirtualFileSystem; public: - Mount(FileSystem&, Custody* host_custody, int flags); + Mount(NonnullRefPtr, Custody* host_custody, int flags); Mount(Inode& source, Custody& host_custody, int flags); RefPtr host() const; @@ -39,7 +39,7 @@ public: private: NonnullRefPtr m_guest; - NonnullLockRefPtr m_guest_fs; + NonnullRefPtr m_guest_fs; SpinlockProtected, LockRank::None> m_host_custody; int m_flags; diff --git a/Kernel/FileSystem/Plan9FS/FileSystem.cpp b/Kernel/FileSystem/Plan9FS/FileSystem.cpp index 19d905698d..d2f0b32245 100644 --- a/Kernel/FileSystem/Plan9FS/FileSystem.cpp +++ b/Kernel/FileSystem/Plan9FS/FileSystem.cpp @@ -10,9 +10,9 @@ namespace Kernel { -ErrorOr> Plan9FS::try_create(OpenFileDescription& file_description) +ErrorOr> Plan9FS::try_create(OpenFileDescription& file_description) { - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Plan9FS(file_description))); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Plan9FS(file_description))); } Plan9FS::Plan9FS(OpenFileDescription& file_description) diff --git a/Kernel/FileSystem/Plan9FS/FileSystem.h b/Kernel/FileSystem/Plan9FS/FileSystem.h index 0bf96da01a..9cbe44fe8e 100644 --- a/Kernel/FileSystem/Plan9FS/FileSystem.h +++ b/Kernel/FileSystem/Plan9FS/FileSystem.h @@ -22,7 +22,7 @@ class Plan9FS final : public FileBackedFileSystem { public: virtual ~Plan9FS() override; - static ErrorOr> try_create(OpenFileDescription&); + static ErrorOr> try_create(OpenFileDescription&); virtual bool supports_watchers() const override { return false; } diff --git a/Kernel/FileSystem/ProcFS/FileSystem.cpp b/Kernel/FileSystem/ProcFS/FileSystem.cpp index 970d523c78..59ecafe179 100644 --- a/Kernel/FileSystem/ProcFS/FileSystem.cpp +++ b/Kernel/FileSystem/ProcFS/FileSystem.cpp @@ -11,9 +11,9 @@ namespace Kernel { -ErrorOr> ProcFS::try_create() +ErrorOr> ProcFS::try_create() { - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) ProcFS)); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ProcFS)); } ProcFS::ProcFS() = default; diff --git a/Kernel/FileSystem/ProcFS/FileSystem.h b/Kernel/FileSystem/ProcFS/FileSystem.h index 6917deb415..38354d0737 100644 --- a/Kernel/FileSystem/ProcFS/FileSystem.h +++ b/Kernel/FileSystem/ProcFS/FileSystem.h @@ -20,7 +20,7 @@ class ProcFS final : public FileSystem { public: virtual ~ProcFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "ProcFS"sv; } diff --git a/Kernel/FileSystem/RAMFS/FileSystem.cpp b/Kernel/FileSystem/RAMFS/FileSystem.cpp index ec89fc6ee4..53ae2eab94 100644 --- a/Kernel/FileSystem/RAMFS/FileSystem.cpp +++ b/Kernel/FileSystem/RAMFS/FileSystem.cpp @@ -10,9 +10,9 @@ namespace Kernel { -ErrorOr> RAMFS::try_create() +ErrorOr> RAMFS::try_create() { - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) RAMFS)); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) RAMFS)); } RAMFS::RAMFS() = default; diff --git a/Kernel/FileSystem/RAMFS/FileSystem.h b/Kernel/FileSystem/RAMFS/FileSystem.h index 08273f1e00..a3ba20b1cf 100644 --- a/Kernel/FileSystem/RAMFS/FileSystem.h +++ b/Kernel/FileSystem/RAMFS/FileSystem.h @@ -18,7 +18,7 @@ class RAMFS final : public FileSystem { public: virtual ~RAMFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "RAMFS"sv; } diff --git a/Kernel/FileSystem/SysFS/FileSystem.cpp b/Kernel/FileSystem/SysFS/FileSystem.cpp index cbd5dd1275..0e054bddc6 100644 --- a/Kernel/FileSystem/SysFS/FileSystem.cpp +++ b/Kernel/FileSystem/SysFS/FileSystem.cpp @@ -11,9 +11,9 @@ namespace Kernel { -ErrorOr> SysFS::try_create() +ErrorOr> SysFS::try_create() { - return TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) SysFS)); + return TRY(adopt_nonnull_ref_or_enomem(new (nothrow) SysFS)); } SysFS::SysFS() = default; diff --git a/Kernel/FileSystem/SysFS/FileSystem.h b/Kernel/FileSystem/SysFS/FileSystem.h index f1669e2287..793c144052 100644 --- a/Kernel/FileSystem/SysFS/FileSystem.h +++ b/Kernel/FileSystem/SysFS/FileSystem.h @@ -20,7 +20,7 @@ class SysFS final : public FileSystem { public: virtual ~SysFS() override; - static ErrorOr> try_create(); + static ErrorOr> try_create(); virtual ErrorOr initialize() override; virtual StringView class_name() const override { return "SysFS"sv; } diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index 50860954f3..1f9a85ae60 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -135,7 +135,7 @@ ErrorOr VirtualFileSystem::remount(Custody& mount_point, int new_flags) void VirtualFileSystem::sync_filesystems() { - Vector, 32> file_systems; + Vector, 32> file_systems; m_file_systems_list.with([&](auto const& list) { for (auto& fs : list) file_systems.append(fs); @@ -147,7 +147,7 @@ void VirtualFileSystem::sync_filesystems() void VirtualFileSystem::lock_all_filesystems() { - Vector, 32> file_systems; + Vector, 32> file_systems; m_file_systems_list.with([&](auto const& list) { for (auto& fs : list) file_systems.append(fs); @@ -334,9 +334,9 @@ ErrorOr VirtualFileSystem::lookup_metadata(Credentials const& cre return custody->inode().metadata(); } -ErrorOr> VirtualFileSystem::find_already_existing_or_create_file_backed_file_system(OpenFileDescription& description, Function>(OpenFileDescription&)> callback) +ErrorOr> VirtualFileSystem::find_already_existing_or_create_file_backed_file_system(OpenFileDescription& description, Function>(OpenFileDescription&)> callback) { - return TRY(m_file_backed_file_systems_list.with([&](auto& list) -> ErrorOr> { + return TRY(m_file_backed_file_systems_list.with([&](auto& list) -> ErrorOr> { for (auto& node : list) { if (&node.file_description() == &description) { return node; diff --git a/Kernel/FileSystem/VirtualFileSystem.h b/Kernel/FileSystem/VirtualFileSystem.h index 5cf0d622c0..361f7809cf 100644 --- a/Kernel/FileSystem/VirtualFileSystem.h +++ b/Kernel/FileSystem/VirtualFileSystem.h @@ -19,7 +19,6 @@ #include #include #include -#include #include namespace Kernel { @@ -82,7 +81,7 @@ public: ErrorOr for_each_mount(Function(Mount const&)>) const; - ErrorOr> find_already_existing_or_create_file_backed_file_system(OpenFileDescription& description, Function>(OpenFileDescription&)> callback); + ErrorOr> find_already_existing_or_create_file_backed_file_system(OpenFileDescription& description, Function>(OpenFileDescription&)> callback); InodeIdentifier root_inode_id() const; diff --git a/Kernel/Storage/StorageManagement.cpp b/Kernel/Storage/StorageManagement.cpp index 415934d839..cf9c589298 100644 --- a/Kernel/Storage/StorageManagement.cpp +++ b/Kernel/Storage/StorageManagement.cpp @@ -446,7 +446,7 @@ u32 StorageManagement::generate_controller_id() return s_controller_id.fetch_add(1); } -NonnullLockRefPtr StorageManagement::root_filesystem() const +NonnullRefPtr StorageManagement::root_filesystem() const { auto boot_device_description = boot_block_device(); if (!boot_device_description) { diff --git a/Kernel/Storage/StorageManagement.h b/Kernel/Storage/StorageManagement.h index b15aced9af..bf106775fa 100644 --- a/Kernel/Storage/StorageManagement.h +++ b/Kernel/Storage/StorageManagement.h @@ -27,7 +27,7 @@ public: void initialize(StringView boot_argument, bool force_pio, bool nvme_poll); static StorageManagement& the(); - NonnullLockRefPtr root_filesystem() const; + NonnullRefPtr root_filesystem() const; static MajorNumber storage_type_major_number(); static MinorNumber generate_storage_minor_number(); diff --git a/Kernel/Syscalls/mount.cpp b/Kernel/Syscalls/mount.cpp index 2ee8e2795d..1c80abf617 100644 --- a/Kernel/Syscalls/mount.cpp +++ b/Kernel/Syscalls/mount.cpp @@ -24,8 +24,8 @@ struct FileSystemInitializer { bool requires_open_file_description { false }; bool requires_block_device { false }; bool requires_seekable_file { false }; - ErrorOr> (*create_with_fd)(OpenFileDescription&) = nullptr; - ErrorOr> (*create)(void) = nullptr; + ErrorOr> (*create_with_fd)(OpenFileDescription&) = nullptr; + ErrorOr> (*create)(void) = nullptr; }; static constexpr FileSystemInitializer s_initializers[] = { @@ -39,14 +39,14 @@ static constexpr FileSystemInitializer s_initializers[] = { { "fat"sv, "FATFS"sv, true, true, true, FATFS::try_create, {} }, }; -static ErrorOr> find_or_create_filesystem_instance(StringView fs_type, OpenFileDescription* possible_description) +static ErrorOr> find_or_create_filesystem_instance(StringView fs_type, OpenFileDescription* possible_description) { for (auto& initializer_entry : s_initializers) { if (fs_type != initializer_entry.short_name && fs_type != initializer_entry.name) continue; if (!initializer_entry.requires_open_file_description) { VERIFY(initializer_entry.create); - NonnullLockRefPtr fs = TRY(initializer_entry.create()); + NonnullRefPtr fs = TRY(initializer_entry.create()); return fs; } // Note: If there's an associated file description with the filesystem, we could @@ -110,7 +110,7 @@ ErrorOr Process::sys$mount(Userspace u return 0; } - LockRefPtr fs; + RefPtr fs; if (!description_or_error.is_error()) { auto description = description_or_error.release_value();