From e21cc4cff63c36917d83730871dfff1a9f0eb927 Mon Sep 17 00:00:00 2001 From: Tom Date: Fri, 4 Sep 2020 11:47:49 -0600 Subject: [PATCH] Kernel: Remove MAP_PURGEABLE from mmap This brings mmap more in line with other operating systems. Prior to this, it was impossible to request memory that was definitely committed, instead MAP_PURGEABLE would provide a region that was not actually purgeable, but also not fully committed, which meant that using such memory still could cause crashes when the underlying pages could no longer be allocated. This fixes some random crashes in low-memory situations where non-volatile memory is mapped (e.g. malloc, tls, Gfx::Bitmap, etc) but when a page in these regions is first accessed, there is insufficient physical memory available to commit a new page. --- Kernel/Process.cpp | 2 +- Kernel/Syscalls/mmap.cpp | 13 +++---------- Kernel/UnixTypes.h | 3 +-- Libraries/LibC/malloc.cpp | 2 +- Libraries/LibC/mman.h | 2 +- Libraries/LibGfx/Bitmap.cpp | 10 +++++----- 6 files changed, 12 insertions(+), 20 deletions(-) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 9c74dd59da..df591b6304 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -144,7 +144,7 @@ Region& Process::allocate_split_region(const Region& source_region, const Range& Region* Process::allocate_region(const Range& range, const String& name, int prot, bool should_commit) { ASSERT(range.is_valid()); - auto vmobject = AnonymousVMObject::create_with_size(range.size()); + auto vmobject = PurgeableVMObject::create_with_size(range.size()); auto region = Region::create_user_accessible(this, range, vmobject, 0, name, prot_to_region_access_flags(prot)); if (!region->map(page_directory())) return nullptr; diff --git a/Kernel/Syscalls/mmap.cpp b/Kernel/Syscalls/mmap.cpp index 429f2be7cd..066603752d 100644 --- a/Kernel/Syscalls/mmap.cpp +++ b/Kernel/Syscalls/mmap.cpp @@ -116,7 +116,6 @@ void* Process::sys$mmap(Userspace user_params) bool map_shared = flags & MAP_SHARED; bool map_anonymous = flags & MAP_ANONYMOUS; - bool map_purgeable = flags & MAP_PURGEABLE; bool map_private = flags & MAP_PRIVATE; bool map_stack = flags & MAP_STACK; bool map_fixed = flags & MAP_FIXED; @@ -136,19 +135,13 @@ void* Process::sys$mmap(Userspace user_params) Region* region = nullptr; Optional range; - if (map_purgeable || map_anonymous) { + if (map_noreserve || map_anonymous) { range = allocate_range(VirtualAddress(addr), size, alignment); if (!range.value().is_valid()) return (void*)-ENOMEM; } - if (map_purgeable) { - - auto vmobject = PurgeableVMObject::create_with_size(size); - region = allocate_region_with_vmobject(range.value(), vmobject, 0, !name.is_null() ? name : "mmap (purgeable)", prot); - if (!region && (!map_fixed && addr != 0)) - region = allocate_region_with_vmobject({}, size, vmobject, 0, !name.is_null() ? name : "mmap (purgeable)", prot); - } else if (map_anonymous) { + if (map_anonymous) { region = allocate_region(range.value(), !name.is_null() ? name : "mmap", prot, !map_noreserve); if (!region && (!map_fixed && addr != 0)) region = allocate_region(allocate_range({}, size), !name.is_null() ? name : "mmap", prot, !map_noreserve); @@ -437,7 +430,7 @@ void* Process::sys$mremap(Userspace user_param if (!old_region->is_mmap()) return (void*)-EPERM; - if (old_region->vmobject().is_shared_inode() && params.flags & MAP_PRIVATE && !(params.flags & MAP_ANONYMOUS) && !(params.flags & MAP_PURGEABLE)) { + if (old_region->vmobject().is_shared_inode() && params.flags & MAP_PRIVATE && !(params.flags & (MAP_ANONYMOUS | MAP_NORESERVE))) { auto range = old_region->range(); auto old_name = old_region->name(); auto old_prot = region_access_flags_to_prot(old_region->access()); diff --git a/Kernel/UnixTypes.h b/Kernel/UnixTypes.h index 5b1830513d..0665d464f6 100644 --- a/Kernel/UnixTypes.h +++ b/Kernel/UnixTypes.h @@ -92,8 +92,7 @@ enum { #define MAP_ANONYMOUS 0x20 #define MAP_ANON MAP_ANONYMOUS #define MAP_STACK 0x40 -#define MAP_PURGEABLE 0x80 -#define MAP_NORESERVE 0x100 +#define MAP_NORESERVE 0x80 #define PROT_READ 0x1 #define PROT_WRITE 0x2 diff --git a/Libraries/LibC/malloc.cpp b/Libraries/LibC/malloc.cpp index 37b748d044..49d1f598e3 100644 --- a/Libraries/LibC/malloc.cpp +++ b/Libraries/LibC/malloc.cpp @@ -154,7 +154,7 @@ extern "C" { static void* os_alloc(size_t size, const char* name) { - auto* ptr = serenity_mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE | MAP_PURGEABLE, 0, 0, ChunkedBlock::block_size, name); + auto* ptr = serenity_mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0, ChunkedBlock::block_size, name); ASSERT(ptr != MAP_FAILED); return ptr; } diff --git a/Libraries/LibC/mman.h b/Libraries/LibC/mman.h index b783554365..5b02c59902 100644 --- a/Libraries/LibC/mman.h +++ b/Libraries/LibC/mman.h @@ -36,7 +36,7 @@ #define MAP_ANONYMOUS 0x20 #define MAP_ANON MAP_ANONYMOUS #define MAP_STACK 0x40 -#define MAP_PURGEABLE 0x80 +#define MAP_NORESERVE 0x80 #define PROT_READ 0x1 #define PROT_WRITE 0x2 diff --git a/Libraries/LibGfx/Bitmap.cpp b/Libraries/LibGfx/Bitmap.cpp index f03851248b..5756451c3f 100644 --- a/Libraries/LibGfx/Bitmap.cpp +++ b/Libraries/LibGfx/Bitmap.cpp @@ -437,13 +437,13 @@ Optional Bitmap::allocate_backing_store(BitmapFormat format, const const auto pitch = minimum_pitch(size.width(), format); const auto data_size_in_bytes = size_in_bytes(pitch, size.height()); - void* data = nullptr; + int map_flags = MAP_ANONYMOUS | MAP_PRIVATE; + if (purgeable == Purgeable::Yes) + map_flags |= MAP_NORESERVE; #ifdef __serenity__ - int map_flags = purgeable == Purgeable::Yes ? (MAP_PURGEABLE | MAP_PRIVATE) : (MAP_ANONYMOUS | MAP_PRIVATE); - data = mmap_with_name(nullptr, data_size_in_bytes, PROT_READ | PROT_WRITE, map_flags, 0, 0, String::format("GraphicsBitmap [%dx%d]", size.width(), size.height()).characters()); + void* data = mmap_with_name(nullptr, data_size_in_bytes, PROT_READ | PROT_WRITE, map_flags, 0, 0, String::format("GraphicsBitmap [%dx%d]", size.width(), size.height()).characters()); #else - int map_flags = (MAP_ANONYMOUS | MAP_PRIVATE); - data = mmap(nullptr, data_size_in_bytes, PROT_READ | PROT_WRITE, map_flags, 0, 0); + void* data = mmap(nullptr, data_size_in_bytes, PROT_READ | PROT_WRITE, map_flags, 0, 0); #endif if (data == MAP_FAILED) { perror("mmap");