Skip to content

Commit

Permalink
Kernel: Factor address space management out of the Process class
Browse files Browse the repository at this point in the history
This patch adds Space, a class representing a process's address space.

- Each Process has a Space.
- The Space owns the PageDirectory and all Regions in the Process.

This allows us to reorganize sys$execve() so that it constructs and
populates a new Space fully before committing to it.

Previously, we would construct the new address space while still
running in the old one, and encountering an error meant we had to do
tedious and error-prone rollback.

Those problems are now gone, replaced by what's hopefully a set of much
smaller problems and missing cleanups. :^)
  • Loading branch information
awesomekling committed Feb 8, 2021
1 parent b2cba30 commit f1b5def
Show file tree
Hide file tree
Showing 27 changed files with 493 additions and 403 deletions.
1 change: 1 addition & 0 deletions Kernel/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ set(KERNEL_SOURCES
VM/RangeAllocator.cpp
VM/Region.cpp
VM/SharedInodeVMObject.cpp
VM/Space.cpp
VM/VMObject.cpp
WaitQueue.cpp
init.cpp
Expand Down
12 changes: 6 additions & 6 deletions Kernel/CoreDump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ OwnPtr<CoreDump> CoreDump::create(NonnullRefPtr<Process> process, const String&
CoreDump::CoreDump(NonnullRefPtr<Process> process, NonnullRefPtr<FileDescription>&& fd)
: m_process(move(process))
, m_fd(move(fd))
, m_num_program_headers(m_process->m_regions.size() + 1) // +1 for NOTE segment
, m_num_program_headers(m_process->space().region_count() + 1) // +1 for NOTE segment
{
}

Expand Down Expand Up @@ -137,7 +137,7 @@ KResult CoreDump::write_elf_header()
KResult CoreDump::write_program_headers(size_t notes_size)
{
size_t offset = sizeof(Elf32_Ehdr) + m_num_program_headers * sizeof(Elf32_Phdr);
for (auto& region : m_process->m_regions) {
for (auto& region : m_process->space().regions()) {
Elf32_Phdr phdr {};

phdr.p_type = PT_LOAD;
Expand Down Expand Up @@ -178,7 +178,7 @@ KResult CoreDump::write_program_headers(size_t notes_size)

KResult CoreDump::write_regions()
{
for (auto& region : m_process->m_regions) {
for (auto& region : m_process->space().regions()) {
if (region.is_kernel())
continue;

Expand Down Expand Up @@ -258,13 +258,13 @@ ByteBuffer CoreDump::create_notes_threads_data() const
ByteBuffer CoreDump::create_notes_regions_data() const
{
ByteBuffer regions_data;
for (size_t region_index = 0; region_index < m_process->m_regions.size(); ++region_index) {
for (size_t region_index = 0; region_index < m_process->space().region_count(); ++region_index) {

ByteBuffer memory_region_info_buffer;
ELF::Core::MemoryRegionInfo info {};
info.header.type = ELF::Core::NotesEntryHeader::Type::MemoryRegionInfo;

auto& region = m_process->m_regions[region_index];
auto& region = m_process->space().regions()[region_index];
info.region_start = reinterpret_cast<uint32_t>(region.vaddr().as_ptr());
info.region_end = reinterpret_cast<uint32_t>(region.vaddr().as_ptr() + region.size());
info.program_header_index = region_index;
Expand Down Expand Up @@ -316,7 +316,7 @@ ByteBuffer CoreDump::create_notes_segment_data() const

KResult CoreDump::write()
{
ScopedSpinLock lock(m_process->get_lock());
ScopedSpinLock lock(m_process->space().get_lock());
ProcessPagingScope scope(m_process);

ByteBuffer notes_segment = create_notes_segment_data();
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Devices/BXVGADevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ KResultOr<Region*> BXVGADevice::mmap(Process& process, FileDescription&, const R
auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes());
if (!vmobject)
return ENOMEM;
return process.allocate_region_with_vmobject(
return process.space().allocate_region_with_vmobject(
range,
vmobject.release_nonnull(),
0,
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Devices/MBVGADevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ KResultOr<Region*> MBVGADevice::mmap(Process& process, FileDescription&, const R
auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes());
if (!vmobject)
return ENOMEM;
return process.allocate_region_with_vmobject(
return process.space().allocate_region_with_vmobject(
range,
vmobject.release_nonnull(),
0,
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Devices/MemoryDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ KResultOr<Region*> MemoryDevice::mmap(Process& process, FileDescription&, const
if (!vmobject)
return ENOMEM;
dbgln("MemoryDevice: Mapped physical memory at {} for range of {} bytes", viewed_address, range.size());
return process.allocate_region_with_vmobject(
return process.space().allocate_region_with_vmobject(
range,
vmobject.release_nonnull(),
0,
Expand Down
2 changes: 1 addition & 1 deletion Kernel/FileSystem/AnonymousFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ KResultOr<Region*> AnonymousFile::mmap(Process& process, FileDescription&, const
if (range.size() != m_vmobject->size())
return EINVAL;

return process.allocate_region_with_vmobject(range, m_vmobject, offset, {}, prot, shared);
return process.space().allocate_region_with_vmobject(range, m_vmobject, offset, {}, prot, shared);
}

}
2 changes: 1 addition & 1 deletion Kernel/FileSystem/InodeFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ KResultOr<Region*> InodeFile::mmap(Process& process, FileDescription& descriptio
vmobject = PrivateInodeVMObject::create_with_inode(inode());
if (!vmobject)
return ENOMEM;
return process.allocate_region_with_vmobject(range, vmobject.release_nonnull(), offset, description.absolute_path(), prot, shared);
return process.space().allocate_region_with_vmobject(range, vmobject.release_nonnull(), offset, description.absolute_path(), prot, shared);
}

String InodeFile::absolute_path(const FileDescription& description) const
Expand Down
4 changes: 2 additions & 2 deletions Kernel/FileSystem/ProcFS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,8 @@ static bool procfs$pid_vm(InodeIdentifier identifier, KBufferBuilder& builder)
return false;
JsonArraySerializer array { builder };
{
ScopedSpinLock lock(process->get_lock());
for (auto& region : process->regions()) {
ScopedSpinLock lock(process->space().get_lock());
for (auto& region : process->space().regions()) {
if (!region.is_user_accessible() && !Process::current()->is_superuser())
continue;
auto region_object = array.add_object();
Expand Down
1 change: 1 addition & 0 deletions Kernel/Forward.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class Region;
class Scheduler;
class SchedulerPerProcessorData;
class Socket;
class Space;
template<typename BaseType>
class SpinLock;
class RecursiveSpinLock;
Expand Down
4 changes: 2 additions & 2 deletions Kernel/PerformanceEventBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,15 @@ bool PerformanceEventBuffer::to_json(KBufferBuilder& builder, ProcessID pid, con
{
auto process = Process::from_pid(pid);
ASSERT(process);
ScopedSpinLock locker(process->get_lock());
ScopedSpinLock locker(process->space().get_lock());

JsonObjectSerializer object(builder);
object.add("pid", pid.value());
object.add("executable", executable_path);

{
auto region_array = object.add_array("regions");
for (const auto& region : process->regions()) {
for (const auto& region : process->space().regions()) {
auto region_object = region_array.add_object();
region_object.add("base", region.vaddr().get());
region_object.add("size", region.size());
Expand Down
Loading

0 comments on commit f1b5def

Please sign in to comment.