From d1f265e85176488055131465baac892a7e2ad4fc Mon Sep 17 00:00:00 2001 From: creator1creeper1 Date: Tue, 11 Jan 2022 18:05:47 +0100 Subject: [PATCH] Kernel: Make VMOBject construction OOM-aware This commit moves the allocation of the resources required for VMObject from its constructors to the constructors of its child classes. We're making this change to give the child classes the chance to expose the fallibility of the allocation. --- Kernel/Memory/AnonymousVMObject.cpp | 8 ++++---- Kernel/Memory/InodeVMObject.cpp | 4 ++-- Kernel/Memory/VMObject.cpp | 24 +++++++++++++++++++----- Kernel/Memory/VMObject.h | 7 +++++-- 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/Kernel/Memory/AnonymousVMObject.cpp b/Kernel/Memory/AnonymousVMObject.cpp index 4511c8760e..32fcfa91f1 100644 --- a/Kernel/Memory/AnonymousVMObject.cpp +++ b/Kernel/Memory/AnonymousVMObject.cpp @@ -114,7 +114,7 @@ ErrorOr> AnonymousVMObject::try_create_for_phys } AnonymousVMObject::AnonymousVMObject(size_t size, AllocationStrategy strategy, Optional committed_pages) - : VMObject(size) + : VMObject(VMObject::must_create_physical_pages_but_fixme_should_propagate_errors(size)) , m_unused_committed_pages(move(committed_pages)) { if (strategy == AllocationStrategy::AllocateNow) { @@ -129,7 +129,7 @@ AnonymousVMObject::AnonymousVMObject(size_t size, AllocationStrategy strategy, O } AnonymousVMObject::AnonymousVMObject(PhysicalAddress paddr, size_t size) - : VMObject(size) + : VMObject(VMObject::must_create_physical_pages_but_fixme_should_propagate_errors(size)) { VERIFY(paddr.page_base() == paddr); for (size_t i = 0; i < page_count(); ++i) @@ -137,7 +137,7 @@ AnonymousVMObject::AnonymousVMObject(PhysicalAddress paddr, size_t size) } AnonymousVMObject::AnonymousVMObject(Span> physical_pages) - : VMObject(physical_pages.size() * PAGE_SIZE) + : VMObject(VMObject::must_create_physical_pages_but_fixme_should_propagate_errors(physical_pages.size() * PAGE_SIZE)) { for (size_t i = 0; i < physical_pages.size(); ++i) { m_physical_pages[i] = physical_pages[i]; @@ -145,7 +145,7 @@ AnonymousVMObject::AnonymousVMObject(Span> physical_ } AnonymousVMObject::AnonymousVMObject(AnonymousVMObject const& other, NonnullRefPtr shared_committed_cow_pages) - : VMObject(other) + : VMObject(other.must_clone_physical_pages_but_fixme_should_propagate_errors()) , m_shared_committed_cow_pages(move(shared_committed_cow_pages)) , m_purgeable(other.m_purgeable) { diff --git a/Kernel/Memory/InodeVMObject.cpp b/Kernel/Memory/InodeVMObject.cpp index 0435fc9522..23e13af719 100644 --- a/Kernel/Memory/InodeVMObject.cpp +++ b/Kernel/Memory/InodeVMObject.cpp @@ -10,14 +10,14 @@ namespace Kernel::Memory { InodeVMObject::InodeVMObject(Inode& inode, size_t size) - : VMObject(size) + : VMObject(VMObject::must_create_physical_pages_but_fixme_should_propagate_errors(size)) , m_inode(inode) , m_dirty_pages(page_count(), false) { } InodeVMObject::InodeVMObject(InodeVMObject const& other) - : VMObject(other) + : VMObject(other.must_clone_physical_pages_but_fixme_should_propagate_errors()) , m_inode(other.m_inode) , m_dirty_pages(page_count(), false) { diff --git a/Kernel/Memory/VMObject.cpp b/Kernel/Memory/VMObject.cpp index 78db6f2d1b..a10897fddb 100644 --- a/Kernel/Memory/VMObject.cpp +++ b/Kernel/Memory/VMObject.cpp @@ -17,14 +17,28 @@ SpinlockProtected& VMObject::all_instances() return s_all_instances; } -VMObject::VMObject(VMObject const& other) - : m_physical_pages(other.m_physical_pages.must_clone_but_fixme_should_propagate_errors()) +ErrorOr>> VMObject::try_clone_physical_pages() const { - all_instances().with([&](auto& list) { list.append(*this); }); + return m_physical_pages.try_clone(); } -VMObject::VMObject(size_t size) - : m_physical_pages(FixedArray>::must_create_but_fixme_should_propagate_errors(ceil_div(size, static_cast(PAGE_SIZE)))) +FixedArray> VMObject::must_clone_physical_pages_but_fixme_should_propagate_errors() const +{ + return MUST(try_clone_physical_pages()); +} + +ErrorOr>> VMObject::try_create_physical_pages(size_t size) +{ + return FixedArray>::try_create(ceil_div(size, static_cast(PAGE_SIZE))); +} + +FixedArray> VMObject::must_create_physical_pages_but_fixme_should_propagate_errors(size_t size) +{ + return MUST(try_create_physical_pages(size)); +} + +VMObject::VMObject(FixedArray>&& new_physical_pages) + : m_physical_pages(move(new_physical_pages)) { all_instances().with([&](auto& list) { list.append(*this); }); } diff --git a/Kernel/Memory/VMObject.h b/Kernel/Memory/VMObject.h index 7997b86cf2..e2c775fd68 100644 --- a/Kernel/Memory/VMObject.h +++ b/Kernel/Memory/VMObject.h @@ -54,8 +54,11 @@ public: } protected: - explicit VMObject(size_t); - explicit VMObject(VMObject const&); + static ErrorOr>> try_create_physical_pages(size_t); + static FixedArray> must_create_physical_pages_but_fixme_should_propagate_errors(size_t); + ErrorOr>> try_clone_physical_pages() const; + FixedArray> must_clone_physical_pages_but_fixme_should_propagate_errors() const; + explicit VMObject(FixedArray>&&); template void for_each_region(Callback);