From 4f4889ddf71d5f60f6f9fff0bb006639191f983b Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Sun, 17 Jul 2022 19:59:49 -0700 Subject: [PATCH] Use futexes on OpenBSD and improve threading --- libc/intrin/describeflags.internal.h | 2 ++ libc/intrin/describefutexresult.c | 31 ++++++++++++++++++ libc/intrin/futex.S | 3 +- libc/intrin/futex.internal.h | 1 - libc/intrin/futex_wait.c | 24 ++++---------- libc/intrin/futex_wake.c | 23 +++----------- libc/intrin/pthread.h | 3 +- libc/intrin/pthread_mutex_lock.c | 4 +-- libc/intrin/pthread_mutex_unlock.c | 2 +- libc/intrin/wait0.c | 6 +++- libc/runtime/clone.c | 9 ++++-- test/libc/calls/reservefd_test.c | 12 ++----- test/libc/stdio/dtoa_test.c | 3 +- test/libc/stdio/memory_test.c | 47 ++++++++++++++++++++++++++++ third_party/dlmalloc/dlmalloc.c | 1 + third_party/dlmalloc/platform.inc | 3 -- third_party/gdtoa/misc.c | 3 +- 17 files changed, 114 insertions(+), 63 deletions(-) create mode 100644 libc/intrin/describefutexresult.c create mode 100644 test/libc/stdio/memory_test.c diff --git a/libc/intrin/describeflags.internal.h b/libc/intrin/describeflags.internal.h index 40d82bde3..e0191347b 100644 --- a/libc/intrin/describeflags.internal.h +++ b/libc/intrin/describeflags.internal.h @@ -27,6 +27,7 @@ const char *DescribeFlags(char *, size_t, struct DescribeFlags *, size_t, const char *DescribeClockName(char[32], int); const char *DescribeDirfd(char[12], int); const char *DescribeFrame(char[32], int); +const char *DescribeFutexResult(char[12], int); const char *DescribeHow(char[12], int); const char *DescribeMapFlags(char[64], int); const char *DescribeMapping(char[8], int, int); @@ -76,6 +77,7 @@ void DescribeIovNt(const struct NtIovec *, uint32_t, ssize_t); #define DescribeClockName(x) DescribeClockName(alloca(32), x) #define DescribeDirfd(dirfd) DescribeDirfd(alloca(12), dirfd) #define DescribeFrame(x) DescribeFrame(alloca(32), x) +#define DescribeFutexResult(x) DescribeFutexResult(alloca(12), x) #define DescribeHow(x) DescribeHow(alloca(12), x) #define DescribeMapFlags(dirfd) DescribeMapFlags(alloca(64), dirfd) #define DescribeMapping(x, y) DescribeMapping(alloca(8), x, y) diff --git a/libc/intrin/describefutexresult.c b/libc/intrin/describefutexresult.c new file mode 100644 index 000000000..77d89c340 --- /dev/null +++ b/libc/intrin/describefutexresult.c @@ -0,0 +1,31 @@ +/*-*- mode:c;indent-tabs-mode:nil;c-basic-offset:2;tab-width:8;coding:utf-8 -*-│ +│vi: set net ft=c ts=2 sts=2 sw=2 fenc=utf-8 :vi│ +╞══════════════════════════════════════════════════════════════════════════════╡ +│ Copyright 2022 Justine Alexandra Roberts Tunney │ +│ │ +│ Permission to use, copy, modify, and/or distribute this software for │ +│ any purpose with or without fee is hereby granted, provided that the │ +│ above copyright notice and this permission notice appear in all copies. │ +│ │ +│ THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL │ +│ WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED │ +│ WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE │ +│ AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL │ +│ DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR │ +│ PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER │ +│ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR │ +│ PERFORMANCE OF THIS SOFTWARE. │ +╚─────────────────────────────────────────────────────────────────────────────*/ +#include "libc/fmt/itoa.h" +#include "libc/intrin/describeflags.internal.h" +#include "libc/str/str.h" + +const char *(DescribeFutexResult)(char buf[12], int ax) { + const char *s; + if (ax > -4095u && (s = strerrno(-ax))) { + return s; + } else { + FormatInt32(buf, ax); + return buf; + } +} diff --git a/libc/intrin/futex.S b/libc/intrin/futex.S index cfb9df4b0..419bf82ba 100644 --- a/libc/intrin/futex.S +++ b/libc/intrin/futex.S @@ -20,7 +20,8 @@ #include "libc/macros.internal.h" .privileged -_futex: mov __NR_futex,%eax +_futex: mov %rcx,%r10 + mov __NR_futex,%eax clc syscall jnc 1f diff --git a/libc/intrin/futex.internal.h b/libc/intrin/futex.internal.h index 1dd8f0573..b67b9014f 100644 --- a/libc/intrin/futex.internal.h +++ b/libc/intrin/futex.internal.h @@ -4,7 +4,6 @@ #if !(__ASSEMBLER__ + __LINKER__ + 0) COSMOPOLITAN_C_START_ -int _futex(void *, int, int, struct timespec *, int *) hidden; int _futex_wait(void *, int, struct timespec *) hidden; int _futex_wake(void *, int) hidden; diff --git a/libc/intrin/futex_wait.c b/libc/intrin/futex_wait.c index 5cf41574d..f72e7b8e3 100644 --- a/libc/intrin/futex_wait.c +++ b/libc/intrin/futex_wait.c @@ -18,28 +18,16 @@ ╚─────────────────────────────────────────────────────────────────────────────*/ #include "libc/calls/strace.internal.h" #include "libc/calls/struct/timespec.h" -#include "libc/errno.h" -#include "libc/fmt/itoa.h" +#include "libc/dce.h" #include "libc/intrin/describeflags.internal.h" #include "libc/intrin/futex.internal.h" -#include "libc/mem/alloca.h" -#include "libc/str/str.h" #include "libc/sysv/consts/futex.h" -static const char *DescribeFutexWaitResult(char buf[12], int ax) { - const char *s; - if (ax && ((s = strerrno(ax)) || (s = strerrno(-ax)))) { - return s; - } else { - FormatInt32(buf, ax); - return buf; - } -} - +int _futex(void *, int, int, struct timespec *) hidden; int _futex_wait(void *addr, int expect, struct timespec *timeout) { - int ax; - ax = _futex(addr, FUTEX_WAIT, expect, timeout, 0); - STRACE("futex(%t[%p], FUTEX_WAIT, %d, %s) → %s", addr, addr, expect, - DescribeTimespec(0, timeout), DescribeFutexWaitResult(alloca(12), ax)); + int ax = _futex(addr, FUTEX_WAIT, expect, timeout); + if (IsOpenbsd() && ax > 0) ax = -ax; // yes openbsd does this w/o cf + STRACE("futex(%t, FUTEX_WAIT, %d, %s) → %s", addr, expect, + DescribeTimespec(0, timeout), DescribeFutexResult(ax)); return ax; } diff --git a/libc/intrin/futex_wake.c b/libc/intrin/futex_wake.c index bbfaf0d3f..7e1602b07 100644 --- a/libc/intrin/futex_wake.c +++ b/libc/intrin/futex_wake.c @@ -16,30 +16,15 @@ │ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR │ │ PERFORMANCE OF THIS SOFTWARE. │ ╚─────────────────────────────────────────────────────────────────────────────*/ -#include "libc/bits/asmflag.h" #include "libc/calls/strace.internal.h" -#include "libc/fmt/itoa.h" #include "libc/intrin/describeflags.internal.h" #include "libc/intrin/futex.internal.h" -#include "libc/mem/alloca.h" -#include "libc/str/str.h" #include "libc/sysv/consts/futex.h" -#include "libc/sysv/consts/nr.h" - -static const char *DescribeFutexWakeResult(char buf[12], int ax) { - const char *s; - if (ax < 0 && (s = strerrno(ax))) { - return s; - } else { - FormatInt32(buf, ax); - return buf; - } -} +int _futex(void *, int, int) hidden; int _futex_wake(void *addr, int count) { - int ax; - ax = _futex(addr, FUTEX_WAKE, count, 0, 0); - STRACE("futex(%t[%p], FUTEX_WAKE, %d) → %s", addr, addr, count, - DescribeFutexWakeResult(alloca(12), ax)); + int ax = _futex(addr, FUTEX_WAKE, count); + STRACE("futex(%t, FUTEX_WAKE, %d) → %s", addr, count, + DescribeFutexResult(ax)); return ax; } diff --git a/libc/intrin/pthread.h b/libc/intrin/pthread.h index 5dc17b45f..0c5c2d414 100644 --- a/libc/intrin/pthread.h +++ b/libc/intrin/pthread.h @@ -137,11 +137,10 @@ void *pthread_getspecific(pthread_key_t); !atomic_exchange(&(mutex)->lock, 1)) \ ? 0 \ : pthread_mutex_lock(mutex)) - #define pthread_mutex_unlock(mutex) \ ((mutex)->attr == PTHREAD_MUTEX_NORMAL \ ? (atomic_store_explicit(&(mutex)->lock, 0, memory_order_relaxed), \ - ((IsLinux() /* || IsOpenbsd() */) && \ + ((IsLinux() || IsOpenbsd()) && \ atomic_load_explicit(&(mutex)->waits, memory_order_relaxed) && \ _pthread_mutex_wake(mutex)), \ 0) \ diff --git a/libc/intrin/pthread_mutex_lock.c b/libc/intrin/pthread_mutex_lock.c index 6720e39a4..bf6c36a44 100644 --- a/libc/intrin/pthread_mutex_lock.c +++ b/libc/intrin/pthread_mutex_lock.c @@ -32,12 +32,12 @@ static int pthread_mutex_lock_spin(pthread_mutex_t *mutex, int expect, int tries) { - volatile int i; if (tries < 7) { + volatile int i; for (i = 0; i != 1 << tries; i++) { } tries++; - } else if (IsLinux() /* || IsOpenbsd() */) { + } else if (IsLinux() || IsOpenbsd()) { atomic_fetch_add(&mutex->waits, 1); _futex_wait(&mutex->lock, expect, &(struct timespec){1}); atomic_fetch_sub(&mutex->waits, 1); diff --git a/libc/intrin/pthread_mutex_unlock.c b/libc/intrin/pthread_mutex_unlock.c index ab6e399cf..f07181963 100644 --- a/libc/intrin/pthread_mutex_unlock.c +++ b/libc/intrin/pthread_mutex_unlock.c @@ -44,7 +44,7 @@ int(pthread_mutex_unlock)(pthread_mutex_t *mutex) { // fallthrough case PTHREAD_MUTEX_NORMAL: atomic_store_explicit(&mutex->lock, 0, memory_order_relaxed); - if ((IsLinux() /* || IsOpenbsd() */) && + if ((IsLinux() || IsOpenbsd()) && atomic_load_explicit(&mutex->waits, memory_order_relaxed) > 0) { _pthread_mutex_wake(mutex); } diff --git a/libc/intrin/wait0.c b/libc/intrin/wait0.c index 6442e2242..79a123220 100644 --- a/libc/intrin/wait0.c +++ b/libc/intrin/wait0.c @@ -36,10 +36,14 @@ void _wait0(const int *ctid) { for (;;) { if (!(x = atomic_load_explicit(ctid, memory_order_acquire))) { break; - } else if (IsLinux() /* || IsOpenbsd() */) { + } else if (IsLinux() || IsOpenbsd()) { _futex_wait(ctid, x, &(struct timespec){2}); } else { sched_yield(); } } + if (IsOpenbsd()) { + // TODO(jart): whyyyy do we need it + sched_yield(); + } } diff --git a/libc/runtime/clone.c b/libc/runtime/clone.c index 5ef64ea21..34fea0a99 100644 --- a/libc/runtime/clone.c +++ b/libc/runtime/clone.c @@ -306,11 +306,14 @@ noasan static wontreturn void OpenbsdThreadMain(void *p) { // although ideally there should be a better solution. // // void __threxit(%rdi = int32_t *notdead); - asm volatile("mov\t%2,%%rsp\n\t" // set stack + asm volatile("mov\t%2,%%rsp\n\t" "movl\t$0,(%%rdi)\n\t" // *wt->ztid = 0 - "syscall" // __threxit() + "syscall\n\t" // futex() + "mov\t$302,%%eax\n\t" // threxit() + "syscall" : "=m"(*wt->ztid) - : "a"(302), "m"(oldrsp), "D"(wt->ztid) + : "a"(83), "m"(oldrsp), "D"(wt->ztid), "S"(FUTEX_WAKE), + "d"(INT_MAX) : "rcx", "r11", "memory"); unreachable; } diff --git a/test/libc/calls/reservefd_test.c b/test/libc/calls/reservefd_test.c index ce6328963..36cc18f4f 100644 --- a/test/libc/calls/reservefd_test.c +++ b/test/libc/calls/reservefd_test.c @@ -48,10 +48,6 @@ STATIC_YOINK("libc/testlib/hyperion.txt"); #define THREADS 8 -__attribute__((__constructor__)) static void init(void) { - if (IsOpenbsd()) exit(0); // TODO(jart): flakes :'( -} - void PullSomeZipFilesIntoLinkage(void) { gmtime(0); } @@ -121,12 +117,8 @@ TEST(reservefd, tortureTest) { .sa_flags = 0 /* SA_NODEFER */}; // ASSERT_SYS(0, 0, sigaction(SIGALRM, &sa, &oldsa)); // ASSERT_SYS(0, 0, setitimer(ITIMER_REAL, &it, &oldit)); - for (i = 0; i < THREADS; ++i) { - _spawn(Worker, 0, th + i); - } - for (i = 0; i < THREADS; ++i) { - _join(th + i); - } + for (i = 0; i < THREADS; ++i) _spawn(Worker, 0, th + i); + for (i = 0; i < THREADS; ++i) _join(th + i); // EXPECT_SYS(0, 0, sigaction(SIGALRM, &oldsa, 0)); // EXPECT_SYS(0, 0, setitimer(ITIMER_REAL, &oldit, 0)); } diff --git a/test/libc/stdio/dtoa_test.c b/test/libc/stdio/dtoa_test.c index 5ea5c6cc7..a20910288 100644 --- a/test/libc/stdio/dtoa_test.c +++ b/test/libc/stdio/dtoa_test.c @@ -17,6 +17,7 @@ │ PERFORMANCE OF THIS SOFTWARE. │ ╚─────────────────────────────────────────────────────────────────────────────*/ #include "libc/calls/calls.h" +#include "libc/calls/struct/sched_param.h" #include "libc/dce.h" #include "libc/fmt/fmt.h" #include "libc/intrin/spinlock.h" @@ -29,6 +30,7 @@ #include "libc/sysv/consts/clone.h" #include "libc/sysv/consts/map.h" #include "libc/sysv/consts/prot.h" +#include "libc/sysv/consts/sched.h" #include "libc/testlib/testlib.h" #include "libc/thread/spawn.h" #include "libc/x/x.h" @@ -61,7 +63,6 @@ int Worker(void *p, int tid) { TEST(dtoa, locks) { int i, n = 32; struct spawn *t = gc(malloc(sizeof(struct spawn) * n)); - if (IsOpenbsd()) return; // TODO(jart): OpenBSD flakes :'( for (i = 0; i < n; ++i) ASSERT_SYS(0, 0, _spawn(Worker, 0, t + i)); for (i = 0; i < n; ++i) EXPECT_SYS(0, 0, _join(t + i)); } diff --git a/test/libc/stdio/memory_test.c b/test/libc/stdio/memory_test.c new file mode 100644 index 000000000..c61ea96c6 --- /dev/null +++ b/test/libc/stdio/memory_test.c @@ -0,0 +1,47 @@ +/*-*- mode:c;indent-tabs-mode:nil;c-basic-offset:2;tab-width:8;coding:utf-8 -*-│ +│vi: set net ft=c ts=2 sts=2 sw=2 fenc=utf-8 :vi│ +╞══════════════════════════════════════════════════════════════════════════════╡ +│ Copyright 2022 Justine Alexandra Roberts Tunney │ +│ │ +│ Permission to use, copy, modify, and/or distribute this software for │ +│ any purpose with or without fee is hereby granted, provided that the │ +│ above copyright notice and this permission notice appear in all copies. │ +│ │ +│ THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL │ +│ WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED │ +│ WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE │ +│ AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL │ +│ DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR │ +│ PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER │ +│ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR │ +│ PERFORMANCE OF THIS SOFTWARE. │ +╚─────────────────────────────────────────────────────────────────────────────*/ +#include "libc/calls/calls.h" +#include "libc/mem/mem.h" +#include "libc/runtime/gc.internal.h" +#include "libc/stdio/stdio.h" +#include "libc/testlib/testlib.h" +#include "libc/thread/spawn.h" + +int Worker(void *arg, int tid) { + int i; + char *volatile p; + char *volatile q; + for (i = 0; i < 256; ++i) { + p = malloc(17); + free(p); + p = malloc(17); + q = malloc(17); + sched_yield(); + free(p); + free(q); + } + return 0; +} + +TEST(memory, test) { + int i, n = 32; + struct spawn *t = gc(malloc(sizeof(struct spawn) * n)); + for (i = 0; i < n; ++i) ASSERT_SYS(0, 0, _spawn(Worker, 0, t + i)); + for (i = 0; i < n; ++i) EXPECT_SYS(0, 0, _join(t + i)); +} diff --git a/third_party/dlmalloc/dlmalloc.c b/third_party/dlmalloc/dlmalloc.c index 37d68f70b..827dd4867 100644 --- a/third_party/dlmalloc/dlmalloc.c +++ b/third_party/dlmalloc/dlmalloc.c @@ -23,6 +23,7 @@ #define HAVE_MMAP 1 #define HAVE_MREMAP 0 #define HAVE_MORECORE 0 +#define USE_LOCKS 1 #define USE_SPIN_LOCKS 1 #define MORECORE_CONTIGUOUS 0 #define MALLOC_INSPECT_ALL 1 diff --git a/third_party/dlmalloc/platform.inc b/third_party/dlmalloc/platform.inc index 81151c0ba..8b0cd9e71 100644 --- a/third_party/dlmalloc/platform.inc +++ b/third_party/dlmalloc/platform.inc @@ -250,12 +250,9 @@ extern void* sbrk(ptrdiff_t); #if USE_LOCKS #ifndef WIN32 #if defined (__SVR4) && defined (__sun) /* solaris */ -#include #elif !defined(LACKS_SCHED_H) -#include #endif /* solaris or LACKS_SCHED_H */ #if (defined(USE_RECURSIVE_LOCKS) && USE_RECURSIVE_LOCKS != 0) || !USE_SPIN_LOCKS -#include #endif /* USE_RECURSIVE_LOCKS ... */ #elif defined(_MSC_VER) #ifndef _M_AMD64 diff --git a/third_party/gdtoa/misc.c b/third_party/gdtoa/misc.c index f8460ca6a..4976bd02a 100644 --- a/third_party/gdtoa/misc.c +++ b/third_party/gdtoa/misc.c @@ -29,7 +29,6 @@ │ THIS SOFTWARE. │ │ │ ╚─────────────────────────────────────────────────────────────────────────────*/ -#include "libc/assert.h" #include "libc/calls/calls.h" #include "libc/macros.internal.h" #include "libc/runtime/runtime.h" @@ -54,7 +53,9 @@ __gdtoa_Bclear(void) __gdtoa_lock(); for (i = 0; i < ARRAYLEN(TI0.Freelist); ++i) __gdtoa_Brelease(TI0.Freelist[i]); + __gdtoa_lock1(); __gdtoa_Brelease(TI0.P5s); + __gdtoa_unlock1(); bzero(&TI0, sizeof(TI0)); __gdtoa_unlock(); }