mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 13:12:46 +00:00 
			
		
		
		
	Kernel: Make Region single-owner instead of ref-counted
This simplifies the ownership model and makes Region easier to reason about. Userspace Regions are now primarily kept by Process::m_regions. Kernel Regions are kept in various OwnPtr<Regions>'s. Regions now only ever get unmapped when they are destroyed.
This commit is contained in:
		
							parent
							
								
									7a7f6a24e9
								
							
						
					
					
						commit
						7f9a33dba1
					
				
					 9 changed files with 44 additions and 41 deletions
				
			
		|  | @ -54,7 +54,7 @@ private: | |||
|         const char* name; | ||||
|     }; | ||||
| #ifdef KERNEL | ||||
|     mutable RefPtr<Region> m_sorted_symbols_region; | ||||
|     mutable OwnPtr<Region> m_sorted_symbols_region; | ||||
| #else | ||||
|     mutable Vector<SortedSymbol> m_sorted_symbols; | ||||
| #endif | ||||
|  |  | |||
|  | @ -21,7 +21,7 @@ public: | |||
|     { | ||||
|         auto region = MM.allocate_kernel_region(PAGE_ROUND_UP(size), "KBuffer", false, false); | ||||
|         ASSERT(region); | ||||
|         return adopt(*new KBufferImpl(*region, size)); | ||||
|         return adopt(*new KBufferImpl(region.release_nonnull(), size)); | ||||
|     } | ||||
| 
 | ||||
|     static NonnullRefPtr<KBufferImpl> copy(const void* data, size_t size) | ||||
|  | @ -43,14 +43,14 @@ public: | |||
|     } | ||||
| 
 | ||||
| private: | ||||
|     explicit KBufferImpl(NonnullRefPtr<Region>&& region, size_t size) | ||||
|     explicit KBufferImpl(NonnullOwnPtr<Region>&& region, size_t size) | ||||
|         : m_size(size) | ||||
|         , m_region(move(region)) | ||||
|     { | ||||
|     } | ||||
| 
 | ||||
|     size_t m_size { 0 }; | ||||
|     NonnullRefPtr<Region> m_region; | ||||
|     NonnullOwnPtr<Region> m_region; | ||||
| }; | ||||
| 
 | ||||
| class KBuffer { | ||||
|  |  | |||
|  | @ -152,7 +152,6 @@ bool Process::deallocate_region(Region& region) | |||
|     InterruptDisabler disabler; | ||||
|     for (int i = 0; i < m_regions.size(); ++i) { | ||||
|         if (&m_regions[i] == ®ion) { | ||||
|             MM.unmap_region(region); | ||||
|             m_regions.remove(i); | ||||
|             return true; | ||||
|         } | ||||
|  | @ -305,12 +304,11 @@ Process* Process::fork(RegisterDump& regs) | |||
| #ifdef FORK_DEBUG | ||||
|         dbg() << "fork: cloning Region{" << ®ion << "} '" << region.name() << "' @ " << region.vaddr(); | ||||
| #endif | ||||
|         auto cloned_region = region.clone(); | ||||
|         child->m_regions.append(move(cloned_region)); | ||||
|         child->m_regions.append(region.clone()); | ||||
|         MM.map_region(*child, child->m_regions.last()); | ||||
| 
 | ||||
|         if (®ion == m_master_tls_region) | ||||
|             child->m_master_tls_region = child->m_regions.last(); | ||||
|             child->m_master_tls_region = &child->m_regions.last(); | ||||
|     } | ||||
| 
 | ||||
|     for (auto gid : m_gids) | ||||
|  | @ -401,10 +399,14 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir | |||
| 
 | ||||
|     ASSERT(description->inode()); | ||||
|     auto vmo = InodeVMObject::create_with_inode(*description->inode()); | ||||
|     RefPtr<Region> region = allocate_region_with_vmo(VirtualAddress(), metadata.size, vmo, 0, description->absolute_path(), PROT_READ); | ||||
|     auto* region = allocate_region_with_vmo(VirtualAddress(), metadata.size, vmo, 0, description->absolute_path(), PROT_READ); | ||||
|     ASSERT(region); | ||||
| 
 | ||||
|     RefPtr<Region> master_tls_region; | ||||
|     // NOTE: We yank this out of 'm_regions' since we're about to manipulate the vector
 | ||||
|     //       and we don't want it getting lost.
 | ||||
|     auto executable_region = m_regions.take_last(); | ||||
| 
 | ||||
|     Region* master_tls_region { nullptr }; | ||||
|     size_t master_tls_size = 0; | ||||
|     size_t master_tls_alignment = 0; | ||||
| 
 | ||||
|  | @ -412,7 +414,7 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir | |||
|     { | ||||
|         // Okay, here comes the sleight of hand, pay close attention..
 | ||||
|         auto old_regions = move(m_regions); | ||||
|         m_regions.append(*region); | ||||
|         m_regions.append(move(executable_region)); | ||||
|         loader = make<ELFLoader>(region->vaddr().as_ptr()); | ||||
|         loader->map_section_hook = [&](VirtualAddress vaddr, size_t size, size_t alignment, size_t offset_in_image, bool is_readable, bool is_writable, bool is_executable, const String& name) { | ||||
|             ASSERT(size); | ||||
|  | @ -451,6 +453,7 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir | |||
|             // FIXME: RAII this somehow instead.
 | ||||
|             ASSERT(¤t->process() == this); | ||||
|             MM.enter_process_paging_scope(*this); | ||||
|             executable_region = m_regions.take_first(); | ||||
|             m_regions = move(old_regions); | ||||
|             kprintf("do_exec: Failure loading %s\n", path.characters()); | ||||
|             return -ENOEXEC; | ||||
|  | @ -464,7 +467,7 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir | |||
|     m_executable = description->custody(); | ||||
| 
 | ||||
|     // Copy of the master TLS region that we will clone for new threads
 | ||||
|     m_master_tls_region = master_tls_region.ptr(); | ||||
|     m_master_tls_region = master_tls_region; | ||||
| 
 | ||||
|     if (metadata.is_setuid()) | ||||
|         m_euid = metadata.uid; | ||||
|  | @ -823,7 +826,7 @@ void create_signal_trampolines() | |||
|     InterruptDisabler disabler; | ||||
| 
 | ||||
|     // NOTE: We leak this region.
 | ||||
|     auto* trampoline_region = MM.allocate_user_accessible_kernel_region(PAGE_SIZE, "Signal trampolines").leak_ref(); | ||||
|     auto* trampoline_region = MM.allocate_user_accessible_kernel_region(PAGE_SIZE, "Signal trampolines").leak_ptr(); | ||||
|     g_return_to_ring3_from_signal_trampoline = trampoline_region->vaddr(); | ||||
| 
 | ||||
|     u8* trampoline = (u8*)asm_signal_trampoline; | ||||
|  |  | |||
|  | @ -1,8 +1,8 @@ | |||
| #pragma once | ||||
| 
 | ||||
| #include <AK/String.h> | ||||
| #include <AK/InlineLinkedList.h> | ||||
| #include <AK/NonnullRefPtrVector.h> | ||||
| #include <AK/String.h> | ||||
| #include <AK/Types.h> | ||||
| #include <AK/Vector.h> | ||||
| #include <AK/WeakPtr.h> | ||||
|  | @ -234,7 +234,7 @@ public: | |||
|     void set_tty(TTY* tty) { m_tty = tty; } | ||||
| 
 | ||||
|     size_t region_count() const { return m_regions.size(); } | ||||
|     const NonnullRefPtrVector<Region>& regions() const { return m_regions; } | ||||
|     const NonnullOwnPtrVector<Region>& regions() const { return m_regions; } | ||||
|     void dump_regions(); | ||||
| 
 | ||||
|     ProcessTracer* tracer() { return m_tracer.ptr(); } | ||||
|  | @ -356,7 +356,7 @@ private: | |||
|     Region* region_from_range(const Range&); | ||||
|     Region* region_containing(const Range&); | ||||
| 
 | ||||
|     NonnullRefPtrVector<Region> m_regions; | ||||
|     NonnullOwnPtrVector<Region> m_regions; | ||||
| 
 | ||||
|     pid_t m_ppid { 0 }; | ||||
|     mode_t m_umask { 022 }; | ||||
|  | @ -372,7 +372,7 @@ private: | |||
|     RefPtr<ProcessTracer> m_tracer; | ||||
|     OwnPtr<ELFLoader> m_elf_loader; | ||||
| 
 | ||||
|     RefPtr<Region> m_master_tls_region; | ||||
|     Region* m_master_tls_region { nullptr }; | ||||
|     size_t m_master_tls_size { 0 }; | ||||
|     size_t m_master_tls_alignment { 0 }; | ||||
| 
 | ||||
|  |  | |||
|  | @ -327,8 +327,8 @@ private: | |||
|     u32 m_signal_mask { 0 }; | ||||
|     u32 m_kernel_stack_base { 0 }; | ||||
|     u32 m_kernel_stack_top { 0 }; | ||||
|     RefPtr<Region> m_userspace_stack_region; | ||||
|     RefPtr<Region> m_kernel_stack_region; | ||||
|     Region* m_userspace_stack_region { nullptr }; | ||||
|     OwnPtr<Region> m_kernel_stack_region; | ||||
|     VirtualAddress m_thread_specific_data; | ||||
|     SignalActionData m_signal_action_data[32]; | ||||
|     Blocker* m_blocker { nullptr }; | ||||
|  |  | |||
|  | @ -444,13 +444,13 @@ PageFaultResponse MemoryManager::handle_page_fault(const PageFault& fault) | |||
|     return PageFaultResponse::ShouldCrash; | ||||
| } | ||||
| 
 | ||||
| RefPtr<Region> MemoryManager::allocate_kernel_region(size_t size, const StringView& name, bool user_accessible, bool should_commit) | ||||
| OwnPtr<Region> MemoryManager::allocate_kernel_region(size_t size, const StringView& name, bool user_accessible, bool should_commit) | ||||
| { | ||||
|     InterruptDisabler disabler; | ||||
|     ASSERT(!(size % PAGE_SIZE)); | ||||
|     auto range = kernel_page_directory().range_allocator().allocate_anywhere(size); | ||||
|     ASSERT(range.is_valid()); | ||||
|     RefPtr<Region> region; | ||||
|     OwnPtr<Region> region; | ||||
|     if (user_accessible) | ||||
|         region = Region::create_user_accessible(range, name, PROT_READ | PROT_WRITE | PROT_EXEC, false); | ||||
|     else | ||||
|  | @ -462,7 +462,7 @@ RefPtr<Region> MemoryManager::allocate_kernel_region(size_t size, const StringVi | |||
|     return region; | ||||
| } | ||||
| 
 | ||||
| RefPtr<Region> MemoryManager::allocate_user_accessible_kernel_region(size_t size, const StringView& name) | ||||
| OwnPtr<Region> MemoryManager::allocate_user_accessible_kernel_region(size_t size, const StringView& name) | ||||
| { | ||||
|     return allocate_kernel_region(size, name, true); | ||||
| } | ||||
|  |  | |||
|  | @ -71,8 +71,8 @@ public: | |||
| 
 | ||||
|     void map_for_kernel(VirtualAddress, PhysicalAddress); | ||||
| 
 | ||||
|     RefPtr<Region> allocate_kernel_region(size_t, const StringView& name, bool user_accessible = false, bool should_commit = true); | ||||
|     RefPtr<Region> allocate_user_accessible_kernel_region(size_t, const StringView& name); | ||||
|     OwnPtr<Region> allocate_kernel_region(size_t, const StringView& name, bool user_accessible = false, bool should_commit = true); | ||||
|     OwnPtr<Region> allocate_user_accessible_kernel_region(size_t, const StringView& name); | ||||
|     void map_region_at_address(PageDirectory&, Region&, VirtualAddress); | ||||
| 
 | ||||
|     unsigned user_physical_pages() const { return m_user_physical_pages; } | ||||
|  |  | |||
|  | @ -50,7 +50,7 @@ Region::~Region() | |||
|     MM.unregister_region(*this); | ||||
| } | ||||
| 
 | ||||
| NonnullRefPtr<Region> Region::clone() | ||||
| NonnullOwnPtr<Region> Region::clone() | ||||
| { | ||||
|     ASSERT(current); | ||||
| 
 | ||||
|  | @ -123,30 +123,30 @@ size_t Region::amount_shared() const | |||
|     return bytes; | ||||
| } | ||||
| 
 | ||||
| NonnullRefPtr<Region> Region::create_user_accessible(const Range& range, const StringView& name, u8 access, bool cow) | ||||
| NonnullOwnPtr<Region> Region::create_user_accessible(const Range& range, const StringView& name, u8 access, bool cow) | ||||
| { | ||||
|     auto region = adopt(*new Region(range, name, access, cow)); | ||||
|     auto region = make<Region>(range, name, access, cow); | ||||
|     region->m_user_accessible = true; | ||||
|     return region; | ||||
| } | ||||
| 
 | ||||
| NonnullRefPtr<Region> Region::create_user_accessible(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const StringView& name, u8 access, bool cow) | ||||
| NonnullOwnPtr<Region> Region::create_user_accessible(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const StringView& name, u8 access, bool cow) | ||||
| { | ||||
|     auto region = adopt(*new Region(range, move(vmobject), offset_in_vmobject, name, access, cow)); | ||||
|     auto region = make<Region>(range, move(vmobject), offset_in_vmobject, name, access, cow); | ||||
|     region->m_user_accessible = true; | ||||
|     return region; | ||||
| } | ||||
| 
 | ||||
| NonnullRefPtr<Region> Region::create_user_accessible(const Range& range, NonnullRefPtr<Inode> inode, const StringView& name, u8 access, bool cow) | ||||
| NonnullOwnPtr<Region> Region::create_user_accessible(const Range& range, NonnullRefPtr<Inode> inode, const StringView& name, u8 access, bool cow) | ||||
| { | ||||
|     auto region = adopt(*new Region(range, move(inode), name, access, cow)); | ||||
|     auto region = make<Region>(range, move(inode), name, access, cow); | ||||
|     region->m_user_accessible = true; | ||||
|     return region; | ||||
| } | ||||
| 
 | ||||
| NonnullRefPtr<Region> Region::create_kernel_only(const Range& range, const StringView& name, u8 access, bool cow) | ||||
| NonnullOwnPtr<Region> Region::create_kernel_only(const Range& range, const StringView& name, u8 access, bool cow) | ||||
| { | ||||
|     auto region = adopt(*new Region(range, name, access, cow)); | ||||
|     auto region = make<Region>(range, name, access, cow); | ||||
|     region->m_user_accessible = false; | ||||
|     return region; | ||||
| } | ||||
|  |  | |||
|  | @ -10,8 +10,7 @@ | |||
| class Inode; | ||||
| class VMObject; | ||||
| 
 | ||||
| class Region final : public RefCounted<Region> | ||||
|     , public InlineLinkedListNode<Region> { | ||||
| class Region final : public InlineLinkedListNode<Region> { | ||||
|     friend class MemoryManager; | ||||
| 
 | ||||
|     MAKE_SLAB_ALLOCATED(Region) | ||||
|  | @ -22,10 +21,10 @@ public: | |||
|         Execute = 4, | ||||
|     }; | ||||
| 
 | ||||
|     static NonnullRefPtr<Region> create_user_accessible(const Range&, const StringView& name, u8 access, bool cow = false); | ||||
|     static NonnullRefPtr<Region> create_user_accessible(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const StringView& name, u8 access, bool cow = false); | ||||
|     static NonnullRefPtr<Region> create_user_accessible(const Range&, NonnullRefPtr<Inode>, const StringView& name, u8 access, bool cow = false); | ||||
|     static NonnullRefPtr<Region> create_kernel_only(const Range&, const StringView& name, u8 access, bool cow = false); | ||||
|     static NonnullOwnPtr<Region> create_user_accessible(const Range&, const StringView& name, u8 access, bool cow = false); | ||||
|     static NonnullOwnPtr<Region> create_user_accessible(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const StringView& name, u8 access, bool cow = false); | ||||
|     static NonnullOwnPtr<Region> create_user_accessible(const Range&, NonnullRefPtr<Inode>, const StringView& name, u8 access, bool cow = false); | ||||
|     static NonnullOwnPtr<Region> create_kernel_only(const Range&, const StringView& name, u8 access, bool cow = false); | ||||
| 
 | ||||
|     ~Region(); | ||||
| 
 | ||||
|  | @ -48,7 +47,7 @@ public: | |||
| 
 | ||||
|     bool is_user_accessible() const { return m_user_accessible; } | ||||
| 
 | ||||
|     NonnullRefPtr<Region> clone(); | ||||
|     NonnullOwnPtr<Region> clone(); | ||||
| 
 | ||||
|     bool contains(VirtualAddress vaddr) const | ||||
|     { | ||||
|  | @ -114,11 +113,12 @@ public: | |||
|     Region* m_next { nullptr }; | ||||
|     Region* m_prev { nullptr }; | ||||
| 
 | ||||
| private: | ||||
|     // NOTE: These are public so we can make<> them.
 | ||||
|     Region(const Range&, const String&, u8 access, bool cow = false); | ||||
|     Region(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmo, const String&, u8 access, bool cow = false); | ||||
|     Region(const Range&, RefPtr<Inode>&&, const String&, u8 access, bool cow = false); | ||||
| 
 | ||||
| private: | ||||
|     RefPtr<PageDirectory> m_page_directory; | ||||
|     Range m_range; | ||||
|     size_t m_offset_in_vmo { 0 }; | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling