From a20f8607de825a0237eab73b4530e411e66132ed Mon Sep 17 00:00:00 2001 From: Bananymous Date: Mon, 6 Nov 2023 21:14:39 +0200 Subject: [PATCH] Kernel: Implement TmpFS Inode unlinking and deletion --- kernel/include/kernel/FS/TmpFS/FileSystem.h | 2 + kernel/include/kernel/FS/TmpFS/Inode.h | 5 ++ kernel/kernel/FS/TmpFS/FileSystem.cpp | 22 ++++- kernel/kernel/FS/TmpFS/Inode.cpp | 97 ++++++++++++++++----- 4 files changed, 102 insertions(+), 24 deletions(-) diff --git a/kernel/include/kernel/FS/TmpFS/FileSystem.h b/kernel/include/kernel/FS/TmpFS/FileSystem.h index b90b85c1..775b4959 100644 --- a/kernel/include/kernel/FS/TmpFS/FileSystem.h +++ b/kernel/include/kernel/FS/TmpFS/FileSystem.h @@ -40,7 +40,9 @@ namespace Kernel virtual BAN::RefPtr root_inode() override { return m_root_inode; } BAN::ErrorOr> open_inode(ino_t ino); + BAN::ErrorOr add_to_cache(BAN::RefPtr); + void remove_from_cache(BAN::RefPtr); // FIXME: read_block and write_block should not require external buffer // probably some wrapper like PageTable::with_fast_page could work? diff --git a/kernel/include/kernel/FS/TmpFS/Inode.h b/kernel/include/kernel/FS/TmpFS/Inode.h index 287f4264..fa385185 100644 --- a/kernel/include/kernel/FS/TmpFS/Inode.h +++ b/kernel/include/kernel/FS/TmpFS/Inode.h @@ -40,12 +40,14 @@ namespace Kernel public: static BAN::ErrorOr> create_from_existing(TmpFileSystem&, ino_t, const TmpInodeInfo&); + ~TmpInode(); protected: TmpInode(TmpFileSystem&, ino_t, const TmpInodeInfo&); void sync(); void free_all_blocks(); + virtual BAN::ErrorOr prepare_unlink() { return {}; }; BAN::Optional block_index(size_t data_block_index); BAN::ErrorOr block_index_with_allocation(size_t data_block_index); @@ -95,6 +97,9 @@ namespace Kernel ~TmpDirectoryInode(); + protected: + virtual BAN::ErrorOr prepare_unlink() override; + protected: virtual BAN::ErrorOr> find_inode_impl(BAN::StringView) override final; virtual BAN::ErrorOr list_next_inodes_impl(off_t, DirectoryEntryList*, size_t) override final; diff --git a/kernel/kernel/FS/TmpFS/FileSystem.cpp b/kernel/kernel/FS/TmpFS/FileSystem.cpp index 97e30855..6e2cfd81 100644 --- a/kernel/kernel/FS/TmpFS/FileSystem.cpp +++ b/kernel/kernel/FS/TmpFS/FileSystem.cpp @@ -71,6 +71,12 @@ namespace Kernel return {}; } + void TmpFileSystem::remove_from_cache(BAN::RefPtr inode) + { + ASSERT(m_inode_cache.contains(inode->ino())); + m_inode_cache.remove(inode->ino()); + } + void TmpFileSystem::read_inode(ino_t ino, TmpInodeInfo& out) { auto inode_location = find_inode(ino); @@ -98,6 +104,7 @@ namespace Kernel ASSERT_EQ(paddr, 0); inode_info = {}; }); + ASSERT(!m_inode_cache.contains(ino)); } BAN::ErrorOr TmpFileSystem::allocate_inode(const TmpInodeInfo& info) @@ -142,7 +149,20 @@ namespace Kernel void TmpFileSystem::free_block(size_t index) { - ASSERT_NOT_REACHED(); + constexpr size_t addresses_per_page = PAGE_SIZE / sizeof(PageInfo); + + const size_t index_of_page = (index - first_data_page) / addresses_per_page; + const size_t index_in_page = (index - first_data_page) % addresses_per_page; + + paddr_t page_containing = find_indirect(m_data_pages, index_of_page, 2); + + PageTable::with_fast_page(page_containing, [&] { + auto& page_info = PageTable::fast_page_as_sized(index_in_page); + ASSERT(page_info.flags() & PageInfo::Flags::Present); + Heap::get().release_page(page_info.paddr()); + page_info.set_paddr(0); + page_info.set_flags(0); + }); } BAN::ErrorOr TmpFileSystem::allocate_block() diff --git a/kernel/kernel/FS/TmpFS/Inode.cpp b/kernel/kernel/FS/TmpFS/Inode.cpp index 9569b6cf..7bfed1ae 100644 --- a/kernel/kernel/FS/TmpFS/Inode.cpp +++ b/kernel/kernel/FS/TmpFS/Inode.cpp @@ -69,6 +69,17 @@ namespace Kernel MUST(fs.add_to_cache(this)); } + TmpInode::~TmpInode() + { + if (nlink() > 0) + { + sync(); + return; + } + free_all_blocks(); + m_fs.delete_inode(ino()); + } + void TmpInode::sync() { m_fs.write_inode(m_ino, m_inode_info); @@ -76,6 +87,12 @@ namespace Kernel void TmpInode::free_all_blocks() { + for (size_t i = 0; i < TmpInodeInfo::direct_block_count; i++) + { + if (m_inode_info.block[i]) + m_fs.free_block(m_inode_info.block[i]); + m_inode_info.block[i] = 0; + } for (auto block : m_inode_info.block) ASSERT(block == 0); } @@ -125,13 +142,6 @@ namespace Kernel TmpFileInode::~TmpFileInode() { - if (nlink() > 0) - { - sync(); - return; - } - free_all_blocks(); - m_fs.delete_inode(ino()); } BAN::ErrorOr TmpFileInode::read_impl(off_t offset, BAN::ByteSpan out_buffer) @@ -247,13 +257,46 @@ namespace Kernel TmpDirectoryInode::~TmpDirectoryInode() { - if (nlink() >= 2) + } + + BAN::ErrorOr TmpDirectoryInode::prepare_unlink() + { + ino_t dot_ino = 0; + ino_t dotdot_ino = 0; + + bool is_empty = true; + for_each_valid_entry([&](TmpDirectoryEntry& entry) { + if (entry.name_sv() == "."sv) + dot_ino = entry.ino; + else if (entry.name_sv() == ".."sv) + dotdot_ino = entry.ino; + else + { + is_empty = false; + return BAN::Iteration::Break; + } + return BAN::Iteration::Continue; + }); + if (!is_empty) + return BAN::Error::from_errno(ENOTEMPTY); + + // FIXME: can these leak inodes? + + if (dot_ino) { - sync(); - return; + auto inode = TRY(m_fs.open_inode(dot_ino)); + ASSERT(inode->nlink() > 0); + inode->m_inode_info.nlink--; } - free_all_blocks(); - m_fs.delete_inode(ino()); + + if (dotdot_ino) + { + auto inode = TRY(m_fs.open_inode(dotdot_ino)); + ASSERT(inode->nlink() > 0); + inode->m_inode_info.nlink--; + } + + return {}; } BAN::ErrorOr> TmpDirectoryInode::find_inode_impl(BAN::StringView name) @@ -335,36 +378,38 @@ namespace Kernel return {}; } - BAN::ErrorOr TmpDirectoryInode::unlink_impl(BAN::StringView) + BAN::ErrorOr TmpDirectoryInode::unlink_impl(BAN::StringView name) { ino_t entry_ino = 0; for_each_valid_entry([&](TmpDirectoryEntry& entry) { if (entry.name_sv() != name) return BAN::Iteration::Continue; - - // get ino of entry entry_ino = entry.ino; - - // invalidate the entry - entry.ino = 0; - entry.type = DT_UNKNOWN; - return BAN::Iteration::Break; }); if (entry_ino == 0) return BAN::Error::from_errno(ENOENT); - // FIXME: this should be able to fail - auto inode = MUST(m_fs.open_inode(entry_ino)); + auto inode = TRY(m_fs.open_inode(entry_ino)); ASSERT(inode->nlink() > 0); + + TRY(inode->prepare_unlink()); inode->m_inode_info.nlink--; - if (inode->nlink() == 0 || (inode->mode().ifdir() && inode->nlink() <= 2)) + if (inode->nlink() == 0) m_fs.remove_from_cache(inode); + for_each_valid_entry([&](TmpDirectoryEntry& entry) { + if (entry.name_sv() != name) + return BAN::Iteration::Continue; + entry.ino = 0; + entry.type = DT_UNKNOWN; + return BAN::Iteration::Break; + }); + return {}; } @@ -372,6 +417,12 @@ namespace Kernel { static constexpr size_t directory_entry_alignment = 16; + auto find_result = find_inode_impl(name); + if (!find_result.is_error()) + return BAN::Error::from_errno(EEXIST); + if (find_result.error().get_error_code() != ENOENT) + return find_result.release_error(); + size_t new_entry_size = sizeof(TmpDirectoryEntry) + name.size(); if (auto rem = new_entry_size % directory_entry_alignment) new_entry_size += directory_entry_alignment - rem;