From f953f3d3ff69ff262a0c02c670d5a704cf0abf63 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Fri, 29 Sep 2023 18:46:44 +0300 Subject: [PATCH] Kernel: Implement MAP_SHARED for regular files Every inode holds a weak pointer to shared file data. This contains physical addresses of pages for inode file data. Physical addresses are allocated and read on demand. When last shared mapping is unmapped. The inodes shared data is freed and written to the inode. --- kernel/include/kernel/FS/Inode.h | 7 + .../include/kernel/Memory/FileBackedRegion.h | 13 ++ kernel/kernel/Memory/FileBackedRegion.cpp | 158 +++++++++++++----- kernel/kernel/Process.cpp | 2 +- 4 files changed, 139 insertions(+), 41 deletions(-) diff --git a/kernel/include/kernel/FS/Inode.h b/kernel/include/kernel/FS/Inode.h index f722b12e08..46d469a21d 100644 --- a/kernel/include/kernel/FS/Inode.h +++ b/kernel/include/kernel/FS/Inode.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -17,6 +18,9 @@ namespace Kernel using namespace API; + class FileBackedRegion; + class SharedFileData; + class Inode : public BAN::RefCounted { public: @@ -112,6 +116,9 @@ namespace Kernel private: mutable RecursiveSpinLock m_lock; + + BAN::WeakPtr m_shared_region; + friend class FileBackedRegion; }; } \ No newline at end of file diff --git a/kernel/include/kernel/Memory/FileBackedRegion.h b/kernel/include/kernel/Memory/FileBackedRegion.h index b2d0df94eb..b7decd8d7e 100644 --- a/kernel/include/kernel/Memory/FileBackedRegion.h +++ b/kernel/include/kernel/Memory/FileBackedRegion.h @@ -6,6 +6,17 @@ namespace Kernel { + struct SharedFileData : public BAN::RefCounted, public BAN::Weakable + { + ~SharedFileData(); + + // FIXME: this should probably be ordered tree like map + // for fast lookup and less memory usage + BAN::Vector pages; + BAN::RefPtr inode; + uint8_t page_buffer[PAGE_SIZE]; + }; + class FileBackedRegion final : public MemoryRegion { BAN_NON_COPYABLE(FileBackedRegion); @@ -25,6 +36,8 @@ namespace Kernel private: BAN::RefPtr m_inode; const off_t m_offset; + + BAN::RefPtr m_shared_data; }; } \ No newline at end of file diff --git a/kernel/kernel/Memory/FileBackedRegion.cpp b/kernel/kernel/Memory/FileBackedRegion.cpp index f4bdb8025f..086c9d62af 100644 --- a/kernel/kernel/Memory/FileBackedRegion.cpp +++ b/kernel/kernel/Memory/FileBackedRegion.cpp @@ -9,9 +9,6 @@ namespace Kernel { ASSERT(inode->mode().ifreg()); - if (type != Type::PRIVATE) - return BAN::Error::from_errno(ENOTSUP); - if (offset < 0 || offset % PAGE_SIZE || size == 0) return BAN::Error::from_errno(EINVAL); if (size > (size_t)inode->size() || (size_t)offset > (size_t)inode->size() - size) @@ -24,6 +21,21 @@ namespace Kernel TRY(region->initialize(address_range)); + if (type == Type::SHARED) + { + LockGuard _(inode->m_lock); + if (inode->m_shared_region.valid()) + region->m_shared_data = inode->m_shared_region.lock(); + else + { + auto shared_data = TRY(BAN::RefPtr::create()); + TRY(shared_data->pages.resize(BAN::Math::div_round_up(inode->size(), PAGE_SIZE))); + shared_data->inode = inode; + inode->m_shared_region = TRY(shared_data->get_weak_ptr()); + region->m_shared_data = BAN::move(shared_data); + } + } + return region; } @@ -38,8 +50,9 @@ namespace Kernel { if (m_vaddr == 0) return; - - ASSERT(m_type == Type::PRIVATE); + + if (m_type == Type::SHARED) + return; size_t needed_pages = BAN::Math::div_round_up(m_size, PAGE_SIZE); for (size_t i = 0; i < needed_pages; i++) @@ -50,10 +63,30 @@ namespace Kernel } } + SharedFileData::~SharedFileData() + { + for (size_t i = 0; i < pages.size(); i++) + { + if (pages[i] == 0) + continue; + + { + auto& page_table = PageTable::current(); + LockGuard _(page_table); + ASSERT(page_table.is_page_free(0)); + + page_table.map_page_at(pages[i], 0, PageTable::Flags::Present); + memcpy(page_buffer, (void*)0, PAGE_SIZE); + page_table.unmap_page(0); + } + + if (auto ret = inode->write(i * PAGE_SIZE, page_buffer, PAGE_SIZE); ret.is_error()) + dwarnln("{}", ret.error()); + } + } + BAN::ErrorOr FileBackedRegion::allocate_page_containing(vaddr_t address) { - ASSERT(m_type == Type::PRIVATE); - ASSERT(contains(address)); // Check if address is already mapped @@ -61,44 +94,89 @@ namespace Kernel if (m_page_table.physical_address_of(vaddr) != 0) return false; - // Map new physcial page to address - paddr_t paddr = Heap::get().take_free_page(); - if (paddr == 0) - return BAN::Error::from_errno(ENOMEM); - m_page_table.map_page_at(paddr, vaddr, m_flags); + if (m_type == Type::PRIVATE) + { + // Map new physcial page to address + paddr_t paddr = Heap::get().take_free_page(); + if (paddr == 0) + return BAN::Error::from_errno(ENOMEM); + m_page_table.map_page_at(paddr, vaddr, m_flags); - size_t file_offset = m_offset + (vaddr - m_vaddr); - size_t bytes = BAN::Math::min(m_size - file_offset, PAGE_SIZE); + size_t file_offset = m_offset + (vaddr - m_vaddr); + size_t bytes = BAN::Math::min(m_size - file_offset, PAGE_SIZE); - BAN::ErrorOr read_ret = 0; + BAN::ErrorOr read_ret = 0; - // Zero out the new page - if (&PageTable::current() == &m_page_table) - read_ret = m_inode->read(file_offset, (void*)vaddr, bytes); + // Zero out the new page + if (&PageTable::current() == &m_page_table) + read_ret = m_inode->read(file_offset, (void*)vaddr, bytes); + else + { + auto& page_table = PageTable::current(); + + LockGuard _(page_table); + ASSERT(page_table.is_page_free(0)); + + page_table.map_page_at(paddr, 0, PageTable::Flags::ReadWrite | PageTable::Flags::Present); + read_ret = m_inode->read(file_offset, (void*)0, bytes); + memset((void*)0, 0x00, PAGE_SIZE); + page_table.unmap_page(0); + } + + if (read_ret.is_error()) + { + Heap::get().release_page(paddr); + m_page_table.unmap_page(vaddr); + return read_ret.release_error(); + } + + if (read_ret.value() < bytes) + { + dwarnln("Only {}/{} bytes read", read_ret.value(), bytes); + Heap::get().release_page(paddr); + m_page_table.unmap_page(vaddr); + return BAN::Error::from_errno(EIO); + } + } + else if (m_type == Type::SHARED) + { + LockGuard _(m_inode->m_lock); + ASSERT(m_inode->m_shared_region.valid()); + ASSERT(m_shared_data->pages.size() == BAN::Math::div_round_up(m_inode->size(), PAGE_SIZE)); + + auto& pages = m_shared_data->pages; + size_t page_index = (vaddr - m_vaddr) / PAGE_SIZE; + + if (pages[page_index] == 0) + { + pages[page_index] = Heap::get().take_free_page(); + if (pages[page_index] == 0) + return BAN::Error::from_errno(ENOMEM); + + size_t offset = vaddr - m_vaddr; + size_t bytes = BAN::Math::min(m_size - offset, PAGE_SIZE); + + TRY(m_inode->read(offset, m_shared_data->page_buffer, bytes)); + + auto& page_table = PageTable::current(); + + // TODO: check if this can cause deadlock? + LockGuard page_table_lock(page_table); + ASSERT(page_table.is_page_free(0)); + + page_table.map_page_at(pages[page_index], 0, PageTable::Flags::ReadWrite | PageTable::Flags::Present); + memcpy((void*)0, m_shared_data->page_buffer, PAGE_SIZE); + page_table.unmap_page(0); + } + + paddr_t paddr = pages[page_index]; + ASSERT(paddr); + + m_page_table.map_page_at(paddr, vaddr, m_flags); + } else { - LockGuard _(PageTable::current()); - ASSERT(PageTable::current().is_page_free(0)); - - PageTable::current().map_page_at(paddr, 0, PageTable::Flags::ReadWrite | PageTable::Flags::Present); - read_ret = m_inode->read(file_offset, (void*)0, bytes); - memset((void*)0, 0x00, PAGE_SIZE); - PageTable::current().unmap_page(0); - } - - if (read_ret.is_error()) - { - Heap::get().release_page(paddr); - m_page_table.unmap_page(vaddr); - return read_ret.release_error(); - } - - if (read_ret.value() < bytes) - { - dwarnln("Only {}/{} bytes read", read_ret.value(), bytes); - Heap::get().release_page(paddr); - m_page_table.unmap_page(vaddr); - return BAN::Error::from_errno(EIO); + ASSERT_NOT_REACHED(); } return true; diff --git a/kernel/kernel/Process.cpp b/kernel/kernel/Process.cpp index f6df4a6e22..507f5fed98 100644 --- a/kernel/kernel/Process.cpp +++ b/kernel/kernel/Process.cpp @@ -863,7 +863,7 @@ namespace Kernel if (!(inode_flags & O_RDONLY)) return BAN::Error::from_errno(EACCES); if (region_type == MemoryRegion::Type::SHARED) - if (!(args->prot & PROT_WRITE) || !(inode_flags & O_WRONLY)) + if ((args->prot & PROT_WRITE) && !(inode_flags & O_WRONLY)) return BAN::Error::from_errno(EACCES); auto region = TRY(FileBackedRegion::create(