From 23dfb79d3349ea26105fa44b339f0021427182a2 Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Thu, 18 Jul 2024 19:19:51 -0700 Subject: [PATCH] Fix minor suboptimalities in memory manager --- libc/intrin/maps.c | 5 ++- libc/intrin/maps.h | 10 ++--- libc/intrin/mmap.c | 94 +++++++++++++++++++++------------------------- 3 files changed, 49 insertions(+), 60 deletions(-) diff --git a/libc/intrin/maps.c b/libc/intrin/maps.c index af394f12b..5ca9861ee 100644 --- a/libc/intrin/maps.c +++ b/libc/intrin/maps.c @@ -89,7 +89,7 @@ privileged bool __maps_lock(void) { if (!__tls_enabled) return false; tib = __get_tls_privileged(); - if (tib->tib_relock_maps++) + if (atomic_fetch_add_explicit(&tib->tib_relock_maps, 1, memory_order_relaxed)) return true; while (atomic_exchange_explicit(&__maps.lock, 1, memory_order_acquire)) { #if defined(__GNUC__) && defined(__aarch64__) @@ -106,6 +106,7 @@ privileged void __maps_unlock(void) { if (!__tls_enabled) return; tib = __get_tls_privileged(); - if (!--tib->tib_relock_maps) + if (atomic_fetch_sub_explicit(&tib->tib_relock_maps, 1, + memory_order_relaxed) == 1) atomic_store_explicit(&__maps.lock, 0, memory_order_release); } diff --git a/libc/intrin/maps.h b/libc/intrin/maps.h index 499530a4e..06b8a813e 100644 --- a/libc/intrin/maps.h +++ b/libc/intrin/maps.h @@ -1,14 +1,12 @@ #ifndef COSMOPOLITAN_MAPS_H_ #define COSMOPOLITAN_MAPS_H_ #include "libc/intrin/atomic.h" -#include "libc/intrin/dll.h" #include "libc/intrin/tree.h" #include "libc/runtime/runtime.h" #include "libc/thread/tls2.internal.h" COSMOPOLITAN_C_START_ #define MAP_TREE_CONTAINER(e) TREE_CONTAINER(struct Map, tree, e) -#define MAP_FREE_CONTAINER(e) DLL_CONTAINER(struct Map, free, e) struct Map { char *addr; /* granule aligned */ @@ -22,14 +20,14 @@ struct Map { intptr_t hand; /* windows nt only */ union { struct Tree tree; - struct Dll free; + struct Map *free; }; }; struct Maps { atomic_int lock; struct Tree *maps; - struct Dll *free; + _Atomic(struct Map *) free; size_t count; size_t pages; atomic_size_t rollo; @@ -64,14 +62,14 @@ forceinline optimizespeed int __maps_search(const void *key, return (addr > map->addr) - (addr < map->addr); } -static struct Map *__maps_next(struct Map *map) { +static inline struct Map *__maps_next(struct Map *map) { struct Tree *node; if ((node = tree_next(&map->tree))) return MAP_TREE_CONTAINER(node); return 0; } -static struct Map *__maps_first(void) { +static inline struct Map *__maps_first(void) { struct Tree *node; if ((node = tree_first(__maps.maps))) return MAP_TREE_CONTAINER(node); diff --git a/libc/intrin/mmap.c b/libc/intrin/mmap.c index 8cbb7feb8..25b2ead83 100644 --- a/libc/intrin/mmap.c +++ b/libc/intrin/mmap.c @@ -31,7 +31,6 @@ #include "libc/intrin/describebacktrace.internal.h" #include "libc/intrin/describeflags.internal.h" #include "libc/intrin/directmap.internal.h" -#include "libc/intrin/dll.h" #include "libc/intrin/kprintf.h" #include "libc/intrin/maps.h" #include "libc/intrin/strace.internal.h" @@ -124,7 +123,7 @@ void __maps_check(void) { } static int __muntrack(char *addr, size_t size, int pagesz, - struct Dll **deleted) { + struct Map **deleted) { int rc = 0; struct Map *map; struct Map *next; @@ -140,8 +139,8 @@ static int __muntrack(char *addr, size_t size, int pagesz, if (addr <= map_addr && addr + PGUP(size) >= map_addr + PGUP(map_size)) { // remove mapping completely tree_remove(&__maps.maps, &map->tree); - dll_init(&map->free); - dll_make_first(deleted, &map->free); + map->free = *deleted; + *deleted = map; __maps.pages -= (map_size + pagesz - 1) / pagesz; __maps.count -= 1; __maps_check(); @@ -166,8 +165,8 @@ static int __muntrack(char *addr, size_t size, int pagesz, __maps.pages -= (left + pagesz - 1) / pagesz; leftmap->addr = map_addr; leftmap->size = left; - dll_init(&leftmap->free); - dll_make_first(deleted, &leftmap->free); + leftmap->free = *deleted; + *deleted = leftmap; __maps_check(); } else { rc = -1; @@ -182,8 +181,8 @@ static int __muntrack(char *addr, size_t size, int pagesz, __maps.pages -= (right + pagesz - 1) / pagesz; rightmap->addr = addr; rightmap->size = right; - dll_init(&rightmap->free); - dll_make_first(deleted, &rightmap->free); + rightmap->free = *deleted; + *deleted = rightmap; __maps_check(); } else { rc = -1; @@ -211,8 +210,8 @@ static int __muntrack(char *addr, size_t size, int pagesz, __maps.count += 1; middlemap->addr = addr; middlemap->size = size; - dll_init(&middlemap->free); - dll_make_first(deleted, &middlemap->free); + middlemap->free = *deleted; + *deleted = middlemap; __maps_check(); } else { rc = -1; @@ -228,8 +227,21 @@ static int __muntrack(char *addr, size_t size, int pagesz, void __maps_free(struct Map *map) { map->size = 0; map->addr = MAP_FAILED; - dll_init(&map->free); - dll_make_first(&__maps.free, &map->free); + map->free = atomic_load_explicit(&__maps.free, memory_order_relaxed); + for (;;) { + if (atomic_compare_exchange_weak_explicit(&__maps.free, &map->free, map, + memory_order_release, + memory_order_relaxed)) + break; + } +} + +static void __maps_free_all(struct Map *list) { + struct Map *next; + for (struct Map *map = list; map; map = next) { + next = map->free; + __maps_free(map); + } } static void __maps_insert(struct Map *map) { @@ -282,12 +294,13 @@ static void __maps_insert(struct Map *map) { } struct Map *__maps_alloc(void) { - struct Dll *e; struct Map *map; - if ((e = dll_first(__maps.free))) { - dll_remove(&__maps.free, e); - map = MAP_FREE_CONTAINER(e); - return map; + map = atomic_load_explicit(&__maps.free, memory_order_relaxed); + while (map) { + if (atomic_compare_exchange_weak_explicit(&__maps.free, &map, map->free, + memory_order_acquire, + memory_order_relaxed)) + return map; } int gransz = getgransize(); struct DirectMap sys = sys_mmap(0, gransz, PROT_READ | PROT_WRITE, @@ -335,14 +348,13 @@ static int __munmap(char *addr, size_t size) { } // untrack mappings - struct Dll *deleted = 0; + struct Map *deleted = 0; __muntrack(addr, pgup_size, pagesz, &deleted); __maps_unlock(); // delete mappings int rc = 0; - for (struct Dll *e = dll_first(deleted); e; e = dll_next(deleted, e)) { - struct Map *map = MAP_FREE_CONTAINER(e); + for (struct Map *map = deleted; map; map = map->free) { if (!IsWindows()) { if (sys_munmap(map->addr, map->size)) rc = -1; @@ -356,11 +368,7 @@ static int __munmap(char *addr, size_t size) { } // free mappings - if (!dll_is_empty(deleted)) { - __maps_lock(); - dll_make_first(&__maps.free, deleted); - __maps_unlock(); - } + __maps_free_all(deleted); return rc; } @@ -392,20 +400,12 @@ static void *__mmap_chunk(void *addr, size_t size, int prot, int flags, int fd, // allocate Map object struct Map *map; - if (__maps_lock()) { - __maps_unlock(); - return (void *)edeadlk(); - } - __maps_check(); - map = __maps_alloc(); - __maps_unlock(); - if (!map) + if (!(map = __maps_alloc())) return MAP_FAILED; // remove mapping we blew away if (IsWindows() && should_untrack) - if (__munmap(addr, size)) - return MAP_FAILED; + __munmap(addr, size); // obtain mapping from operating system int olderr = errno; @@ -424,9 +424,7 @@ TryAgain: goto TryAgain; } } - __maps_lock(); __maps_free(map); - __maps_unlock(); return MAP_FAILED; } @@ -440,21 +438,15 @@ TryAgain: UnmapViewOfFile(res.addr); CloseHandle(res.maphandle); } - __maps_lock(); __maps_free(map); - __maps_unlock(); return (void *)eexist(); } // untrack mapping we blew away if (!IsWindows() && should_untrack) { - struct Dll *deleted = 0; + struct Map *deleted = 0; __muntrack(res.addr, size, pagesz, &deleted); - if (!dll_is_empty(deleted)) { - __maps_lock(); - dll_make_first(&__maps.free, deleted); - __maps_unlock(); - } + __maps_free_all(deleted); } // track map object @@ -632,25 +624,23 @@ static void *__mremap_impl(char *old_addr, size_t old_size, size_t new_size, } else { res = sys_mremap(old_addr, old_size, new_size, flags, (uintptr_t)new_addr); } + if (res == MAP_FAILED) + __maps_free(map); // re-acquire lock if needed if (!flags) __maps_lock(); - // check result - if (res == MAP_FAILED) { - __maps_free(map); + if (res == MAP_FAILED) return MAP_FAILED; - } if (!(flags & MREMAP_MAYMOVE)) ASSERT(res == old_addr); // untrack old mapping - struct Dll *deleted = 0; + struct Map *deleted = 0; __muntrack(old_addr, old_size, pagesz, &deleted); - dll_make_first(&__maps.free, deleted); - deleted = 0; + __maps_free_all(deleted); // track map object map->addr = res;