From 601b0a8c68ee377783670d3c4aa9ef4717f6302f Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 27 Jun 2019 13:34:28 +0200 Subject: [PATCH] Kernel: Use NonnullRefPtrVector in parts of the kernel. --- Kernel/FileSystem/ProcFS.cpp | 36 +++++++++++++------------- Kernel/Process.cpp | 38 +++++++++++++-------------- Kernel/Process.h | 5 ++-- Kernel/VM/MemoryManager.cpp | 50 ++++++++++++++++-------------------- Kernel/VM/MemoryManager.h | 5 ++-- 5 files changed, 65 insertions(+), 69 deletions(-) diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index b64fd70343..d34f065ab2 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -224,17 +224,17 @@ ByteBuffer procfs$pid_vm(InodeIdentifier identifier) builder.appendf("BEGIN END SIZE COMMIT FLAGS NAME\n"); for (auto& region : process.regions()) { StringBuilder flags_builder; - if (region->is_readable()) + if (region.is_readable()) flags_builder.append('R'); - if (region->is_writable()) + if (region.is_writable()) flags_builder.append('W'); builder.appendf("%x -- %x %x %x %-4s %s\n", - region->vaddr().get(), - region->vaddr().offset(region->size() - 1).get(), - region->size(), - region->amount_resident(), + region.vaddr().get(), + region.vaddr().offset(region.size() - 1).get(), + region.size(), + region.amount_resident(), flags_builder.to_string().characters(), - region->name().characters()); + region.name().characters()); } return builder.to_byte_buffer(); } @@ -285,20 +285,20 @@ ByteBuffer procfs$pid_vmo(InodeIdentifier identifier) builder.appendf("BEGIN END SIZE NAME\n"); for (auto& region : process.regions()) { builder.appendf("%x -- %x %x %s\n", - region->vaddr().get(), - region->vaddr().offset(region->size() - 1).get(), - region->size(), - region->name().characters()); + region.vaddr().get(), + region.vaddr().offset(region.size() - 1).get(), + region.size(), + region.name().characters()); builder.appendf("VMO: %s \"%s\" @ %x(%u)\n", - region->vmo().is_anonymous() ? "anonymous" : "file-backed", - region->vmo().name().characters(), - ®ion->vmo(), - region->vmo().ref_count()); - for (int i = 0; i < region->vmo().page_count(); ++i) { - auto& physical_page = region->vmo().physical_pages()[i]; + region.vmo().is_anonymous() ? "anonymous" : "file-backed", + region.vmo().name().characters(), + ®ion.vmo(), + region.vmo().ref_count()); + for (int i = 0; i < region.vmo().page_count(); ++i) { + auto& physical_page = region.vmo().physical_pages()[i]; builder.appendf("P%x%s(%u) ", physical_page ? physical_page->paddr().get() : 0, - region->should_cow(i) ? "!" : "", + region.should_cow(i) ? "!" : "", physical_page ? physical_page->ref_count() : 0); } builder.appendf("\n"); diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 00454a048f..c5f3a22483 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -99,10 +99,10 @@ Region* Process::allocate_region(VirtualAddress vaddr, size_t size, const String if (!range.is_valid()) return nullptr; m_regions.append(adopt(*new Region(range, move(name), prot_to_region_access_flags(prot)))); - MM.map_region(*this, *m_regions.last()); + MM.map_region(*this, m_regions.last()); if (commit) - m_regions.last()->commit(); - return m_regions.last().ptr(); + m_regions.last().commit(); + return &m_regions.last(); } Region* Process::allocate_file_backed_region(VirtualAddress vaddr, size_t size, RefPtr&& inode, const String& name, int prot) @@ -111,8 +111,8 @@ Region* Process::allocate_file_backed_region(VirtualAddress vaddr, size_t size, if (!range.is_valid()) return nullptr; m_regions.append(adopt(*new Region(range, move(inode), name, prot_to_region_access_flags(prot)))); - MM.map_region(*this, *m_regions.last()); - return m_regions.last().ptr(); + MM.map_region(*this, m_regions.last()); + return &m_regions.last(); } Region* Process::allocate_region_with_vmo(VirtualAddress vaddr, size_t size, NonnullRefPtr&& vmo, size_t offset_in_vmo, const String& name, int prot) @@ -122,15 +122,15 @@ Region* Process::allocate_region_with_vmo(VirtualAddress vaddr, size_t size, Non return nullptr; offset_in_vmo &= PAGE_MASK; m_regions.append(adopt(*new Region(range, move(vmo), offset_in_vmo, name, prot_to_region_access_flags(prot)))); - MM.map_region(*this, *m_regions.last()); - return m_regions.last().ptr(); + MM.map_region(*this, m_regions.last()); + return &m_regions.last(); } bool Process::deallocate_region(Region& region) { InterruptDisabler disabler; for (int i = 0; i < m_regions.size(); ++i) { - if (m_regions[i] == ®ion) { + if (&m_regions[i] == ®ion) { page_directory().range_allocator().deallocate({ region.vaddr(), region.size() }); MM.unmap_region(region); m_regions.remove(i); @@ -144,8 +144,8 @@ Region* Process::region_from_range(VirtualAddress vaddr, size_t size) { size = PAGE_ROUND_UP(size); for (auto& region : m_regions) { - if (region->vaddr() == vaddr && region->size() == size) - return region.ptr(); + if (region.vaddr() == vaddr && region.size() == size) + return ®ion; } return nullptr; } @@ -241,9 +241,9 @@ Process* Process::fork(RegisterDump& regs) #ifdef FORK_DEBUG dbgprintf("fork: cloning Region{%p} \"%s\" L%x\n", region.ptr(), region->name().characters(), region->vaddr().get()); #endif - auto cloned_region = region->clone(); + auto cloned_region = region.clone(); child->m_regions.append(move(cloned_region)); - MM.map_region(*child, *child->m_regions.last()); + MM.map_region(*child, child->m_regions.last()); } for (auto gid : m_gids) @@ -649,10 +649,10 @@ void Process::dump_regions() kprintf("BEGIN END SIZE NAME\n"); for (auto& region : m_regions) { kprintf("%x -- %x %x %s\n", - region->vaddr().get(), - region->vaddr().offset(region->size() - 1).get(), - region->size(), - region->name().characters()); + region.vaddr().get(), + region.vaddr().offset(region.size() - 1).get(), + region.size(), + region.name().characters()); } } @@ -2013,7 +2013,7 @@ size_t Process::amount_virtual() const { size_t amount = 0; for (auto& region : m_regions) { - amount += region->size(); + amount += region.size(); } return amount; } @@ -2023,7 +2023,7 @@ size_t Process::amount_resident() const // FIXME: This will double count if multiple regions use the same physical page. size_t amount = 0; for (auto& region : m_regions) { - amount += region->amount_resident(); + amount += region.amount_resident(); } return amount; } @@ -2036,7 +2036,7 @@ size_t Process::amount_shared() const // so that every Region contributes +1 ref to each of its PhysicalPages. size_t amount = 0; for (auto& region : m_regions) { - amount += region->amount_shared(); + amount += region.amount_shared(); } return amount; } diff --git a/Kernel/Process.h b/Kernel/Process.h index c8c7583603..86946af79e 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -211,7 +212,7 @@ public: void set_tty(TTY* tty) { m_tty = tty; } size_t region_count() const { return m_regions.size(); } - const Vector>& regions() const { return m_regions; } + const NonnullRefPtrVector& regions() const { return m_regions; } void dump_regions(); ProcessTracer* tracer() { return m_tracer.ptr(); } @@ -326,7 +327,7 @@ private: Region* region_from_range(VirtualAddress, size_t); - Vector> m_regions; + NonnullRefPtrVector m_regions; VirtualAddress m_return_to_ring3_from_signal_trampoline; VirtualAddress m_return_to_ring0_from_signal_trampoline; diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp index 2eca7940fa..f49a438923 100644 --- a/Kernel/VM/MemoryManager.cpp +++ b/Kernel/VM/MemoryManager.cpp @@ -79,7 +79,7 @@ void MemoryManager::initialize_paging() #endif m_quickmap_addr = VirtualAddress((1 * MB) - PAGE_SIZE); - RefPtr region = nullptr; + RefPtr region; bool region_is_super = false; for (auto* mmap = (multiboot_memory_map_t*)multiboot_info_ptr->mmap_addr; (unsigned long)mmap < multiboot_info_ptr->mmap_addr + multiboot_info_ptr->mmap_length; mmap = (multiboot_memory_map_t*)((unsigned long)mmap + mmap->size + sizeof(mmap->size))) { @@ -118,7 +118,7 @@ void MemoryManager::initialize_paging() } else { if (region.is_null() || region_is_super || region->upper().offset(PAGE_SIZE) != addr) { m_user_physical_regions.append(PhysicalRegion::create(addr, addr)); - region = m_user_physical_regions.last(); + region = &m_user_physical_regions.last(); region_is_super = false; } else { region->expand(region->lower(), addr); @@ -128,10 +128,10 @@ void MemoryManager::initialize_paging() } for (auto& region : m_super_physical_regions) - m_super_physical_pages += region->finalize_capacity(); + m_super_physical_pages += region.finalize_capacity(); for (auto& region : m_user_physical_regions) - m_user_physical_pages += region->finalize_capacity(); + m_user_physical_pages += region.finalize_capacity(); #ifdef MM_DEBUG dbgprintf("MM: Installing page directory\n"); @@ -268,8 +268,8 @@ Region* MemoryManager::region_from_vaddr(Process& process, VirtualAddress vaddr) // FIXME: Use a binary search tree (maybe red/black?) or some other more appropriate data structure! for (auto& region : process.m_regions) { - if (region->contains(vaddr)) - return region.ptr(); + if (region.contains(vaddr)) + return ®ion; } dbgprintf("%s(%u) Couldn't find region for L%x (CR3=%x)\n", process.name().characters(), process.pid(), vaddr.get(), process.page_directory().cr3()); return nullptr; @@ -286,8 +286,8 @@ const Region* MemoryManager::region_from_vaddr(const Process& process, VirtualAd // FIXME: Use a binary search tree (maybe red/black?) or some other more appropriate data structure! for (auto& region : process.m_regions) { - if (region->contains(vaddr)) - return region.ptr(); + if (region.contains(vaddr)) + return ®ion; } dbgprintf("%s(%u) Couldn't find region for L%x (CR3=%x)\n", process.name().characters(), process.pid(), vaddr.get(), process.page_directory().cr3()); return nullptr; @@ -467,15 +467,15 @@ RefPtr MemoryManager::allocate_kernel_region(size_t size, String&& name) void MemoryManager::deallocate_user_physical_page(PhysicalPage&& page) { for (auto& region : m_user_physical_regions) { - if (!region->contains(page)) { + if (!region.contains(page)) { kprintf( "MM: deallocate_user_physical_page: %p not in %p -> %p\n", - page.paddr(), region->lower().get(), region->upper().get()); + page.paddr(), region.lower().get(), region.upper().get()); continue; } - region->return_page(move(page)); - m_user_physical_pages_used--; + region.return_page(move(page)); + --m_user_physical_pages_used; return; } @@ -487,11 +487,10 @@ void MemoryManager::deallocate_user_physical_page(PhysicalPage&& page) RefPtr MemoryManager::allocate_user_physical_page(ShouldZeroFill should_zero_fill) { InterruptDisabler disabler; - - RefPtr page = nullptr; + RefPtr page; for (auto& region : m_user_physical_regions) { - page = region->take_free_page(false); + page = region.take_free_page(false); if (page.is_null()) continue; } @@ -516,24 +515,22 @@ RefPtr MemoryManager::allocate_user_physical_page(ShouldZeroFill s unquickmap_page(); } - m_user_physical_pages_used++; - + ++m_user_physical_pages_used; return page; } void MemoryManager::deallocate_supervisor_physical_page(PhysicalPage&& page) { for (auto& region : m_super_physical_regions) { - if (!region->contains(page)) { + if (!region.contains(page)) { kprintf( "MM: deallocate_supervisor_physical_page: %p not in %p -> %p\n", - page.paddr(), region->lower().get(), region->upper().get()); + page.paddr(), region.lower().get(), region.upper().get()); continue; } - region->return_page(move(page)); - m_super_physical_pages_used--; - + region.return_page(move(page)); + --m_super_physical_pages_used; return; } @@ -544,11 +541,10 @@ void MemoryManager::deallocate_supervisor_physical_page(PhysicalPage&& page) RefPtr MemoryManager::allocate_supervisor_physical_page() { InterruptDisabler disabler; - - RefPtr page = nullptr; + RefPtr page; for (auto& region : m_super_physical_regions) { - page = region->take_free_page(true); + page = region.take_free_page(true); if (page.is_null()) continue; } @@ -568,9 +564,7 @@ RefPtr MemoryManager::allocate_supervisor_physical_page() #endif fast_dword_fill((dword*)page->paddr().as_ptr(), 0, PAGE_SIZE / sizeof(dword)); - - m_super_physical_pages_used++; - + ++m_super_physical_pages_used; return page; } diff --git a/Kernel/VM/MemoryManager.h b/Kernel/VM/MemoryManager.h index 7acf50c72b..2a758f12dd 100644 --- a/Kernel/VM/MemoryManager.h +++ b/Kernel/VM/MemoryManager.h @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -125,8 +126,8 @@ private: unsigned m_super_physical_pages { 0 }; unsigned m_super_physical_pages_used { 0 }; - Vector> m_user_physical_regions {}; - Vector> m_super_physical_regions {}; + NonnullRefPtrVector m_user_physical_regions; + NonnullRefPtrVector m_super_physical_regions; HashTable m_vmos; HashTable m_user_regions;