From 50472fd69f7fb6faee017c401513c7f6331f0c66 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 26 Jul 2021 00:31:26 +0200 Subject: [PATCH] Kernel: Don't try to return a committed page that we don't have When we get a COW fault and discover that whoever we were COW'ing together with has either COW'ed that page on their end (or they have unmapped/exited) we simplify life for ourselves by clearing the COW bit and keeping the page we already have. (No need to COW if the page is not shared!) The act of doing this does not return a committed page to the pool. In fact, that committed page we had reserved for this purpose was used up (allocated) by our COW buddy when they COW'ed the page. This fixes a kernel panic when running TestLibCMkTemp. :^) --- Kernel/VM/AnonymousVMObject.cpp | 22 +++++++--------------- Kernel/VM/AnonymousVMObject.h | 10 +++++----- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/Kernel/VM/AnonymousVMObject.cpp b/Kernel/VM/AnonymousVMObject.cpp index 4c15078a2e..2700590f43 100644 --- a/Kernel/VM/AnonymousVMObject.cpp +++ b/Kernel/VM/AnonymousVMObject.cpp @@ -319,20 +319,21 @@ PageFaultResponse AnonymousVMObject::handle_cow_fault(size_t page_index, Virtual } auto& page_slot = physical_pages()[page_index]; - bool have_committed = m_shared_committed_cow_pages; if (page_slot->ref_count() == 1) { dbgln_if(PAGE_FAULT_DEBUG, " >> It's a COW page but nobody is sharing it anymore. Remap r/w"); set_should_cow(page_index, false); - if (have_committed) { - if (m_shared_committed_cow_pages->return_one()) - m_shared_committed_cow_pages = nullptr; - } + + // If we were sharing committed COW pages with another process, and the other process + // has exhausted the supply, we can stop counting the shared pages. + if (m_shared_committed_cow_pages && m_shared_committed_cow_pages->is_empty()) + m_shared_committed_cow_pages = nullptr; + return PageFaultResponse::Continue; } RefPtr page; - if (have_committed) { + if (m_shared_committed_cow_pages) { dbgln_if(PAGE_FAULT_DEBUG, " >> It's a committed COW page and it's time to COW!"); page = m_shared_committed_cow_pages->allocate_one(); } else { @@ -386,13 +387,4 @@ NonnullRefPtr CommittedCowPages::allocate_one() return MM.allocate_committed_user_physical_page(MemoryManager::ShouldZeroFill::Yes); } -bool CommittedCowPages::return_one() -{ - VERIFY(m_committed_pages > 0); - m_committed_pages--; - - MM.uncommit_user_physical_pages(1); - return m_committed_pages == 0; -} - } diff --git a/Kernel/VM/AnonymousVMObject.h b/Kernel/VM/AnonymousVMObject.h index 6388918218..277c7e9385 100644 --- a/Kernel/VM/AnonymousVMObject.h +++ b/Kernel/VM/AnonymousVMObject.h @@ -20,14 +20,14 @@ class CommittedCowPages : public RefCounted { public: CommittedCowPages() = delete; - CommittedCowPages(size_t); + explicit CommittedCowPages(size_t); ~CommittedCowPages(); - NonnullRefPtr allocate_one(); - bool return_one(); + [[nodiscard]] NonnullRefPtr allocate_one(); + [[nodiscard]] size_t is_empty() const { return m_committed_pages == 0; } -private: - size_t m_committed_pages; +public: + size_t m_committed_pages { 0 }; }; class AnonymousVMObject final : public VMObject {