From ab4f03338553b8d636725f14e1e524a1160dd6e5 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Mon, 6 Nov 2023 20:07:09 +0200 Subject: [PATCH] Kernel: Cleanup TmpFS code and block access doesn't require allocs TmpFS blocks are now accessed with a simple wrapper --- kernel/include/kernel/FS/TmpFS/FileSystem.h | 43 +++++-- kernel/include/kernel/FS/TmpFS/Inode.h | 21 ++-- kernel/kernel/FS/TmpFS/FileSystem.cpp | 54 ++++---- kernel/kernel/FS/TmpFS/Inode.cpp | 131 ++++++++++---------- 4 files changed, 132 insertions(+), 117 deletions(-) diff --git a/kernel/include/kernel/FS/TmpFS/FileSystem.h b/kernel/include/kernel/FS/TmpFS/FileSystem.h index 0c3115334..b90b85c1a 100644 --- a/kernel/include/kernel/FS/TmpFS/FileSystem.h +++ b/kernel/include/kernel/FS/TmpFS/FileSystem.h @@ -4,16 +4,29 @@ #include #include #include +#include #include namespace Kernel { - template - concept for_each_indirect_paddr_allocating_callback = requires(F func, paddr_t paddr, bool was_allocated) + namespace TmpFuncs { - requires BAN::is_same_v; - }; + + template + concept for_each_indirect_paddr_allocating_callback = requires(F func, paddr_t paddr, bool was_allocated) + { + requires BAN::is_same_v; + }; + + template + concept with_block_buffer_callback = requires(F func, BAN::ByteSpan buffer) + { + requires BAN::is_same_v; + }; + + } + class TmpFileSystem : public FileSystem { @@ -27,6 +40,7 @@ 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); // FIXME: read_block and write_block should not require external buffer // probably some wrapper like PageTable::with_fast_page could work? @@ -36,8 +50,8 @@ namespace Kernel void delete_inode(ino_t ino); BAN::ErrorOr allocate_inode(const TmpInodeInfo&); - void read_block(size_t index, BAN::ByteSpan buffer); - void write_block(size_t index, BAN::ConstByteSpan buffer); + template + void with_block_buffer(size_t index, F callback); void free_block(size_t index); BAN::ErrorOr allocate_block(); @@ -46,7 +60,8 @@ namespace Kernel { enum Flags : paddr_t { - Present = 1 << 0, + Present = 1 << 0, + Internal = 1 << 1, }; // 12 bottom bits of paddr can be used as flags, since @@ -79,9 +94,9 @@ namespace Kernel paddr_t find_block(size_t index); - template + template BAN::ErrorOr for_each_indirect_paddr_allocating(PageInfo page_info, F callback, size_t depth); - template + template BAN::ErrorOr for_each_indirect_paddr_allocating_internal(PageInfo page_info, F callback, size_t depth); paddr_t find_indirect(PageInfo root, size_t index, size_t depth); @@ -119,4 +134,14 @@ namespace Kernel const size_t m_max_pages; }; + template + void TmpFileSystem::with_block_buffer(size_t index, F callback) + { + paddr_t block_paddr = find_block(index); + PageTable::with_fast_page(block_paddr, [&] { + BAN::ByteSpan buffer(reinterpret_cast(PageTable::fast_page()), PAGE_SIZE); + callback(buffer); + }); + } + } \ No newline at end of file diff --git a/kernel/include/kernel/FS/TmpFS/Inode.h b/kernel/include/kernel/FS/TmpFS/Inode.h index 4a4da99f8..f80dd74bf 100644 --- a/kernel/include/kernel/FS/TmpFS/Inode.h +++ b/kernel/include/kernel/FS/TmpFS/Inode.h @@ -8,6 +8,17 @@ namespace Kernel { + namespace TmpFuncs + { + + template + concept for_each_entry_callback = requires(F func, const TmpDirectoryEntry& entry) + { + requires BAN::is_same_v; + }; + + } + class TmpFileSystem; class TmpInode : public Inode @@ -51,7 +62,7 @@ namespace Kernel class TmpFileInode : public TmpInode { public: - static BAN::ErrorOr> create(TmpFileSystem&, mode_t, uid_t, gid_t); + static BAN::ErrorOr> create_new(TmpFileSystem&, mode_t, uid_t, gid_t); ~TmpFileInode(); protected: @@ -74,12 +85,6 @@ namespace Kernel TmpSymlinkInode(TmpFileSystem&, ino_t, const TmpInodeInfo&, BAN::StringView target); }; - template - concept for_each_entry_callback = requires(F func, const TmpDirectoryEntry& entry) - { - requires BAN::is_same_v; - }; - class TmpDirectoryInode : public TmpInode { public: @@ -100,7 +105,7 @@ namespace Kernel BAN::ErrorOr link_inode(TmpInode&, BAN::StringView); - template + template void for_each_entry(F callback); friend class TmpInode; diff --git a/kernel/kernel/FS/TmpFS/FileSystem.cpp b/kernel/kernel/FS/TmpFS/FileSystem.cpp index d5f90029f..97e30855a 100644 --- a/kernel/kernel/FS/TmpFS/FileSystem.cpp +++ b/kernel/kernel/FS/TmpFS/FileSystem.cpp @@ -1,6 +1,5 @@ #include #include -#include namespace Kernel { @@ -39,7 +38,6 @@ namespace Kernel }); m_root_inode = TRY(TmpDirectoryInode::create_root(*this, mode, uid, gid)); - TRY(m_inode_cache.insert(m_root_inode->ino(), m_root_inode)); return {}; } @@ -66,6 +64,13 @@ namespace Kernel return inode; } + BAN::ErrorOr TmpFileSystem::add_to_cache(BAN::RefPtr inode) + { + if (!m_inode_cache.contains(inode->ino())) + TRY(m_inode_cache.insert(inode->ino(), inode)); + return {}; + } + void TmpFileSystem::read_inode(ino_t ino, TmpInodeInfo& out) { auto inode_location = find_inode(ino); @@ -135,24 +140,6 @@ namespace Kernel }; } - void TmpFileSystem::read_block(size_t index, BAN::ByteSpan buffer) - { - ASSERT(buffer.size() >= PAGE_SIZE); - paddr_t block_paddr = find_block(index); - PageTable::with_fast_page(block_paddr, [&] { - memcpy(buffer.data(), PageTable::fast_page_as_ptr(), PAGE_SIZE); - }); - } - - void TmpFileSystem::write_block(size_t index, BAN::ConstByteSpan buffer) - { - ASSERT(buffer.size() >= PAGE_SIZE); - paddr_t block_paddr = find_block(index); - PageTable::with_fast_page(block_paddr, [&] { - memcpy(PageTable::fast_page_as_ptr(), buffer.data(), PAGE_SIZE); - }); - } - void TmpFileSystem::free_block(size_t index) { ASSERT_NOT_REACHED(); @@ -180,12 +167,15 @@ namespace Kernel { ASSERT(root.flags() & PageInfo::Flags::Present); if (depth == 0) + { + ASSERT(index == 0); return root.paddr(); + } constexpr size_t addresses_per_page = PAGE_SIZE / sizeof(PageInfo); size_t divisor = 1; - for (size_t i = 0; i < depth; i++) + for (size_t i = 1; i < depth; i++) divisor *= addresses_per_page; size_t index_of_page = index / divisor; @@ -201,11 +191,15 @@ namespace Kernel return find_indirect(next, index_in_page, depth - 1); } - template + template BAN::ErrorOr TmpFileSystem::for_each_indirect_paddr_allocating_internal(PageInfo page_info, F callback, size_t depth) { - ASSERT_GT(depth, 0); ASSERT(page_info.flags() & PageInfo::Flags::Present); + if (depth == 0) + { + bool is_new_block = page_info.flags() & PageInfo::Flags::Internal; + return callback(page_info.paddr(), is_new_block); + } for (size_t i = 0; i < PAGE_SIZE / sizeof(PageInfo); i++) { @@ -214,8 +208,6 @@ namespace Kernel next_info = PageTable::fast_page_as_sized(i); }); - bool allocated = false; - if (!(next_info.flags() & PageInfo::Flags::Present)) { paddr_t new_paddr = Heap::get().take_free_page(); @@ -234,15 +226,11 @@ namespace Kernel to_update_info = next_info; }); - allocated = true; + // Don't sync the internal bit to actual memory + next_info.set_flags(PageInfo::Flags::Internal | PageInfo::Flags::Present); } - BAN::Iteration result; - if (depth == 1) - result = callback(next_info.paddr(), allocated); - else - result = TRY(for_each_indirect_paddr_allocating_internal(next_info, callback, depth - 1)); - + auto result = TRY(for_each_indirect_paddr_allocating_internal(next_info, callback, depth - 1)); switch (result) { case BAN::Iteration::Continue: @@ -257,7 +245,7 @@ namespace Kernel return BAN::Iteration::Continue; } - template + template BAN::ErrorOr TmpFileSystem::for_each_indirect_paddr_allocating(PageInfo page_info, F callback, size_t depth) { BAN::Iteration result = TRY(for_each_indirect_paddr_allocating_internal(page_info, callback, depth)); diff --git a/kernel/kernel/FS/TmpFS/Inode.cpp b/kernel/kernel/FS/TmpFS/Inode.cpp index a97dd1b3a..4d01b1a19 100644 --- a/kernel/kernel/FS/TmpFS/Inode.cpp +++ b/kernel/kernel/FS/TmpFS/Inode.cpp @@ -64,7 +64,10 @@ namespace Kernel : m_fs(fs) , m_inode_info(info) , m_ino(ino) - {} + { + // FIXME: this should be able to fail + MUST(fs.add_to_cache(this)); + } void TmpInode::sync() { @@ -102,7 +105,7 @@ namespace Kernel /* FILE INODE */ - BAN::ErrorOr> TmpFileInode::create(TmpFileSystem& fs, mode_t mode, uid_t uid, gid_t gid) + BAN::ErrorOr> TmpFileInode::create_new(TmpFileSystem& fs, mode_t mode, uid_t uid, gid_t gid) { auto info = create_inode_info(Mode::IFREG | mode, uid, gid); ino_t ino = TRY(fs.allocate_inode(info)); @@ -131,15 +134,12 @@ namespace Kernel m_fs.delete_inode(ino()); } - BAN::ErrorOr TmpFileInode::read_impl(off_t offset, BAN::ByteSpan buffer) + BAN::ErrorOr TmpFileInode::read_impl(off_t offset, BAN::ByteSpan out_buffer) { - if (offset >= size() || buffer.size() == 0) + if (offset >= size() || out_buffer.size() == 0) return 0; - BAN::Vector block_buffer; - TRY(block_buffer.resize(blksize())); - - const size_t bytes_to_read = BAN::Math::min(size() - offset, buffer.size()); + const size_t bytes_to_read = BAN::Math::min(size() - offset, out_buffer.size()); size_t read_done = 0; while (read_done < bytes_to_read) @@ -152,11 +152,11 @@ namespace Kernel const size_t bytes = BAN::Math::min(bytes_to_read - read_done, blksize() - block_offset); if (block_index.has_value()) - m_fs.read_block(block_index.value(), block_buffer.span()); + m_fs.with_block_buffer(block_index.value(), [&](BAN::ByteSpan block_buffer) { + memcpy(out_buffer.data() + read_done, block_buffer.data() + block_offset, bytes); + }); else - memset(block_buffer.data(), 0x00, block_buffer.size()); - - memcpy(buffer.data() + read_done, block_buffer.data() + block_offset, bytes); + memset(out_buffer.data() + read_done, 0x00, bytes); read_done += bytes; } @@ -164,17 +164,14 @@ namespace Kernel return read_done; } - BAN::ErrorOr TmpFileInode::write_impl(off_t offset, BAN::ConstByteSpan buffer) + BAN::ErrorOr TmpFileInode::write_impl(off_t offset, BAN::ConstByteSpan in_buffer) { // FIXME: handle overflow - if (offset + buffer.size() > (size_t)size()) - TRY(truncate_impl(offset + buffer.size())); + if (offset + in_buffer.size() > (size_t)size()) + TRY(truncate_impl(offset + in_buffer.size())); - BAN::Vector block_buffer; - TRY(block_buffer.resize(blksize())); - - const size_t bytes_to_write = buffer.size(); + const size_t bytes_to_write = in_buffer.size(); size_t write_done = 0; while (write_done < bytes_to_write) @@ -186,11 +183,9 @@ namespace Kernel const size_t bytes = BAN::Math::min(bytes_to_write - write_done, blksize() - block_offset); - if (bytes < (size_t)blksize()) - m_fs.read_block(block_index, block_buffer.span()); - memcpy(block_buffer.data() + block_offset, buffer.data() + write_done, bytes); - - m_fs.write_block(block_index, block_buffer.span()); + m_fs.with_block_buffer(block_index, [&](BAN::ByteSpan block_buffer) { + memcpy(block_buffer.data() + block_offset, in_buffer.data() + write_done, bytes); + }); write_done += bytes; } @@ -233,7 +228,7 @@ namespace Kernel auto inode = BAN::RefPtr::adopt(inode_ptr); TRY(inode->link_inode(*inode, "."sv)); - TRY(inode->link_inode(parent, "."sv)); + TRY(inode->link_inode(parent, ".."sv)); return inode; } @@ -282,7 +277,7 @@ namespace Kernel BAN::ErrorOr TmpDirectoryInode::create_file_impl(BAN::StringView name, mode_t mode, uid_t uid, gid_t gid) { - auto new_inode = TRY(TmpFileInode::create(m_fs, mode, uid, gid)); + auto new_inode = TRY(TmpFileInode::create_new(m_fs, mode, uid, gid)); TRY(link_inode(*new_inode, name)); return {}; } @@ -303,37 +298,41 @@ namespace Kernel { static constexpr size_t directory_entry_alignment = 16; - size_t current_size = size(); - 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; ASSERT(new_entry_size < (size_t)blksize()); - size_t new_entry_offset = current_size % blksize(); + size_t new_entry_offset = size() % blksize(); // Target is the last block, or if it doesn't fit the new entry, the next one. - size_t target_data_block = current_size / blksize(); + size_t target_data_block = size() / blksize(); if (blksize() - new_entry_offset < new_entry_size) + { + // insert an empty entry at the end of current block + m_fs.with_block_buffer(block_index(target_data_block).value(), [&](BAN::ByteSpan bytespan) { + auto& empty_entry = bytespan.slice(new_entry_offset).as(); + empty_entry.type = DT_UNKNOWN; + empty_entry.ino = 0; + empty_entry.rec_len = blksize() - new_entry_offset; + }); + m_inode_info.size += blksize() - new_entry_offset; + target_data_block++; + new_entry_offset = 0; + } size_t block_index = TRY(block_index_with_allocation(target_data_block)); - - BAN::Vector buffer; - TRY(buffer.resize(blksize())); - BAN::ByteSpan bytespan = buffer.span(); - - m_fs.read_block(block_index, bytespan); - - auto& new_entry = bytespan.slice(new_entry_offset).as(); - new_entry.type = inode_mode_to_dt_type(inode.mode()); - new_entry.ino = inode.ino(); - new_entry.name_len = name.size(); - new_entry.rec_len = new_entry_size; - memcpy(new_entry.name, name.data(), name.size()); - - m_fs.write_block(block_index, bytespan); + m_fs.with_block_buffer(block_index, [&](BAN::ByteSpan bytespan) { + auto& new_entry = bytespan.slice(new_entry_offset).as(); + ASSERT(new_entry.type == DT_UNKNOWN); + new_entry.type = inode_mode_to_dt_type(inode.mode()); + new_entry.ino = inode.ino(); + new_entry.name_len = name.size(); + new_entry.rec_len = new_entry_size; + memcpy(new_entry.name, name.data(), name.size()); + }); // increase current size m_inode_info.size += new_entry_size; @@ -344,33 +343,31 @@ namespace Kernel return {}; } - template + template void TmpDirectoryInode::for_each_entry(F callback) { - size_t full_offset = 0; - while (full_offset < (size_t)size()) + for (size_t data_block_index = 0; data_block_index * blksize() < (size_t)size(); data_block_index++) { - const size_t data_block_index = full_offset / blksize(); const size_t block_index = this->block_index(data_block_index).value(); + const size_t byte_count = BAN::Math::min(size() - data_block_index * blksize(), blksize()); - // FIXME: implement fast heap pages? - BAN::Vector buffer; - MUST(buffer.resize(blksize())); - - BAN::ByteSpan bytespan = buffer.span(); - m_fs.read_block(block_index, bytespan); - - size_t byte_count = BAN::Math::min(blksize(), size() - full_offset); - - bytespan = bytespan.slice(0, byte_count); - while (bytespan.size() > 0) - { - auto& entry = bytespan.as(); - callback(entry); - bytespan = bytespan.slice(entry.rec_len); - } - - full_offset += blksize(); + m_fs.with_block_buffer(block_index, [&](BAN::ByteSpan bytespan) { + bytespan = bytespan.slice(0, byte_count); + while (bytespan.size() > 0) + { + const auto& entry = bytespan.as(); + switch (callback(entry)) + { + case BAN::Iteration::Continue: + break; + case BAN::Iteration::Break: + return; + default: + ASSERT_NOT_REACHED(); + } + bytespan = bytespan.slice(entry.rec_len); + } + }); } }