Skip to content

Commit

Permalink
Kernel: Remove some unnecessary indirection in InodeFile::mmap()
Browse files Browse the repository at this point in the history
InodeFile now directly calls Process::allocate_region_with_vmobject()
instead of taking an awkward detour via a special Region constructor.
  • Loading branch information
awesomekling committed Feb 28, 2020
1 parent 651417a commit aa1e209
Show file tree
Hide file tree
Showing 5 changed files with 3 additions and 33 deletions.
3 changes: 2 additions & 1 deletion Kernel/FileSystem/InodeFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <Kernel/FileSystem/InodeFile.h>
#include <Kernel/FileSystem/VirtualFileSystem.h>
#include <Kernel/Process.h>
#include <Kernel/VM/SharedInodeVMObject.h>

namespace Kernel {

Expand Down Expand Up @@ -63,7 +64,7 @@ KResultOr<Region*> InodeFile::mmap(Process& process, FileDescription& descriptio
{
ASSERT(offset == 0);
// FIXME: If PROT_EXEC, check that the underlying file system isn't mounted noexec.
auto* region = process.allocate_file_backed_region(preferred_vaddr, size, inode(), description.absolute_path(), prot);
auto* region = process.allocate_region_with_vmobject(preferred_vaddr, size, SharedInodeVMObject::create_with_inode(inode()), offset, description.absolute_path(), prot);
if (!region)
return KResult(-ENOMEM);
return region;
Expand Down
13 changes: 1 addition & 12 deletions Kernel/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,9 @@
#include <Kernel/TTY/MasterPTY.h>
#include <Kernel/TTY/TTY.h>
#include <Kernel/Thread.h>
#include <Kernel/VM/SharedInodeVMObject.h>
#include <Kernel/VM/PageDirectory.h>
#include <Kernel/VM/PurgeableVMObject.h>
#include <Kernel/VM/SharedInodeVMObject.h>
#include <LibBareMetal/IO.h>
#include <LibBareMetal/Output/Console.h>
#include <LibBareMetal/StdLib.h>
Expand Down Expand Up @@ -205,16 +205,6 @@ Region* Process::allocate_region(VirtualAddress vaddr, size_t size, const String
return allocate_region(range, name, prot, commit);
}

Region* Process::allocate_file_backed_region(VirtualAddress vaddr, size_t size, NonnullRefPtr<Inode> inode, const String& name, int prot)
{
auto range = allocate_range(vaddr, size);
if (!range.is_valid())
return nullptr;
auto& region = add_region(Region::create_user_accessible(range, inode, name, prot_to_region_access_flags(prot)));
region.map(page_directory());
return &region;
}

Region* Process::allocate_region_with_vmobject(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, int prot, bool user_accessible)
{
ASSERT(range.is_valid());
Expand All @@ -241,7 +231,6 @@ Region* Process::allocate_region_with_vmobject(const Range& range, NonnullRefPtr
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, bool user_accessible)
{
auto range = allocate_range(vaddr, size);
Expand Down
1 change: 0 additions & 1 deletion Kernel/Process.h
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,6 @@ class Process : public InlineLinkedListNode<Process> {
bool is_superuser() const { return m_euid == 0; }

Region* allocate_region_with_vmobject(VirtualAddress, size_t, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot, bool user_accessible = true);
Region* allocate_file_backed_region(VirtualAddress, size_t, NonnullRefPtr<Inode>, const String& name, int prot);
Region* allocate_region(VirtualAddress, size_t, const String& name, int prot = PROT_READ | PROT_WRITE, bool commit = true);
Region* allocate_region_with_vmobject(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String& name, int prot, bool user_accessible = true);
Region* allocate_region(const Range&, const String& name, int prot = PROT_READ | PROT_WRITE, bool commit = true);
Expand Down
17 changes: 0 additions & 17 deletions Kernel/VM/Region.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ Region::Region(const Range& range, const String& name, u8 access, bool cacheable
MM.register_region(*this);
}

Region::Region(const Range& range, NonnullRefPtr<Inode> inode, const String& name, u8 access, bool cacheable)
: m_range(range)
, m_vmobject(SharedInodeVMObject::create_with_inode(*inode))
, m_name(name)
, m_access(access)
, m_cacheable(cacheable)
{
MM.register_region(*this);
}

Region::Region(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, const String& name, u8 access, bool cacheable)
: m_range(range)
, m_offset_in_vmobject(offset_in_vmobject)
Expand Down Expand Up @@ -206,13 +196,6 @@ NonnullOwnPtr<Region> Region::create_user_accessible(const Range& range, Nonnull
return region;
}

NonnullOwnPtr<Region> Region::create_user_accessible(const Range& range, NonnullRefPtr<Inode> inode, const StringView& name, u8 access, bool cacheable)
{
auto region = make<Region>(range, move(inode), name, access, cacheable);
region->m_user_accessible = true;
return region;
}

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);
Expand Down
2 changes: 0 additions & 2 deletions Kernel/VM/Region.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ class Region final

static NonnullOwnPtr<Region> create_user_accessible(const Range&, const StringView& name, u8 access, bool cacheable = true);
static NonnullOwnPtr<Region> create_user_accessible(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const StringView& name, u8 access, bool cacheable = true);
static NonnullOwnPtr<Region> create_user_accessible(const Range&, NonnullRefPtr<Inode>, const StringView& name, u8 access, bool cacheable = true);
static NonnullOwnPtr<Region> create_kernel_only(const Range&, const StringView& name, u8 access, bool cacheable = true);
static NonnullOwnPtr<Region> create_kernel_only(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const StringView& name, u8 access, bool cacheable = true);

Expand Down Expand Up @@ -163,7 +162,6 @@ class Region final
// NOTE: These are public so we can make<> them.
Region(const Range&, const String&, u8 access, bool cacheable);
Region(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, const String&, u8 access, bool cacheable);
Region(const Range&, NonnullRefPtr<Inode>, const String&, u8 access, bool cacheable);

private:
Bitmap& ensure_cow_map() const;
Expand Down

0 comments on commit aa1e209

Please sign in to comment.