Kernel/FileSystem: Discard safely filesystems when unmounted last time

This commit reached that goal of "safely discarding" a filesystem by
doing the following:
1. Stop using the s_file_system_map HashMap as it was an unsafe measure
to access pointers of FileSystems. Instead, make sure to register all
FileSystems at the VFS layer, with an IntrusiveList, to avoid problems
related to OOM conditions.
2. Make sure to cleanly remove the DiskCache object from a BlockBased
filesystem, so the destructor of such object will not need to do that in
the destruction point.
3. For ext2 filesystems, don't cache the root inode at m_inode_cache
HashMap. The reason for this is that when unmounting an ext2 filesystem,
we lookup at the cache to see if there's a reference to a cached inode
and if that's the case, we fail with EBUSY. If we keep the m_root_inode
also being referenced at the m_inode_cache map, we have 2 references to
that object, which will lead to fail with EBUSY. Also, it's much simpler
to always ask for a root inode and get it immediately from m_root_inode,
instead of looking up the cache for that inode.

Kernel/FileSystem/BlockBasedFileSystem.cpp

@@ -27,7 +27,7 @@ public:
         , m_entries(move(entries_buffer))
     {
         for (size_t i = 0; i < EntryCount; ++i) {
-            entries()[i].data = m_cached_block_data->data() + i * m_fs.block_size();
+            entries()[i].data = m_cached_block_data->data() + i * m_fs->block_size();
             m_clean_list.append(entries()[i]);
         }
     }
@@ -72,7 +72,7 @@ public:
             // Not a single clean entry! Flush writes and try again.
             // NOTE: We want to make sure we only call FileBackedFileSystem flush here,
             //       not some FileBackedFileSystem subclass flush!
-            m_fs.flush_writes_impl();
+            m_fs->flush_writes_impl();
             return ensure(block_index);
         }
 
@@ -100,10 +100,10 @@ public:
     }
 
 private:
-    BlockBasedFileSystem& m_fs;
-    mutable HashMap<BlockBasedFileSystem::BlockIndex, CacheEntry*> m_hash;
-    mutable IntrusiveList<&CacheEntry::list_node> m_clean_list;
+    mutable NonnullRefPtr<BlockBasedFileSystem> m_fs;
     mutable IntrusiveList<&CacheEntry::list_node> m_dirty_list;
+    mutable IntrusiveList<&CacheEntry::list_node> m_clean_list;
+    mutable HashMap<BlockBasedFileSystem::BlockIndex, CacheEntry*> m_hash;
     NonnullOwnPtr<KBuffer> m_cached_block_data;
     NonnullOwnPtr<KBuffer> m_entries;
 };
@@ -116,6 +116,14 @@ BlockBasedFileSystem::BlockBasedFileSystem(OpenFileDescription& file_description
 
 BlockBasedFileSystem::~BlockBasedFileSystem() = default;
 
+void BlockBasedFileSystem::remove_disk_cache_before_last_unmount()
+{
+    VERIFY(m_lock.is_locked());
+    m_cache.with_exclusive([&](auto& cache) {
+        cache.clear();
+    });
+}
+
 ErrorOr<void> BlockBasedFileSystem::initialize_while_locked()
 {
     VERIFY(m_lock.is_locked());

Kernel/FileSystem/BlockBasedFileSystem.h

@@ -41,6 +41,8 @@ protected:
 
     u64 m_logical_block_size { 512 };
 
+    void remove_disk_cache_before_last_unmount();
+
 private:
     DiskCache& cache() const;
     void flush_specific_block_if_needed(BlockIndex index);

Kernel/FileSystem/Ext2FileSystem.cpp

@@ -140,7 +140,7 @@ ErrorOr<void> Ext2FS::initialize_while_locked()
         }
     }
 
-    m_root_inode = static_ptr_cast<Ext2FSInode>(TRY(get_inode({ fsid(), EXT2_ROOT_INO })));
+    m_root_inode = TRY(build_root_inode());
     return {};
 }
 
@@ -770,10 +770,29 @@ ErrorOr<void> Ext2FSInode::flush_metadata()
     return {};
 }
 
+ErrorOr<NonnullLockRefPtr<Ext2FSInode>> Ext2FS::build_root_inode() const
+{
+    MutexLocker locker(m_lock);
+    BlockIndex block_index;
+    unsigned offset;
+    if (!find_block_containing_inode(EXT2_ROOT_INO, block_index, offset))
+        return EINVAL;
+
+    auto inode = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Ext2FSInode(const_cast<Ext2FS&>(*this), EXT2_ROOT_INO)));
+
+    auto buffer = UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast<u8*>(&inode->m_raw_inode));
+    TRY(read_block(block_index, &buffer, sizeof(ext2_inode), offset));
+    return inode;
+}
+
 ErrorOr<NonnullLockRefPtr<Inode>> Ext2FS::get_inode(InodeIdentifier inode) const
 {
     MutexLocker locker(m_lock);
     VERIFY(inode.fsid() == fsid());
+    VERIFY(m_root_inode);
+
+    if (inode.index() == EXT2_ROOT_INO)
+        return *m_root_inode;
 
     {
         auto it = m_inode_cache.find(inode.index());
@@ -1690,12 +1709,12 @@ unsigned Ext2FS::free_inode_count() const
 ErrorOr<void> Ext2FS::prepare_to_clear_last_mount()
 {
     MutexLocker locker(m_lock);
-
     for (auto& it : m_inode_cache) {
         if (it.value->ref_count() > 1)
             return EBUSY;
     }
 
+    BlockBasedFileSystem::remove_disk_cache_before_last_unmount();
     m_inode_cache.clear();
     m_root_inode = nullptr;
     return {};

Kernel/FileSystem/Ext2FileSystem.h

@@ -118,6 +118,8 @@ private:
     u64 blocks_per_group() const;
     u64 inode_size() const;
 
+    ErrorOr<NonnullLockRefPtr<Ext2FSInode>> build_root_inode() const;
+
     ErrorOr<void> write_ext2_inode(InodeIndex, ext2_inode const&);
     bool find_block_containing_inode(InodeIndex, BlockIndex& block_index, unsigned& offset) const;
 

Kernel/FileSystem/FileSystem.cpp

@@ -9,6 +9,7 @@
 #include <AK/StringView.h>
 #include <Kernel/FileSystem/FileSystem.h>
 #include <Kernel/FileSystem/Inode.h>
+#include <Kernel/FileSystem/VirtualFileSystem.h>
 #include <Kernel/InterruptDisabler.h>
 #include <Kernel/Memory/MemoryManager.h>
 #include <Kernel/Net/LocalSocket.h>
@@ -16,30 +17,14 @@
 namespace Kernel {
 
 static u32 s_lastFileSystemID;
-static Singleton<HashMap<FileSystemID, FileSystem*>> s_file_system_map;
-
-static HashMap<FileSystemID, FileSystem*>& all_file_systems()
-{
-    return *s_file_system_map;
-}
 
 FileSystem::FileSystem()
     : m_fsid(++s_lastFileSystemID)
 {
-    s_file_system_map->set(m_fsid, this);
 }
 
 FileSystem::~FileSystem()
 {
-    s_file_system_map->remove(m_fsid);
-}
-
-FileSystem* FileSystem::from_fsid(FileSystemID id)
-{
-    auto it = all_file_systems().find(id);
-    if (it != all_file_systems().end())
-        return (*it).value;
-    return nullptr;
 }
 
 ErrorOr<void> FileSystem::prepare_to_unmount()
@@ -61,23 +46,12 @@ FileSystem::DirectoryEntryView::DirectoryEntryView(StringView n, InodeIdentifier
 void FileSystem::sync()
 {
     Inode::sync_all();
-
-    NonnullLockRefPtrVector<FileSystem, 32> file_systems;
-    {
-        InterruptDisabler disabler;
-        for (auto& it : all_file_systems())
-            file_systems.append(*it.value);
-    }
-
-    for (auto& fs : file_systems)
-        fs.flush_writes();
+    VirtualFileSystem::the().sync_filesystems();
 }
 
 void FileSystem::lock_all()
 {
-    for (auto& it : all_file_systems()) {
-        it.value->m_lock.lock();
-    }
+    VirtualFileSystem::the().lock_all_filesystems();
 }
 
 }

Kernel/FileSystem/FileSystem.h

@@ -20,12 +20,12 @@ namespace Kernel {
 
 class FileSystem : public AtomicRefCounted<FileSystem> {
     friend class Inode;
+    friend class VirtualFileSystem;
 
 public:
     virtual ~FileSystem();
 
     FileSystemID fsid() const { return m_fsid; }
-    static FileSystem* from_fsid(FileSystemID);
     static void sync();
     static void lock_all();
 
@@ -80,18 +80,9 @@ private:
     bool m_readonly { false };
 
     SpinlockProtected<size_t> m_attach_count { LockRank::FileSystem, 0 };
+    IntrusiveListNode<FileSystem> m_file_system_node;
 };
 
-inline FileSystem* InodeIdentifier::fs() // NOLINT(readability-make-member-function-const) const InodeIdentifiers should not be able to modify the FileSystem
-{
-    return FileSystem::from_fsid(m_fsid);
-}
-
-inline FileSystem const* InodeIdentifier::fs() const
-{
-    return FileSystem::from_fsid(m_fsid);
-}
-
 }
 
 namespace AK {

Kernel/FileSystem/ISO9660FileSystem.cpp

@@ -235,6 +235,7 @@ u8 ISO9660FS::internal_file_type_to_directory_entry_type(DirectoryEntryView cons
 ErrorOr<void> ISO9660FS::prepare_to_clear_last_mount()
 {
     // FIXME: Do proper cleaning here.
+    BlockBasedFileSystem::remove_disk_cache_before_last_unmount();
     return {};
 }
 

Kernel/FileSystem/VirtualFileSystem.cpp

@@ -111,6 +111,30 @@ ErrorOr<void> VirtualFileSystem::remount(Custody& mount_point, int new_flags)
     return {};
 }
 
+void VirtualFileSystem::sync_filesystems()
+{
+    NonnullLockRefPtrVector<FileSystem, 32> file_systems;
+    m_file_systems_list.with([&](auto const& list) {
+        for (auto& fs : list)
+            file_systems.append(fs);
+    });
+
+    for (auto& fs : file_systems)
+        fs.flush_writes();
+}
+
+void VirtualFileSystem::lock_all_filesystems()
+{
+    NonnullLockRefPtrVector<FileSystem, 32> file_systems;
+    m_file_systems_list.with([&](auto const& list) {
+        for (auto& fs : list)
+            file_systems.append(fs);
+    });
+
+    for (auto& fs : file_systems)
+        fs.m_lock.lock();
+}
+
 ErrorOr<void> VirtualFileSystem::unmount(Custody& mountpoint_custody)
 {
     auto& guest_inode = mountpoint_custody.inode();
@@ -125,11 +149,27 @@ ErrorOr<void> VirtualFileSystem::unmount(Custody& mountpoint_custody)
             auto mountpoint_path = TRY(mount->absolute_path());
             if (custody_path->view() != mountpoint_path->view())
                 continue;
-            TRY(mount->guest_fs().prepare_to_unmount());
-            mount->guest_fs().mounted_count({}).with([&](auto& mounted_count) {
-                mounted_count--;
+            NonnullRefPtr<FileSystem> fs = mount->guest_fs();
+            TRY(fs->prepare_to_unmount());
+            fs->mounted_count({}).with([&](auto& mounted_count) {
+                VERIFY(mounted_count > 0);
+                if (mounted_count == 1) {
+                    dbgln("VirtualFileSystem: Unmounting file system {} for the last time...", fs->fsid());
+                    m_file_systems_list.with([&](auto& list) {
+                        list.remove(*fs);
                     });
-            dbgln("VirtualFileSystem: Unmounting file system {}...", mount->guest_fs().fsid());
+                    if (fs->is_file_backed()) {
+                        dbgln("VirtualFileSystem: Unmounting file backed file system {} for the last time...", fs->fsid());
+                        auto& file_backed_fs = static_cast<FileBackedFileSystem&>(*fs);
+                        m_file_backed_file_systems_list.with([&](auto& list) {
+                            list.remove(file_backed_fs);
+                        });
+                    }
+                } else {
+                    mounted_count--;
+                }
+            });
+            dbgln("VirtualFileSystem: Unmounting file system {}...", fs->fsid());
             (void)mounts.unstable_take(i);
             return {};
         }
@@ -154,15 +194,20 @@ ErrorOr<void> VirtualFileSystem::mount_root(FileSystem& fs)
     }
 
     m_root_inode = root_inode;
+    if (fs.is_file_backed()) {
         auto pseudo_path = TRY(static_cast<FileBackedFileSystem&>(fs).file_description().pseudo_path());
         dmesgln("VirtualFileSystem: mounted root({}) from {} ({})", fs.fsid(), fs.class_name(), pseudo_path);
-
-    if (fs.is_file_backed()) {
         m_file_backed_file_systems_list.with([&](auto& list) {
             list.append(static_cast<FileBackedFileSystem&>(fs));
         });
+    } else {
+        dmesgln("VirtualFileSystem: mounted root({}) from {}", fs.fsid(), fs.class_name());
     }
 
+    m_file_systems_list.with([&](auto& fs_list) {
+        fs_list.append(fs);
+    });
+
     // 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) {
@@ -278,6 +323,9 @@ ErrorOr<NonnullLockRefPtr<FileBackedFileSystem>> VirtualFileSystem::find_already
         auto fs = TRY(callback(description));
         VERIFY(fs->is_file_backed());
         list.append(static_cast<FileBackedFileSystem&>(*fs));
+        m_file_systems_list.with([&](auto& fs_list) {
+            fs_list.append(*fs);
+        });
         return static_ptr_cast<FileBackedFileSystem>(fs);
     }));
 }

Kernel/FileSystem/VirtualFileSystem.h

@@ -77,6 +77,9 @@ public:
 
     InodeIdentifier root_inode_id() const;
 
+    void sync_filesystems();
+    void lock_all_filesystems();
+
     static void sync();
 
     NonnullRefPtr<Custody> root_custody();
@@ -106,6 +109,7 @@ private:
 
     SpinlockProtected<Vector<NonnullOwnPtr<Mount>, 16>> 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 };
 };
 
 }