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; };