summaryrefslogtreecommitdiff
path: root/Kernel
diff options
context:
space:
mode:
authorLiav A <liavalb@gmail.com>2022-11-21 22:10:56 +0200
committerAndrew Kaster <andrewdkaster@gmail.com>2022-12-09 23:29:33 -0700
commitaa9fab9c3a102ab03d3d5d5a1a721d187ddb8b36 (patch)
treeb9e6397a9c333ac47a6e17cbbf655bd359adbff6 /Kernel
parent905becc991e66112ea89a173388006360e46bf73 (diff)
downloadserenity-aa9fab9c3a102ab03d3d5d5a1a721d187ddb8b36.zip
Kernel/FileSystem: Convert the mount table from Vector to IntrusiveList
The fact that we used a Vector meant that even if creating a Mount object succeeded, we were still at a risk that appending to the actual mounts Vector could fail due to OOM condition. To guard against this, the mount table is now an IntrusiveList, which always means that when allocation of a Mount object succeeded, then inserting that object to the list will succeed, which allows us to fail early in case of OOM condition.
Diffstat (limited to 'Kernel')
-rw-r--r--Kernel/FileSystem/Mount.h5
-rw-r--r--Kernel/FileSystem/VirtualFileSystem.cpp47
-rw-r--r--Kernel/FileSystem/VirtualFileSystem.h2
3 files changed, 34 insertions, 20 deletions
diff --git a/Kernel/FileSystem/Mount.h b/Kernel/FileSystem/Mount.h
index c2cb23be44..6d4dc6c608 100644
--- a/Kernel/FileSystem/Mount.h
+++ b/Kernel/FileSystem/Mount.h
@@ -15,7 +15,10 @@
namespace Kernel {
+class VirtualFileSystem;
class Mount {
+ friend class VirtualFileSystem;
+
public:
Mount(FileSystem&, Custody* host_custody, int flags);
Mount(Inode& source, Custody& host_custody, int flags);
@@ -39,6 +42,8 @@ private:
NonnullLockRefPtr<FileSystem> m_guest_fs;
SpinlockProtected<RefPtr<Custody>> m_host_custody;
int m_flags;
+
+ IntrusiveListNode<Mount> m_vfs_list_node;
};
}
diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp
index 9bed735a10..52ec729898 100644
--- a/Kernel/FileSystem/VirtualFileSystem.cpp
+++ b/Kernel/FileSystem/VirtualFileSystem.cpp
@@ -55,7 +55,7 @@ bool VirtualFileSystem::mount_point_exists_at_inode(InodeIdentifier inode_identi
{
return m_mounts.with([&](auto& mounts) -> bool {
return any_of(mounts, [&inode_identifier](auto const& existing_mount) {
- return existing_mount->host() && existing_mount->host()->identifier() == inode_identifier;
+ return existing_mount.host() && existing_mount.host()->identifier() == inode_identifier;
});
});
}
@@ -78,7 +78,10 @@ ErrorOr<void> VirtualFileSystem::mount(FileSystem& fs, Custody& mount_point, int
new_mount->guest_fs().mounted_count({}).with([&](auto& mounted_count) {
mounted_count++;
});
- mounts.append(move(new_mount));
+
+ // NOTE: Leak the mount pointer so it can be added to the mount list, but it won't be
+ // deleted after being added.
+ mounts.append(*new_mount.leak_ptr());
return {};
});
}
@@ -94,7 +97,10 @@ ErrorOr<void> VirtualFileSystem::bind_mount(Custody& source, Custody& mount_poin
mount_point.inode().identifier());
return EBUSY;
}
- mounts.append(move(new_mount));
+
+ // NOTE: Leak the mount pointer so it can be added to the mount list, but it won't be
+ // deleted after being added.
+ mounts.append(*new_mount.leak_ptr());
return {};
});
}
@@ -142,14 +148,13 @@ ErrorOr<void> VirtualFileSystem::unmount(Custody& mountpoint_custody)
dbgln("VirtualFileSystem: unmount called with inode {} on mountpoint {}", guest_inode.identifier(), custody_path->view());
return m_mounts.with([&](auto& mounts) -> ErrorOr<void> {
- for (size_t i = 0; i < mounts.size(); ++i) {
- auto& mount = mounts[i];
- if (&mount->guest() != &guest_inode)
+ for (auto& mount : mounts) {
+ if (&mount.guest() != &guest_inode)
continue;
- auto mountpoint_path = TRY(mount->absolute_path());
+ auto mountpoint_path = TRY(mount.absolute_path());
if (custody_path->view() != mountpoint_path->view())
continue;
- NonnullRefPtr<FileSystem> fs = mount->guest_fs();
+ NonnullRefPtr<FileSystem> fs = mount.guest_fs();
TRY(fs->prepare_to_unmount());
fs->mounted_count({}).with([&](auto& mounted_count) {
VERIFY(mounted_count > 0);
@@ -170,7 +175,9 @@ ErrorOr<void> VirtualFileSystem::unmount(Custody& mountpoint_custody)
}
});
dbgln("VirtualFileSystem: Unmounting file system {}...", fs->fsid());
- (void)mounts.unstable_take(i);
+ mount.m_vfs_list_node.remove();
+ // Note: This is balanced by a `new` statement that is happening in various places before inserting the Mount object to the list.
+ delete &mount;
return {};
}
dbgln("VirtualFileSystem: Nothing mounted on inode {}", guest_inode.identifier());
@@ -186,7 +193,6 @@ ErrorOr<void> VirtualFileSystem::mount_root(FileSystem& fs)
}
auto new_mount = TRY(adopt_nonnull_own_or_enomem(new (nothrow) Mount(fs, nullptr, root_mount_flags)));
-
auto& root_inode = fs.root_inode();
if (!root_inode.is_directory()) {
dmesgln("VirtualFileSystem: root inode ({}) for / is not a directory :(", root_inode.identifier());
@@ -208,12 +214,15 @@ ErrorOr<void> VirtualFileSystem::mount_root(FileSystem& fs)
fs_list.append(fs);
});
+ fs.mounted_count({}).with([&](auto& mounted_count) {
+ mounted_count++;
+ });
+
// Note: Actually add a mount for the filesystem and increment the filesystem mounted count
m_mounts.with([&](auto& mounts) {
- new_mount->guest_fs().mounted_count({}).with([&](auto& mounted_count) {
- mounted_count++;
- });
- mounts.append(move(new_mount));
+ // NOTE: Leak the mount pointer so it can be added to the mount list, but it won't be
+ // deleted after being added.
+ mounts.append(*new_mount.leak_ptr());
});
RefPtr<Custody> new_root_custody = TRY(Custody::try_create(nullptr, ""sv, *m_root_inode, root_mount_flags));
@@ -227,8 +236,8 @@ auto VirtualFileSystem::find_mount_for_host(InodeIdentifier id) -> Mount*
{
return m_mounts.with([&](auto& mounts) -> Mount* {
for (auto& mount : mounts) {
- if (mount->host() && mount->host()->identifier() == id)
- return mount.ptr();
+ if (mount.host() && mount.host()->identifier() == id)
+ return &mount;
}
return nullptr;
});
@@ -238,8 +247,8 @@ auto VirtualFileSystem::find_mount_for_guest(InodeIdentifier id) -> Mount*
{
return m_mounts.with([&](auto& mounts) -> Mount* {
for (auto& mount : mounts) {
- if (mount->guest().identifier() == id)
- return mount.ptr();
+ if (mount.guest().identifier() == id)
+ return &mount;
}
return nullptr;
});
@@ -859,7 +868,7 @@ ErrorOr<void> VirtualFileSystem::for_each_mount(Function<ErrorOr<void>(Mount con
{
return m_mounts.with([&](auto& mounts) -> ErrorOr<void> {
for (auto& mount : mounts)
- TRY(callback(*mount));
+ TRY(callback(mount));
return {};
});
}
diff --git a/Kernel/FileSystem/VirtualFileSystem.h b/Kernel/FileSystem/VirtualFileSystem.h
index 87decc58e5..cd6ea83944 100644
--- a/Kernel/FileSystem/VirtualFileSystem.h
+++ b/Kernel/FileSystem/VirtualFileSystem.h
@@ -107,7 +107,7 @@ private:
SpinlockProtected<RefPtr<Custody>> m_root_custody;
- SpinlockProtected<Vector<NonnullOwnPtr<Mount>, 16>> m_mounts { LockRank::None };
+ SpinlockProtected<IntrusiveList<&Mount::m_vfs_list_node>> m_mounts { LockRank::None };
SpinlockProtected<IntrusiveList<&FileBackedFileSystem::m_file_backed_file_system_node>> m_file_backed_file_systems_list { LockRank::None };
SpinlockProtected<IntrusiveList<&FileSystem::m_file_system_node>> m_file_systems_list { LockRank::FileSystem };
};