mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 15:42:44 +00:00 
			
		
		
		
	Kernel: Remove allocate_region() functions that don't take a Range
Let's force callers to provide a VM range when allocating a region. This makes ENOMEM error handling more visible and removes implicit VM allocation which felt a bit magical.
This commit is contained in:
		
							parent
							
								
									d697d33fa6
								
							
						
					
					
						commit
						1e25d2b734
					
				
					 5 changed files with 41 additions and 25 deletions
				
			
		|  | @ -150,14 +150,6 @@ KResultOr<Region*> Process::allocate_region(const Range& range, const String& na | ||||||
|     return &add_region(move(region)); |     return &add_region(move(region)); | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResultOr<Region*> Process::allocate_region(VirtualAddress vaddr, size_t size, const String& name, int prot, AllocationStrategy strategy) |  | ||||||
| { |  | ||||||
|     auto range = allocate_range(vaddr, size); |  | ||||||
|     if (!range.is_valid()) |  | ||||||
|         return ENOMEM; |  | ||||||
|     return allocate_region(range, name, prot, strategy); |  | ||||||
| } |  | ||||||
| 
 |  | ||||||
| KResultOr<Region*> Process::allocate_region_with_vmobject(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, int prot, bool shared) | KResultOr<Region*> Process::allocate_region_with_vmobject(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, int prot, bool shared) | ||||||
| { | { | ||||||
|     ASSERT(range.is_valid()); |     ASSERT(range.is_valid()); | ||||||
|  | @ -183,14 +175,6 @@ KResultOr<Region*> Process::allocate_region_with_vmobject(const Range& range, No | ||||||
|     return ®ion; |     return ®ion; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| KResultOr<Region*> Process::allocate_region_with_vmobject(VirtualAddress vaddr, size_t size, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, int prot, bool shared) |  | ||||||
| { |  | ||||||
|     auto range = allocate_range(vaddr, size); |  | ||||||
|     if (!range.is_valid()) |  | ||||||
|         return ENOMEM; |  | ||||||
|     return allocate_region_with_vmobject(range, move(vmobject), offset_in_vmobject, name, prot, shared); |  | ||||||
| } |  | ||||||
| 
 |  | ||||||
| bool Process::deallocate_region(Region& region) | bool Process::deallocate_region(Region& region) | ||||||
| { | { | ||||||
|     OwnPtr<Region> region_protector; |     OwnPtr<Region> region_protector; | ||||||
|  |  | ||||||
|  | @ -437,8 +437,6 @@ public: | ||||||
|         return m_euid == 0; |         return m_euid == 0; | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     KResultOr<Region*> allocate_region_with_vmobject(VirtualAddress, size_t, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot, bool shared); |  | ||||||
|     KResultOr<Region*> allocate_region(VirtualAddress, size_t, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve); |  | ||||||
|     KResultOr<Region*> allocate_region_with_vmobject(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot, bool shared); |     KResultOr<Region*> allocate_region_with_vmobject(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot, bool shared); | ||||||
|     KResultOr<Region*> allocate_region(const Range&, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve); |     KResultOr<Region*> allocate_region(const Range&, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve); | ||||||
|     bool deallocate_region(Region& region); |     bool deallocate_region(Region& region); | ||||||
|  |  | ||||||
|  | @ -184,7 +184,13 @@ KResultOr<Process::LoadResult> Process::load_elf_object(FileDescription& object_ | ||||||
|                 return IterationDecision::Break; |                 return IterationDecision::Break; | ||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|             auto region_or_error = allocate_region({}, program_header.size_in_memory(), String::formatted("{} (master-tls)", elf_name), PROT_READ | PROT_WRITE, AllocationStrategy::Reserve); |             auto range = allocate_range({}, program_header.size_in_memory()); | ||||||
|  |             if (!range.is_valid()) { | ||||||
|  |                 ph_load_result = ENOMEM; | ||||||
|  |                 return IterationDecision::Break; | ||||||
|  |             } | ||||||
|  | 
 | ||||||
|  |             auto region_or_error = allocate_region(range, String::formatted("{} (master-tls)", elf_name), PROT_READ | PROT_WRITE, AllocationStrategy::Reserve); | ||||||
|             if (region_or_error.is_error()) { |             if (region_or_error.is_error()) { | ||||||
|                 ph_load_result = region_or_error.error(); |                 ph_load_result = region_or_error.error(); | ||||||
|                 return IterationDecision::Break; |                 return IterationDecision::Break; | ||||||
|  | @ -219,7 +225,12 @@ KResultOr<Process::LoadResult> Process::load_elf_object(FileDescription& object_ | ||||||
|             if (program_header.is_writable()) |             if (program_header.is_writable()) | ||||||
|                 prot |= PROT_WRITE; |                 prot |= PROT_WRITE; | ||||||
|             auto region_name = String::formatted("{} (data-{}{})", elf_name, program_header.is_readable() ? "r" : "", program_header.is_writable() ? "w" : ""); |             auto region_name = String::formatted("{} (data-{}{})", elf_name, program_header.is_readable() ? "r" : "", program_header.is_writable() ? "w" : ""); | ||||||
|             auto region_or_error = allocate_region(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), move(region_name), prot, AllocationStrategy::Reserve); |             auto range = allocate_range(program_header.vaddr().offset(load_offset), program_header.size_in_memory()); | ||||||
|  |             if (!range.is_valid()) { | ||||||
|  |                 ph_load_result = ENOMEM; | ||||||
|  |                 return IterationDecision::Break; | ||||||
|  |             } | ||||||
|  |             auto region_or_error = allocate_region(range, region_name, prot, AllocationStrategy::Reserve); | ||||||
|             if (region_or_error.is_error()) { |             if (region_or_error.is_error()) { | ||||||
|                 ph_load_result = region_or_error.error(); |                 ph_load_result = region_or_error.error(); | ||||||
|                 return IterationDecision::Break; |                 return IterationDecision::Break; | ||||||
|  | @ -251,7 +262,12 @@ KResultOr<Process::LoadResult> Process::load_elf_object(FileDescription& object_ | ||||||
|             prot |= PROT_WRITE; |             prot |= PROT_WRITE; | ||||||
|         if (program_header.is_executable()) |         if (program_header.is_executable()) | ||||||
|             prot |= PROT_EXEC; |             prot |= PROT_EXEC; | ||||||
|         auto region_or_error = allocate_region_with_vmobject(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), *vmobject, program_header.offset(), elf_name, prot, true); |         auto range = allocate_range(program_header.vaddr().offset(load_offset), program_header.size_in_memory()); | ||||||
|  |         if (!range.is_valid()) { | ||||||
|  |             ph_load_result = ENOMEM; | ||||||
|  |             return IterationDecision::Break; | ||||||
|  |         } | ||||||
|  |         auto region_or_error = allocate_region_with_vmobject(range, *vmobject, program_header.offset(), elf_name, prot, true); | ||||||
|         if (region_or_error.is_error()) { |         if (region_or_error.is_error()) { | ||||||
|             ph_load_result = region_or_error.error(); |             ph_load_result = region_or_error.error(); | ||||||
|             return IterationDecision::Break; |             return IterationDecision::Break; | ||||||
|  | @ -271,7 +287,13 @@ KResultOr<Process::LoadResult> Process::load_elf_object(FileDescription& object_ | ||||||
|         return ENOEXEC; |         return ENOEXEC; | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     auto stack_region_or_error = allocate_region(VirtualAddress(), Thread::default_userspace_stack_size, "Stack (Main thread)", PROT_READ | PROT_WRITE, AllocationStrategy::Reserve); |     auto stack_range = allocate_range({}, Thread::default_userspace_stack_size); | ||||||
|  |     if (!stack_range.is_valid()) { | ||||||
|  |         dbgln("do_exec: Failed to allocate VM range for stack"); | ||||||
|  |         return ENOMEM; | ||||||
|  |     } | ||||||
|  | 
 | ||||||
|  |     auto stack_region_or_error = allocate_region(stack_range, "Stack (Main thread)", PROT_READ | PROT_WRITE, AllocationStrategy::Reserve); | ||||||
|     if (stack_region_or_error.is_error()) |     if (stack_region_or_error.is_error()) | ||||||
|         return stack_region_or_error.error(); |         return stack_region_or_error.error(); | ||||||
|     auto& stack_region = *stack_region_or_error.value(); |     auto& stack_region = *stack_region_or_error.value(); | ||||||
|  |  | ||||||
|  | @ -404,10 +404,14 @@ void* Process::sys$mremap(Userspace<const Syscall::SC_mremap_params*> user_param | ||||||
|         auto old_name = old_region->name(); |         auto old_name = old_region->name(); | ||||||
|         auto old_prot = region_access_flags_to_prot(old_region->access()); |         auto old_prot = region_access_flags_to_prot(old_region->access()); | ||||||
|         NonnullRefPtr inode = static_cast<SharedInodeVMObject&>(old_region->vmobject()).inode(); |         NonnullRefPtr inode = static_cast<SharedInodeVMObject&>(old_region->vmobject()).inode(); | ||||||
|  | 
 | ||||||
|  |         // Unmap without deallocating the VM range since we're going to reuse it.
 | ||||||
|  |         old_region->unmap(Region::ShouldDeallocateVirtualMemoryRange::No); | ||||||
|         deallocate_region(*old_region); |         deallocate_region(*old_region); | ||||||
| 
 | 
 | ||||||
|         auto new_vmobject = PrivateInodeVMObject::create_with_inode(inode); |         auto new_vmobject = PrivateInodeVMObject::create_with_inode(inode); | ||||||
|         auto new_region_or_error = allocate_region_with_vmobject(range.base(), range.size(), new_vmobject, 0, old_name, old_prot, false); | 
 | ||||||
|  |         auto new_region_or_error = allocate_region_with_vmobject(range, new_vmobject, 0, old_name, old_prot, false); | ||||||
|         if (new_region_or_error.is_error()) |         if (new_region_or_error.is_error()) | ||||||
|             return (void*)new_region_or_error.error().error(); |             return (void*)new_region_or_error.error().error(); | ||||||
|         auto& new_region = *new_region_or_error.value(); |         auto& new_region = *new_region_or_error.value(); | ||||||
|  | @ -439,7 +443,11 @@ void* Process::sys$allocate_tls(size_t size) | ||||||
|     }); |     }); | ||||||
|     ASSERT(main_thread); |     ASSERT(main_thread); | ||||||
| 
 | 
 | ||||||
|     auto region_or_error = allocate_region({}, size, String(), PROT_READ | PROT_WRITE); |     auto range = allocate_range({}, size); | ||||||
|  |     if (!range.is_valid()) | ||||||
|  |         return (void*)-ENOMEM; | ||||||
|  | 
 | ||||||
|  |     auto region_or_error = allocate_region(range, String(), PROT_READ | PROT_WRITE); | ||||||
|     if (region_or_error.is_error()) |     if (region_or_error.is_error()) | ||||||
|         return (void*)region_or_error.error().error(); |         return (void*)region_or_error.error().error(); | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -1041,7 +1041,11 @@ KResult Thread::make_thread_specific_region(Badge<Process>) | ||||||
|     if (!process().m_master_tls_region) |     if (!process().m_master_tls_region) | ||||||
|         return KSuccess; |         return KSuccess; | ||||||
| 
 | 
 | ||||||
|     auto region_or_error = process().allocate_region({}, thread_specific_region_size(), "Thread-specific", PROT_READ | PROT_WRITE); |     auto range = process().allocate_range({}, thread_specific_region_size()); | ||||||
|  |     if (!range.is_valid()) | ||||||
|  |         return ENOMEM; | ||||||
|  | 
 | ||||||
|  |     auto region_or_error = process().allocate_region(range, "Thread-specific", PROT_READ | PROT_WRITE); | ||||||
|     if (region_or_error.is_error()) |     if (region_or_error.is_error()) | ||||||
|         return region_or_error.error(); |         return region_or_error.error(); | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling