From d481ae95b5f1267a06221af7e3b77cfa3d4872d7 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 1 Oct 2019 19:58:41 +0200 Subject: [PATCH] Kernel: Defer creation of Region CoW bitmaps until they're needed Instead of allocating and populating a Copy-on-Write bitmap for each Region up front, wait until we actually clone the Region for sharing with another process. In most cases, we never need any CoW bits and we save ourselves a lot of kmalloc() memory and time. --- Kernel/VM/MemoryManager.cpp | 5 ++-- Kernel/VM/Region.cpp | 51 +++++++++++++++++++++++++------------ Kernel/VM/Region.h | 22 ++++++++-------- 3 files changed, 49 insertions(+), 29 deletions(-) diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp index 4e6733b9b3..0e3cfcfc3d 100644 --- a/Kernel/VM/MemoryManager.cpp +++ b/Kernel/VM/MemoryManager.cpp @@ -307,7 +307,6 @@ bool MemoryManager::zero_page(Region& region, unsigned page_index_in_region) #ifdef PAGE_FAULT_DEBUG dbgprintf(" >> ZERO P%p\n", physical_page->paddr().get()); #endif - region.set_should_cow(page_index_in_region, false); vmo.physical_pages()[page_index_in_region] = move(physical_page); remap_region_page(region, page_index_in_region); return true; @@ -468,9 +467,9 @@ OwnPtr MemoryManager::allocate_kernel_region(size_t size, const StringVi ASSERT(range.is_valid()); OwnPtr region; if (user_accessible) - region = Region::create_user_accessible(range, name, PROT_READ | PROT_WRITE | PROT_EXEC, false); + region = Region::create_user_accessible(range, name, PROT_READ | PROT_WRITE | PROT_EXEC); else - region = Region::create_kernel_only(range, name, PROT_READ | PROT_WRITE | PROT_EXEC, false); + region = Region::create_kernel_only(range, name, PROT_READ | PROT_WRITE | PROT_EXEC); MM.map_region_at_address(*m_kernel_page_directory, *region, range.base()); // FIXME: It would be cool if these could zero-fill on demand instead. if (should_commit) diff --git a/Kernel/VM/Region.cpp b/Kernel/VM/Region.cpp index 2a3345b480..1fae206797 100644 --- a/Kernel/VM/Region.cpp +++ b/Kernel/VM/Region.cpp @@ -6,33 +6,30 @@ #include #include -Region::Region(const Range& range, const String& name, u8 access, bool cow) +Region::Region(const Range& range, const String& name, u8 access) : m_range(range) , m_vmobject(AnonymousVMObject::create_with_size(size())) , m_name(name) , m_access(access) - , m_cow_map(Bitmap::create(m_vmobject->page_count(), cow)) { MM.register_region(*this); } -Region::Region(const Range& range, RefPtr&& inode, const String& name, u8 access, bool cow) +Region::Region(const Range& range, RefPtr&& inode, const String& name, u8 access) : m_range(range) , m_vmobject(InodeVMObject::create_with_inode(*inode)) , m_name(name) , m_access(access) - , m_cow_map(Bitmap::create(m_vmobject->page_count(), cow)) { MM.register_region(*this); } -Region::Region(const Range& range, NonnullRefPtr vmo, size_t offset_in_vmo, const String& name, u8 access, bool cow) +Region::Region(const Range& range, NonnullRefPtr vmo, size_t offset_in_vmo, const String& name, u8 access) : m_range(range) , m_offset_in_vmo(offset_in_vmo) , m_vmobject(move(vmo)) , m_name(name) , m_access(access) - , m_cow_map(Bitmap::create(m_vmobject->page_count(), cow)) { MM.register_region(*this); } @@ -77,9 +74,11 @@ NonnullOwnPtr Region::clone() vaddr().get()); #endif // Set up a COW region. The parent (this) region becomes COW as well! - m_cow_map.fill(true); + ensure_cow_map().fill(true); MM.remap_region(current->process().page_directory(), *this); - return Region::create_user_accessible(m_range, m_vmobject->clone(), m_offset_in_vmo, m_name, m_access, true); + auto clone_region = Region::create_user_accessible(m_range, m_vmobject->clone(), m_offset_in_vmo, m_name, m_access); + clone_region->ensure_cow_map(); + return clone_region; } int Region::commit() @@ -123,30 +122,50 @@ size_t Region::amount_shared() const return bytes; } -NonnullOwnPtr Region::create_user_accessible(const Range& range, const StringView& name, u8 access, bool cow) +NonnullOwnPtr Region::create_user_accessible(const Range& range, const StringView& name, u8 access) { - auto region = make(range, name, access, cow); + auto region = make(range, name, access); region->m_user_accessible = true; return region; } -NonnullOwnPtr Region::create_user_accessible(const Range& range, NonnullRefPtr vmobject, size_t offset_in_vmobject, const StringView& name, u8 access, bool cow) +NonnullOwnPtr Region::create_user_accessible(const Range& range, NonnullRefPtr vmobject, size_t offset_in_vmobject, const StringView& name, u8 access) { - auto region = make(range, move(vmobject), offset_in_vmobject, name, access, cow); + auto region = make(range, move(vmobject), offset_in_vmobject, name, access); region->m_user_accessible = true; return region; } -NonnullOwnPtr Region::create_user_accessible(const Range& range, NonnullRefPtr inode, const StringView& name, u8 access, bool cow) +NonnullOwnPtr Region::create_user_accessible(const Range& range, NonnullRefPtr inode, const StringView& name, u8 access) { - auto region = make(range, move(inode), name, access, cow); + auto region = make(range, move(inode), name, access); region->m_user_accessible = true; return region; } -NonnullOwnPtr Region::create_kernel_only(const Range& range, const StringView& name, u8 access, bool cow) +NonnullOwnPtr Region::create_kernel_only(const Range& range, const StringView& name, u8 access) { - auto region = make(range, name, access, cow); + auto region = make(range, name, access); region->m_user_accessible = false; return region; } + +bool Region::should_cow(size_t page_index) const +{ + if (m_shared) + return false; + return m_cow_map && m_cow_map->get(page_index); +} + +void Region::set_should_cow(size_t page_index, bool cow) +{ + ASSERT(!m_shared); + ensure_cow_map().set(page_index, cow); +} + +Bitmap& Region::ensure_cow_map() const +{ + if (!m_cow_map) + m_cow_map = make(page_count(), true); + return *m_cow_map; +} diff --git a/Kernel/VM/Region.h b/Kernel/VM/Region.h index 8ee5600e4d..82b626cf11 100644 --- a/Kernel/VM/Region.h +++ b/Kernel/VM/Region.h @@ -21,10 +21,10 @@ public: Execute = 4, }; - static NonnullOwnPtr create_user_accessible(const Range&, const StringView& name, u8 access, bool cow = false); - static NonnullOwnPtr create_user_accessible(const Range&, NonnullRefPtr, size_t offset_in_vmobject, const StringView& name, u8 access, bool cow = false); - static NonnullOwnPtr create_user_accessible(const Range&, NonnullRefPtr, const StringView& name, u8 access, bool cow = false); - static NonnullOwnPtr create_kernel_only(const Range&, const StringView& name, u8 access, bool cow = false); + static NonnullOwnPtr create_user_accessible(const Range&, const StringView& name, u8 access); + static NonnullOwnPtr create_user_accessible(const Range&, NonnullRefPtr, size_t offset_in_vmobject, const StringView& name, u8 access); + static NonnullOwnPtr create_user_accessible(const Range&, NonnullRefPtr, const StringView& name, u8 access); + static NonnullOwnPtr create_kernel_only(const Range&, const StringView& name, u8 access); ~Region(); @@ -103,8 +103,8 @@ public: m_page_directory.clear(); } - bool should_cow(size_t page_index) const { return m_cow_map.get(page_index); } - void set_should_cow(size_t page_index, bool cow) { m_cow_map.set(page_index, cow); } + bool should_cow(size_t page_index) const; + void set_should_cow(size_t page_index, bool); void set_writable(bool b) { @@ -119,11 +119,13 @@ public: Region* m_prev { nullptr }; // NOTE: These are public so we can make<> them. - Region(const Range&, const String&, u8 access, bool cow = false); - Region(const Range&, NonnullRefPtr, size_t offset_in_vmo, const String&, u8 access, bool cow = false); - Region(const Range&, RefPtr&&, const String&, u8 access, bool cow = false); + Region(const Range&, const String&, u8 access); + Region(const Range&, NonnullRefPtr, size_t offset_in_vmo, const String&, u8 access); + Region(const Range&, RefPtr&&, const String&, u8 access); private: + Bitmap& ensure_cow_map() const; + RefPtr m_page_directory; Range m_range; size_t m_offset_in_vmo { 0 }; @@ -132,5 +134,5 @@ private: u8 m_access { 0 }; bool m_shared { false }; bool m_user_accessible { false }; - Bitmap m_cow_map; + mutable OwnPtr m_cow_map; };