From dd58d0f6508606993d67df1e256ebd563212be71 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 5 Aug 2021 17:14:13 +0200 Subject: [PATCH] Kernel: Uncommit a shared COW page when discovering it was unshared When we hit a COW fault and discover than no other process is sharing the physical page, we simply remap it r/w and save ourselves the trouble. When this happens, we can also give back (uncommit) one of our shared committed COW pages, since we won't be needing it. We had this optimization before, but I mistakenly removed it in 50472fd69f7fb6faee017c401513c7f6331f0c66 since I had misunderstood it to be the reason for a panic. --- Kernel/VM/AnonymousVMObject.cpp | 12 ++++++++++++ Kernel/VM/AnonymousVMObject.h | 4 +++- Kernel/VM/MemoryManager.cpp | 7 +++++++ Kernel/VM/MemoryManager.h | 1 + 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/Kernel/VM/AnonymousVMObject.cpp b/Kernel/VM/AnonymousVMObject.cpp index 6807c44da0..5bd9297d66 100644 --- a/Kernel/VM/AnonymousVMObject.cpp +++ b/Kernel/VM/AnonymousVMObject.cpp @@ -315,6 +315,12 @@ PageFaultResponse AnonymousVMObject::handle_cow_fault(size_t page_index, Virtual 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 (m_shared_committed_cow_pages) { + m_shared_committed_cow_pages->uncommit_one(); + if (!m_shared_committed_cow_pages->is_empty()) + m_shared_committed_cow_pages = nullptr; + } return PageFaultResponse::Continue; } @@ -368,4 +374,10 @@ NonnullRefPtr AnonymousVMObject::SharedCommittedCowPages::take_one return m_committed_pages.take_one(); } +void AnonymousVMObject::SharedCommittedCowPages::uncommit_one() +{ + ScopedSpinLock locker(m_lock); + m_committed_pages.uncommit_one(); +} + } diff --git a/Kernel/VM/AnonymousVMObject.h b/Kernel/VM/AnonymousVMObject.h index 88a523605d..0a6d38aadd 100644 --- a/Kernel/VM/AnonymousVMObject.h +++ b/Kernel/VM/AnonymousVMObject.h @@ -70,9 +70,11 @@ private: explicit SharedCommittedCowPages(CommittedPhysicalPageSet&&); ~SharedCommittedCowPages(); - [[nodiscard]] NonnullRefPtr take_one(); [[nodiscard]] bool is_empty() const { return m_committed_pages.is_empty(); } + [[nodiscard]] NonnullRefPtr take_one(); + void uncommit_one(); + public: SpinLock m_lock; CommittedPhysicalPageSet m_committed_pages; diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp index 6173221678..2df0469669 100644 --- a/Kernel/VM/MemoryManager.cpp +++ b/Kernel/VM/MemoryManager.cpp @@ -1137,4 +1137,11 @@ NonnullRefPtr CommittedPhysicalPageSet::take_one() return MM.allocate_committed_user_physical_page(MemoryManager::ShouldZeroFill::Yes); } +void CommittedPhysicalPageSet::uncommit_one() +{ + VERIFY(m_page_count > 0); + --m_page_count; + MM.uncommit_user_physical_pages({}, 1); +} + } diff --git a/Kernel/VM/MemoryManager.h b/Kernel/VM/MemoryManager.h index d9e912504f..f548e182ca 100644 --- a/Kernel/VM/MemoryManager.h +++ b/Kernel/VM/MemoryManager.h @@ -126,6 +126,7 @@ public: size_t page_count() const { return m_page_count; } [[nodiscard]] NonnullRefPtr take_one(); + void uncommit_one(); void operator=(CommittedPhysicalPageSet&&) = delete;