1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 17:07:34 +00:00

Kernel: Make Memory::Region::map() return KResult

..and use TRY() at the call sites to propagate errors. :^)
This commit is contained in:
Andreas Kling 2021-09-06 12:52:23 +02:00
parent 7981422500
commit e3a716ceff
6 changed files with 17 additions and 25 deletions

View file

@ -72,8 +72,7 @@ KResult AddressSpace::unmap_mmap_range(VirtualAddress addr, size_t size)
for (auto* new_region : new_regions) { for (auto* new_region : new_regions) {
// TODO: Ideally we should do this in a way that can be rolled back on failure, as failing here // TODO: Ideally we should do this in a way that can be rolled back on failure, as failing here
// leaves the caller in an undefined state. // leaves the caller in an undefined state.
if (!new_region->map(page_directory())) TRY(new_region->map(page_directory()));
return ENOMEM;
} }
PerformanceManager::add_unmap_perf_event(Process::current(), range_to_unmap); PerformanceManager::add_unmap_perf_event(Process::current(), range_to_unmap);
@ -122,8 +121,7 @@ KResult AddressSpace::unmap_mmap_range(VirtualAddress addr, size_t size)
for (auto* new_region : new_regions) { for (auto* new_region : new_regions) {
// TODO: Ideally we should do this in a way that can be rolled back on failure, as failing here // TODO: Ideally we should do this in a way that can be rolled back on failure, as failing here
// leaves the caller in an undefined state. // leaves the caller in an undefined state.
if (!new_region->map(page_directory())) TRY(new_region->map(page_directory()));
return ENOMEM;
} }
PerformanceManager::add_unmap_perf_event(Process::current(), range_to_unmap); PerformanceManager::add_unmap_perf_event(Process::current(), range_to_unmap);
@ -161,8 +159,7 @@ KResultOr<Region*> AddressSpace::allocate_region(VirtualRange const& range, Stri
VERIFY(range.is_valid()); VERIFY(range.is_valid());
auto vmobject = TRY(AnonymousVMObject::try_create_with_size(range.size(), strategy)); auto vmobject = TRY(AnonymousVMObject::try_create_with_size(range.size(), strategy));
auto region = TRY(Region::try_create_user_accessible(range, move(vmobject), 0, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, false)); auto region = TRY(Region::try_create_user_accessible(range, move(vmobject), 0, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, false));
if (!region->map(page_directory())) TRY(region->map(page_directory()));
return ENOMEM;
return add_region(move(region)); return add_region(move(region));
} }
@ -185,8 +182,7 @@ KResultOr<Region*> AddressSpace::allocate_region_with_vmobject(VirtualRange cons
offset_in_vmobject &= PAGE_MASK; offset_in_vmobject &= PAGE_MASK;
auto region = TRY(Region::try_create_user_accessible(range, move(vmobject), offset_in_vmobject, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, shared)); auto region = TRY(Region::try_create_user_accessible(range, move(vmobject), offset_in_vmobject, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, shared));
auto* added_region = TRY(add_region(move(region))); auto* added_region = TRY(add_region(move(region)));
if (!added_region->map(page_directory())) TRY(added_region->map(page_directory()));
return ENOMEM;
return added_region; return added_region;
} }

View file

@ -730,8 +730,7 @@ KResultOr<NonnullOwnPtr<Region>> MemoryManager::allocate_kernel_region(PhysicalA
KResultOr<NonnullOwnPtr<Region>> MemoryManager::allocate_kernel_region_with_vmobject(VirtualRange const& range, VMObject& vmobject, StringView name, Region::Access access, Region::Cacheable cacheable) KResultOr<NonnullOwnPtr<Region>> MemoryManager::allocate_kernel_region_with_vmobject(VirtualRange const& range, VMObject& vmobject, StringView name, Region::Access access, Region::Cacheable cacheable)
{ {
auto region = TRY(Region::try_create_kernel_only(range, vmobject, 0, KString::try_create(name), access, cacheable)); auto region = TRY(Region::try_create_kernel_only(range, vmobject, 0, KString::try_create(name), access, cacheable));
if (!region->map(kernel_page_directory())) TRY(region->map(kernel_page_directory()));
return ENOMEM;
return region; return region;
} }

View file

@ -243,7 +243,7 @@ void Region::set_page_directory(PageDirectory& page_directory)
m_page_directory = page_directory; m_page_directory = page_directory;
} }
bool Region::map(PageDirectory& page_directory, ShouldFlushTLB should_flush_tlb) KResult Region::map(PageDirectory& page_directory, ShouldFlushTLB should_flush_tlb)
{ {
SpinlockLocker page_lock(page_directory.get_lock()); SpinlockLocker page_lock(page_directory.get_lock());
SpinlockLocker lock(s_mm_lock); SpinlockLocker lock(s_mm_lock);
@ -263,16 +263,18 @@ bool Region::map(PageDirectory& page_directory, ShouldFlushTLB should_flush_tlb)
if (page_index > 0) { if (page_index > 0) {
if (should_flush_tlb == ShouldFlushTLB::Yes) if (should_flush_tlb == ShouldFlushTLB::Yes)
MM.flush_tlb(m_page_directory, vaddr(), page_index); MM.flush_tlb(m_page_directory, vaddr(), page_index);
return page_index == page_count(); if (page_index == page_count())
return KSuccess;
} }
return false; return ENOMEM;
} }
void Region::remap() void Region::remap()
{ {
VERIFY(m_page_directory); VERIFY(m_page_directory);
auto result = map(*m_page_directory); auto result = map(*m_page_directory);
VERIFY(result); if (result.is_error())
TODO();
} }
PageFaultResponse Region::handle_fault(PageFault const& fault) PageFaultResponse Region::handle_fault(PageFault const& fault)

View file

@ -170,7 +170,7 @@ public:
void set_executable(bool b) { set_access_bit(Access::Execute, b); } void set_executable(bool b) { set_access_bit(Access::Execute, b); }
void set_page_directory(PageDirectory&); void set_page_directory(PageDirectory&);
[[nodiscard]] bool map(PageDirectory&, ShouldFlushTLB = ShouldFlushTLB::Yes); KResult map(PageDirectory&, ShouldFlushTLB = ShouldFlushTLB::Yes);
enum class ShouldDeallocateVirtualRange { enum class ShouldDeallocateVirtualRange {
No, No,
Yes, Yes,

View file

@ -95,8 +95,7 @@ KResultOr<FlatPtr> Process::sys$fork(RegisterState& regs)
dbgln_if(FORK_DEBUG, "fork: cloning Region({}) '{}' @ {}", region, region->name(), region->vaddr()); dbgln_if(FORK_DEBUG, "fork: cloning Region({}) '{}' @ {}", region, region->name(), region->vaddr());
auto region_clone = TRY(region->try_clone()); auto region_clone = TRY(region->try_clone());
auto* child_region = TRY(child->address_space().add_region(move(region_clone))); auto* child_region = TRY(child->address_space().add_region(move(region_clone)));
if (!child_region->map(child->address_space().page_directory(), Memory::ShouldFlushTLB::No)) TRY(child_region->map(child->address_space().page_directory(), Memory::ShouldFlushTLB::No));
return ENOMEM;
if (region == m_master_tls_region.unsafe_ptr()) if (region == m_master_tls_region.unsafe_ptr())
child->m_master_tls_region = child_region; child->m_master_tls_region = child_region;

View file

@ -338,11 +338,9 @@ KResultOr<FlatPtr> Process::sys$mprotect(Userspace<void*> addr, size_t size, int
// Map the new regions using our page directory (they were just allocated and don't have one). // Map the new regions using our page directory (they were just allocated and don't have one).
for (auto* adjacent_region : adjacent_regions) { for (auto* adjacent_region : adjacent_regions) {
if (!adjacent_region->map(address_space().page_directory())) TRY(adjacent_region->map(address_space().page_directory()));
return ENOMEM;
} }
if (!new_region->map(address_space().page_directory())) TRY(new_region->map(address_space().page_directory()));
return ENOMEM;
return 0; return 0;
} }
@ -401,11 +399,9 @@ KResultOr<FlatPtr> Process::sys$mprotect(Userspace<void*> addr, size_t size, int
// Map the new region using our page directory (they were just allocated and don't have one) if any. // Map the new region using our page directory (they were just allocated and don't have one) if any.
if (adjacent_regions.size()) if (adjacent_regions.size())
if (!adjacent_regions[0]->map(address_space().page_directory())) TRY(adjacent_regions[0]->map(address_space().page_directory()));
return ENOMEM;
if (!new_region->map(address_space().page_directory())) TRY(new_region->map(address_space().page_directory()));
return ENOMEM;
} }
return 0; return 0;