Parcourir la source

Kernel: Pass "shared" flag to Region constructor

Before this change, we would sometimes map a region into the address
space with !is_shared(), and then moments later call set_shared(true).

I found this very confusing while debugging, so this patch makes us pass
the initial shared flag to the Region constructor, ensuring that it's in
the correct state by the time we first map the region.
Andreas Kling il y a 4 ans
Parent
commit
5dae85afe7

+ 2 - 1
Kernel/Devices/BXVGADevice.cpp

@@ -191,7 +191,8 @@ KResultOr<Region*> BXVGADevice::mmap(Process& process, FileDescription&, Virtual
         vmobject.release_nonnull(),
         0,
         "BXVGA Framebuffer",
-        prot);
+        prot,
+        shared);
     if (!region)
         return KResult(-ENOMEM);
     dbg() << "BXVGADevice: mmap with size " << region->size() << " at " << region->vaddr();

+ 2 - 1
Kernel/Devices/MBVGADevice.cpp

@@ -67,7 +67,8 @@ KResultOr<Region*> MBVGADevice::mmap(Process& process, FileDescription&, Virtual
         vmobject.release_nonnull(),
         0,
         "MBVGA Framebuffer",
-        prot);
+        prot,
+        shared);
     if (!region)
         return KResult(-ENOMEM);
     dbg() << "MBVGADevice: mmap with size " << region->size() << " at " << region->vaddr();

+ 1 - 1
Kernel/FileSystem/InodeFile.cpp

@@ -79,7 +79,7 @@ KResultOr<Region*> InodeFile::mmap(Process& process, FileDescription& descriptio
         vmobject = PrivateInodeVMObject::create_with_inode(inode());
     if (!vmobject)
         return KResult(-ENOMEM);
-    auto* region = process.allocate_region_with_vmobject(preferred_vaddr, size, *vmobject, offset, description.absolute_path(), prot);
+    auto* region = process.allocate_region_with_vmobject(preferred_vaddr, size, *vmobject, offset, description.absolute_path(), prot, shared);
     if (!region)
         return KResult(-ENOMEM);
     return region;

+ 4 - 4
Kernel/Process.cpp

@@ -163,7 +163,7 @@ Region* Process::allocate_region(VirtualAddress vaddr, size_t size, const String
     return allocate_region(range, name, prot, strategy);
 }
 
-Region* Process::allocate_region_with_vmobject(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, int prot)
+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());
     size_t end_in_vmobject = offset_in_vmobject + range.size();
@@ -180,18 +180,18 @@ Region* Process::allocate_region_with_vmobject(const Range& range, NonnullRefPtr
         return nullptr;
     }
     offset_in_vmobject &= PAGE_MASK;
-    auto& region = add_region(Region::create_user_accessible(this, range, move(vmobject), offset_in_vmobject, name, prot_to_region_access_flags(prot)));
+    auto& region = add_region(Region::create_user_accessible(this, range, move(vmobject), offset_in_vmobject, name, prot_to_region_access_flags(prot), true, shared));
     if (!region.map(page_directory()))
         return nullptr;
     return &region;
 }
 
-Region* Process::allocate_region_with_vmobject(VirtualAddress vaddr, size_t size, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, int prot)
+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 nullptr;
-    return allocate_region_with_vmobject(range, move(vmobject), offset_in_vmobject, name, prot);
+    return allocate_region_with_vmobject(range, move(vmobject), offset_in_vmobject, name, prot, shared);
 }
 
 bool Process::deallocate_region(Region& region)

+ 2 - 2
Kernel/Process.h

@@ -437,9 +437,9 @@ public:
         return m_euid == 0;
     }
 
-    Region* allocate_region_with_vmobject(VirtualAddress, size_t, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot);
+    Region* allocate_region_with_vmobject(VirtualAddress, size_t, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot, bool shared);
     Region* allocate_region(VirtualAddress, size_t, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve);
-    Region* allocate_region_with_vmobject(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot);
+    Region* allocate_region_with_vmobject(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot, bool shared);
     Region* allocate_region(const Range&, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve);
     bool deallocate_region(Region& region);
 

+ 1 - 2
Kernel/SharedBuffer.cpp

@@ -89,11 +89,10 @@ void* SharedBuffer::ref_for_process_and_get_address(Process& process)
     for (auto& ref : m_refs) {
         if (ref.pid == process.pid()) {
             if (!ref.region) {
-                auto* region = process.allocate_region_with_vmobject(VirtualAddress(), size(), m_vmobject, 0, "SharedBuffer", PROT_READ | (m_writable ? PROT_WRITE : 0));
+                auto* region = process.allocate_region_with_vmobject(VirtualAddress(), size(), m_vmobject, 0, "SharedBuffer", PROT_READ | (m_writable ? PROT_WRITE : 0), true);
                 if (!region)
                     return (void*)-ENOMEM;
                 ref.region = region;
-                region->set_shared(true);
             }
             ref.count++;
             m_total_refs++;

+ 1 - 2
Kernel/Syscalls/execve.cpp

@@ -238,12 +238,11 @@ KResultOr<Process::LoadResult> Process::load_elf_object(FileDescription& object_
             prot |= PROT_WRITE;
         if (program_header.is_executable())
             prot |= PROT_EXEC;
-        auto* region = allocate_region_with_vmobject(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), *vmobject, program_header.offset(), elf_name, prot);
+        auto* region = allocate_region_with_vmobject(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), *vmobject, program_header.offset(), elf_name, prot, true);
         if (!region) {
             ph_load_result = KResult(-ENOMEM);
             return IterationDecision::Break;
         }
-        region->set_shared(true);
         if (program_header.offset() == 0)
             load_base_address = (FlatPtr)region->vaddr().as_ptr();
         return IterationDecision::Continue;

+ 1 - 1
Kernel/Syscalls/mmap.cpp

@@ -438,7 +438,7 @@ void* Process::sys$mremap(Userspace<const Syscall::SC_mremap_params*> user_param
         deallocate_region(*old_region);
 
         auto new_vmobject = PrivateInodeVMObject::create_with_inode(inode);
-        auto* new_region = allocate_region_with_vmobject(range.base(), range.size(), new_vmobject, 0, old_name, old_prot);
+        auto* new_region = allocate_region_with_vmobject(range.base(), range.size(), new_vmobject, 0, old_name, old_prot, false);
         new_region->set_mmap(true);
 
         if (!new_region)

+ 5 - 4
Kernel/VM/Region.cpp

@@ -40,13 +40,14 @@
 
 namespace Kernel {
 
-Region::Region(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, u8 access, bool cacheable, bool kernel)
+Region::Region(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, u8 access, bool cacheable, bool kernel, bool shared)
     : PurgeablePageRanges(vmobject)
     , m_range(range)
     , m_offset_in_vmobject(offset_in_vmobject)
     , m_vmobject(move(vmobject))
     , m_name(name)
     , m_access(access)
+    , m_shared(shared)
     , m_cacheable(cacheable)
     , m_kernel(kernel)
 {
@@ -236,9 +237,9 @@ size_t Region::amount_shared() const
     return bytes;
 }
 
-NonnullOwnPtr<Region> Region::create_user_accessible(Process* owner, const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const StringView& name, u8 access, bool cacheable)
+NonnullOwnPtr<Region> Region::create_user_accessible(Process* owner, const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const StringView& name, u8 access, bool cacheable, bool shared)
 {
-    auto region = make<Region>(range, move(vmobject), offset_in_vmobject, name, access, cacheable, false);
+    auto region = make<Region>(range, move(vmobject), offset_in_vmobject, name, access, cacheable, false, shared);
     if (owner)
         region->m_owner = owner->make_weak_ptr();
     region->m_user_accessible = true;
@@ -247,7 +248,7 @@ NonnullOwnPtr<Region> Region::create_user_accessible(Process* owner, const Range
 
 NonnullOwnPtr<Region> Region::create_kernel_only(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const StringView& name, u8 access, bool cacheable)
 {
-    auto region = make<Region>(range, move(vmobject), offset_in_vmobject, name, access, cacheable, true);
+    auto region = make<Region>(range, move(vmobject), offset_in_vmobject, name, access, cacheable, true, false);
     region->m_user_accessible = false;
     return region;
 }

+ 2 - 2
Kernel/VM/Region.h

@@ -61,7 +61,7 @@ public:
         ZeroedOnFork,
     };
 
-    static NonnullOwnPtr<Region> create_user_accessible(Process*, const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const StringView& name, u8 access, bool cacheable = true);
+    static NonnullOwnPtr<Region> create_user_accessible(Process*, const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const StringView& name, u8 access, bool cacheable = true, bool shared = false);
     static NonnullOwnPtr<Region> create_kernel_only(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const StringView& name, u8 access, bool cacheable = true);
 
     ~Region();
@@ -182,7 +182,7 @@ public:
     Region* m_prev { nullptr };
 
     // NOTE: These are public so we can make<> them.
-    Region(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String&, u8 access, bool cacheable, bool kernel);
+    Region(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String&, u8 access, bool cacheable, bool kernel, bool shared);
 
     void set_inherit_mode(InheritMode inherit_mode) { m_inherit_mode = inherit_mode; }