From 9505cd15a64933bf58ec50548339cf98b1854646 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 14 Jan 2020 18:03:05 -0500 Subject: lib9: make formatting lock-free again First use of . We will see if any supported systems don't have it yet. (C11 was so last decade.) Fixes #338. --- src/lib9/fmt/fmt.c | 165 ++++++++++++++++++++++--------------------------- src/lib9/fmt/fmtdef.h | 6 +- src/lib9/fmt/fmtlock.c | 14 +---- src/lib9/fmtlock2.c | 22 ++----- 4 files changed, 83 insertions(+), 124 deletions(-) (limited to 'src/lib9') diff --git a/src/lib9/fmt/fmt.c b/src/lib9/fmt/fmt.c index 9c3f45d3..a86482c3 100644 --- a/src/lib9/fmt/fmt.c +++ b/src/lib9/fmt/fmt.c @@ -1,102 +1,107 @@ /* Copyright (c) 2002-2006 Lucent Technologies; see LICENSE */ #include #include +#include #include "plan9.h" #include "fmt.h" #include "fmtdef.h" enum { - Maxfmt = 64 + Maxfmt = 128 }; typedef struct Convfmt Convfmt; struct Convfmt { int c; - volatile Fmts fmt; /* for spin lock in fmtfmt; avoids race due to write order */ + Fmts fmt; }; static struct { - /* lock by calling __fmtlock, __fmtunlock */ - int nfmt; + /* + * lock updates to fmt by calling __fmtlock, __fmtunlock. + * reads can start at nfmt and work backward without + * further locking. later fmtinstalls take priority over earlier + * ones because of the backwards loop. + * once installed, a format is never overwritten. + */ + _Atomic int nfmt; Convfmt fmt[Maxfmt]; -} fmtalloc; - -static Convfmt knownfmt[] = { - ' ', __flagfmt, - '#', __flagfmt, - '%', __percentfmt, - '\'', __flagfmt, - '+', __flagfmt, - ',', __flagfmt, - '-', __flagfmt, - 'C', __runefmt, /* Plan 9 addition */ - 'E', __efgfmt, -#ifndef PLAN9PORT - 'F', __efgfmt, /* ANSI only */ -#endif - 'G', __efgfmt, -#ifndef PLAN9PORT - 'L', __flagfmt, /* ANSI only */ -#endif - 'S', __runesfmt, /* Plan 9 addition */ - 'X', __ifmt, - 'b', __ifmt, /* Plan 9 addition */ - 'c', __charfmt, - 'd', __ifmt, - 'e', __efgfmt, - 'f', __efgfmt, - 'g', __efgfmt, - 'h', __flagfmt, -#ifndef PLAN9PORT - 'i', __ifmt, /* ANSI only */ -#endif - 'l', __flagfmt, - 'n', __countfmt, - 'o', __ifmt, - 'p', __ifmt, - 'r', __errfmt, - 's', __strfmt, -#ifdef PLAN9PORT - 'u', __flagfmt, -#else - 'u', __ifmt, -#endif - 'x', __ifmt, - 0, nil, +} fmtalloc = { + #ifdef PLAN9PORT + ATOMIC_VAR_INIT(27), + #else + ATOMIC_VAR_INIT(30), + #endif + { + {' ', __flagfmt}, + {'#', __flagfmt}, + {'%', __percentfmt}, + {'\'', __flagfmt}, + {'+', __flagfmt}, + {',', __flagfmt}, + {'-', __flagfmt}, + {'C', __runefmt}, /* Plan 9 addition */ + {'E', __efgfmt}, + #ifndef PLAN9PORT + {'F', __efgfmt}, /* ANSI only */ + #endif + {'G', __efgfmt}, + #ifndef PLAN9PORT + {'L', __flagfmt}, /* ANSI only */ + #endif + {'S', __runesfmt}, /* Plan 9 addition */ + {'X', __ifmt}, + {'b', __ifmt}, /* Plan 9 addition */ + {'c', __charfmt}, + {'d', __ifmt}, + {'e', __efgfmt}, + {'f', __efgfmt}, + {'g', __efgfmt}, + {'h', __flagfmt}, + #ifndef PLAN9PORT + {'i', __ifmt}, /* ANSI only */ + #endif + {'l', __flagfmt}, + {'n', __countfmt}, + {'o', __ifmt}, + {'p', __ifmt}, + {'r', __errfmt}, + {'s', __strfmt}, + #ifdef PLAN9PORT + {'u', __flagfmt}, + #else + {'u', __ifmt}, + #endif + {'x', __ifmt}, + } }; - int (*fmtdoquote)(int); /* - * __fmtwlock() must be set + * __fmtlock() must be set */ static int __fmtinstall(int c, Fmts f) { - Convfmt *p, *ep; + Convfmt *p; + int i; if(c<=0 || c>=65536) return -1; if(!f) f = __badfmt; - ep = &fmtalloc.fmt[fmtalloc.nfmt]; - for(p=fmtalloc.fmt; pc == c) - break; - - if(p == &fmtalloc.fmt[Maxfmt]) + i = atomic_load(&fmtalloc.nfmt); + if(i == Maxfmt) return -1; - + p = &fmtalloc.fmt[i]; + p->c = c; p->fmt = f; - if(p == ep){ /* installing a new format character */ - fmtalloc.nfmt++; - p->c = c; - } + atomic_store(&fmtalloc.nfmt, i+1); return 0; } @@ -106,43 +111,21 @@ fmtinstall(int c, int (*f)(Fmt*)) { int ret; - __fmtwlock(); + __fmtlock(); ret = __fmtinstall(c, f); - __fmtwunlock(); + __fmtunlock(); return ret; } static Fmts fmtfmt(int c) { - Convfmt *p, *ep, *kp; - - /* conflict-free check - common case */ - __fmtrlock(); - ep = &fmtalloc.fmt[fmtalloc.nfmt]; - for(p=fmtalloc.fmt; pc == c){ - __fmtrunlock(); + Convfmt *p, *ep; + + ep = &fmtalloc.fmt[atomic_load(&fmtalloc.nfmt)]; + for(p=ep; p-- > fmtalloc.fmt; ) + if(p->c == c) return p->fmt; - } - __fmtrunlock(); - - /* is this a predefined format char? */ - for(kp=knownfmt; kp->c; kp++){ - if(kp->c == c){ - __fmtwlock(); - /* double-check fmtinstall didn't happen */ - for(p=fmtalloc.fmt; pc == c){ - __fmtwunlock(); - return p->fmt; - } - } - __fmtinstall(kp->c, kp->fmt); - __fmtwunlock(); - return kp->fmt; - } - } return __badfmt; } diff --git a/src/lib9/fmt/fmtdef.h b/src/lib9/fmt/fmtdef.h index d547184d..2bafd36d 100644 --- a/src/lib9/fmt/fmtdef.h +++ b/src/lib9/fmt/fmtdef.h @@ -36,10 +36,8 @@ void * __fmtflush(Fmt *f, void *t, int len); int __fmtpad(Fmt *f, int n); double __fmtpow10(int n); int __fmtrcpy(Fmt *f, const void *vm, int n); -void __fmtrlock(void); -void __fmtrunlock(void); -void __fmtwlock(void); -void __fmtwunlock(void); +void __fmtlock(void); +void __fmtunlock(void); int __ifmt(Fmt *f); int __isInf(double d, int sign); int __isNaN(double d); diff --git a/src/lib9/fmt/fmtlock.c b/src/lib9/fmt/fmtlock.c index eb9cd845..cabe05f4 100644 --- a/src/lib9/fmt/fmtlock.c +++ b/src/lib9/fmt/fmtlock.c @@ -5,21 +5,11 @@ #include "fmtdef.h" void -__fmtrlock(void) +__fmtlock(void) { } void -__fmtrunlock(void) -{ -} - -void -__fmtwlock(void) -{ -} - -void -__fmtwunlock(void) +__fmtunlock(void) { } diff --git a/src/lib9/fmtlock2.c b/src/lib9/fmtlock2.c index b755daa3..d711e6d4 100644 --- a/src/lib9/fmtlock2.c +++ b/src/lib9/fmtlock2.c @@ -1,28 +1,16 @@ #include #include -static RWLock fmtlock; +static Lock fmtlock; void -__fmtrlock(void) +__fmtlock(void) { - rlock(&fmtlock); + lock(&fmtlock); } void -__fmtrunlock(void) +__fmtunlock(void) { - runlock(&fmtlock); -} - -void -__fmtwlock(void) -{ - wlock(&fmtlock); -} - -void -__fmtwunlock(void) -{ - wunlock(&fmtlock); + unlock(&fmtlock); } -- cgit v1.2.3 From 3ef80ba5f5c29a8367d32353a9620ec4cf9cb880 Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Thu, 16 Jan 2020 16:46:58 +0000 Subject: lib9: putenv wraps POSIX setenv, not legacy putenv POSIX setenv does everything that p9putenv's body, so just delegate to that. Signed-off-by: Dan Cross --- src/lib9/getenv.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'src/lib9') diff --git a/src/lib9/getenv.c b/src/lib9/getenv.c index 384196cf..5dfc8577 100644 --- a/src/lib9/getenv.c +++ b/src/lib9/getenv.c @@ -16,11 +16,5 @@ p9getenv(char *s) int p9putenv(char *s, char *v) { - char *t; - - t = smprint("%s=%s", s, v); - if(t == nil) - return -1; - putenv(t); - return 0; + return setenv(s, v, 1); } -- cgit v1.2.3 From 7bf2db4c2ae30c0f7b320e57060715bf6279e98a Mon Sep 17 00:00:00 2001 From: Dan Cross Date: Thu, 16 Jan 2020 16:54:19 +0000 Subject: malloc: remove locking The issue manifests in fork: POSIX fork mandates that a fork'd process is created with a single thread. If a multithreaded program forks, and some thread was in malloc() when the fork() happened, then in the child the lock will be held but there will be no thread to release it. We assume the system malloc() must already know how to deal with this and is thread-safe, but it won't know about our custom spinlock. Judging that this is no longer necessary (the lock code was added 15 years ago) we remove it. Signed-off-by: Dan Cross --- src/lib9/debugmalloc.c | 9 --------- src/lib9/malloc.c | 25 +++---------------------- 2 files changed, 3 insertions(+), 31 deletions(-) (limited to 'src/lib9') diff --git a/src/lib9/debugmalloc.c b/src/lib9/debugmalloc.c index 51a2c61f..744af835 100644 --- a/src/lib9/debugmalloc.c +++ b/src/lib9/debugmalloc.c @@ -6,7 +6,6 @@ * The Unix libc routines cannot be trusted to do their own locking. * Sad but apparently true. */ -static Lock malloclock; static int mallocpid; /* @@ -112,11 +111,9 @@ p9malloc(ulong n) if(n == 0) n++; /*fprint(2, "%s %d malloc\n", argv0, getpid()); */ - lock(&malloclock); mallocpid = getpid(); v = malloc(n+Overhead); v = mark(v, getcallerpc(&n), n, MallocMagic); - unlock(&malloclock); /*fprint(2, "%s %d donemalloc\n", argv0, getpid()); */ return v; } @@ -128,11 +125,9 @@ p9free(void *v) return; /*fprint(2, "%s %d free\n", argv0, getpid()); */ - lock(&malloclock); mallocpid = getpid(); v = mark(v, getcallerpc(&v), 0, FreeMagic); free(v); - unlock(&malloclock); /*fprint(2, "%s %d donefree\n", argv0, getpid()); */ } @@ -142,11 +137,9 @@ p9calloc(ulong a, ulong b) void *v; /*fprint(2, "%s %d calloc\n", argv0, getpid()); */ - lock(&malloclock); mallocpid = getpid(); v = calloc(a*b+Overhead, 1); v = mark(v, getcallerpc(&a), a*b, CallocMagic); - unlock(&malloclock); /*fprint(2, "%s %d donecalloc\n", argv0, getpid()); */ return v; } @@ -155,12 +148,10 @@ void* p9realloc(void *v, ulong n) { /*fprint(2, "%s %d realloc\n", argv0, getpid()); */ - lock(&malloclock); mallocpid = getpid(); v = mark(v, getcallerpc(&v), 0, CheckMagic); v = realloc(v, n+Overhead); v = mark(v, getcallerpc(&v), n, ReallocMagic); - unlock(&malloclock); /*fprint(2, "%s %d donerealloc\n", argv0, getpid()); */ return v; } diff --git a/src/lib9/malloc.c b/src/lib9/malloc.c index 33593aa2..695ff8bc 100644 --- a/src/lib9/malloc.c +++ b/src/lib9/malloc.c @@ -7,50 +7,31 @@ #define NOPLAN9DEFINES #include -static Lock malloclock; void* p9malloc(ulong n) { - void *v; - if(n == 0) n++; - lock(&malloclock); - v = malloc(n); - unlock(&malloclock); - return v; + return malloc(n); } void p9free(void *v) { - if(v == nil) - return; - lock(&malloclock); free(v); - unlock(&malloclock); } void* p9calloc(ulong a, ulong b) { - void *v; - if(a*b == 0) a = b = 1; - - lock(&malloclock); - v = calloc(a*b, 1); - unlock(&malloclock); - return v; + return calloc(a, b); } void* p9realloc(void *v, ulong n) { - lock(&malloclock); - v = realloc(v, n); - unlock(&malloclock); - return v; + return realloc(v, n); } -- cgit v1.2.3