From a7356716ffcf2d76f1f7579d2980880e5d95bb4f Mon Sep 17 00:00:00 2001 From: Bananymous Date: Sat, 9 May 2026 15:08:26 +0300 Subject: [PATCH] Kernel: Reduce locking FileBackedRegions --- .../include/kernel/Memory/FileBackedRegion.h | 6 +- kernel/kernel/Memory/FileBackedRegion.cpp | 59 +++++++++++-------- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/kernel/include/kernel/Memory/FileBackedRegion.h b/kernel/include/kernel/Memory/FileBackedRegion.h index 94f47e5e..4b6c5706 100644 --- a/kernel/include/kernel/Memory/FileBackedRegion.h +++ b/kernel/include/kernel/Memory/FileBackedRegion.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include namespace Kernel @@ -10,15 +11,14 @@ namespace Kernel { ~SharedFileData(); - void sync(size_t page_index); + void sync_no_lock(size_t page_index); - Mutex mutex; + RWLock rw_lock; // 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 diff --git a/kernel/kernel/Memory/FileBackedRegion.cpp b/kernel/kernel/Memory/FileBackedRegion.cpp index 42c54663..d29a07ac 100644 --- a/kernel/kernel/Memory/FileBackedRegion.cpp +++ b/kernel/kernel/Memory/FileBackedRegion.cpp @@ -2,8 +2,12 @@ #include #include +#include + #include +#pragma GCC diagnostic ignored "-Wstack-usage=" + namespace Kernel { @@ -62,28 +66,23 @@ namespace Kernel SharedFileData::~SharedFileData() { - // no-one should be referencing this anymore - [[maybe_unused]] bool success = mutex.try_lock(); - ASSERT(success); + // TODO: validate that this is not locked for (size_t i = 0; i < pages.size(); i++) { if (pages[i] == 0) continue; - sync(i); + sync_no_lock(i); Heap::get().release_page(pages[i]); } - - mutex.unlock(); } - void SharedFileData::sync(size_t page_index) + void SharedFileData::sync_no_lock(size_t page_index) { - ASSERT(mutex.is_locked()); - if (pages[page_index] == 0) return; + uint8_t page_buffer[PAGE_SIZE]; PageTable::with_fast_page(pages[page_index], [&] { memcpy(page_buffer, PageTable::fast_page_as_ptr(), PAGE_SIZE); }); @@ -103,10 +102,10 @@ namespace Kernel const vaddr_t first_page = address & PAGE_ADDR_MASK; const vaddr_t last_page = BAN::Math::div_round_up(address + size, PAGE_SIZE) * PAGE_SIZE; - LockGuard _(m_shared_data->mutex); + RWLockRDGuard _(m_shared_data->rw_lock); for (vaddr_t page_addr = first_page; page_addr < last_page; page_addr += PAGE_SIZE) if (contains(page_addr)) - m_shared_data->sync((m_offset + page_addr - m_vaddr) / PAGE_SIZE); + m_shared_data->sync_no_lock((m_offset + page_addr - m_vaddr) / PAGE_SIZE); return {}; } @@ -125,29 +124,43 @@ namespace Kernel if (m_page_table.physical_address_of(vaddr) == 0) { ASSERT(m_shared_data); - LockGuard _(m_shared_data->mutex); + + uint8_t page_buffer[PAGE_SIZE]; + + m_shared_data->rw_lock.rd_lock(); bool shared_data_has_correct_page = false; if (m_shared_data->pages[shared_page_index] == 0) { - m_shared_data->pages[shared_page_index] = Heap::get().take_free_page(); - if (m_shared_data->pages[shared_page_index] == 0) - return BAN::Error::from_errno(ENOMEM); + m_shared_data->rw_lock.rd_unlock(); const size_t offset = (vaddr - m_vaddr) + m_offset; ASSERT(offset % PAGE_SIZE == 0); const size_t bytes = BAN::Math::min(m_inode->size() - offset, PAGE_SIZE); - TRY(m_inode->read(offset, BAN::ByteSpan(m_shared_data->page_buffer, bytes))); - memset(m_shared_data->page_buffer + bytes, 0, PAGE_SIZE - bytes); + TRY(m_inode->read(offset, BAN::ByteSpan(page_buffer, bytes))); + memset(page_buffer + bytes, 0, PAGE_SIZE - bytes); - PageTable::with_fast_page(m_shared_data->pages[shared_page_index], [&] { - memcpy(PageTable::fast_page_as_ptr(), m_shared_data->page_buffer, PAGE_SIZE); - }); - shared_data_has_correct_page = true; + { + RWLockWRGuard _(m_shared_data->rw_lock); + if (m_shared_data->pages[shared_page_index] == 0) + { + m_shared_data->pages[shared_page_index] = Heap::get().take_free_page(); + if (m_shared_data->pages[shared_page_index] == 0) + return BAN::Error::from_errno(ENOMEM); + PageTable::with_fast_page(m_shared_data->pages[shared_page_index], [&] { + memcpy(PageTable::fast_page_as_ptr(), page_buffer, PAGE_SIZE); + }); + shared_data_has_correct_page = true; + } + } + + m_shared_data->rw_lock.rd_lock(); } + BAN::ScopeGuard _([this] { m_shared_data->rw_lock.rd_unlock(); }); + if (m_type == Type::PRIVATE && wants_write) { const paddr_t paddr = Heap::get().take_free_page(); @@ -156,11 +169,11 @@ namespace Kernel if (!shared_data_has_correct_page) { PageTable::with_fast_page(m_shared_data->pages[shared_page_index], [&] { - memcpy(m_shared_data->page_buffer, PageTable::fast_page_as_ptr(), PAGE_SIZE); + memcpy(page_buffer, PageTable::fast_page_as_ptr(), PAGE_SIZE); }); } PageTable::with_fast_page(paddr, [&] { - memcpy(PageTable::fast_page_as_ptr(), m_shared_data->page_buffer, PAGE_SIZE); + memcpy(PageTable::fast_page_as_ptr(), page_buffer, PAGE_SIZE); }); m_dirty_pages[local_page_index] = paddr; m_page_table.map_page_at(paddr, vaddr, m_flags);