add proper fuxed-based locking for stdio

previously, stdio used spinlocks, which would be unacceptable if we
ever add support for thread priorities, and which yielded
pathologically bad performance if an application attempted to use
flockfile on a key file as a major/primary locking mechanism.

i had held off on making this change for fear that it would hurt
performance in the non-threaded case, but actually support for
recursive locking had already inflicted that cost. by having the
internal locking functions store a flag indicating whether they need
to perform unlocking, rather than using the actual recursive lock
counter, i was able to combine the conditionals at unlock time,
eliminating any additional cost, and also avoid a nasty corner case
where a huge number of calls to ftrylockfile could cause deadlock
later at the point of internal locking.

this commit also fixes some issues with usage of pthread_self
conflicting with __attribute__((const)) which resulted in crashes with
some compiler versions/optimizations, mainly in flockfile prior to
pthread_create.

diff --git a/src/internal/libc.h b/src/internal/libc.h
index 906de2caba70c42988ca5123f5bc6f505ec88267..929ff97ae4257d8493d6c6c0e7c32c6ce58c5445 100644 (file)
--- a/src/internal/libc.h
+++ b/src/internal/libc.h
@@ -38,7 +38,8 @@ extern struct __libc *__libc_loc(void) __attribute__((const));
 
 /* Designed to avoid any overhead in non-threaded processes */
 void __lock(volatile int *);
-void __lockfile(FILE *);
+int __lockfile(FILE *);
+void __unlockfile(FILE *);
 #define LOCK(x) (libc.threads_minus_1 ? (__lock(x),1) : ((void)(x),1))
 #define UNLOCK(x) (*(volatile int *)(x)=0)
 

diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h
index e6089f02f7ec601f2c8752cf03d1848395892410..03af4c120dc13d352b18a953818e5dfca00e3d21 100644 (file)
--- a/src/internal/pthread_impl.h
+++ b/src/internal/pthread_impl.h
@@ -87,6 +87,8 @@ struct __timer {
 #define SIGTIMER_SET ((sigset_t *)(unsigned long [1+(sizeof(long)==4)]){ \
         0x80000000 })
 
+pthread_t __pthread_self_init(void);
+
 int __set_thread_area(void *);
 int __libc_sigaction(int, const struct sigaction *, struct sigaction *);
 int __libc_sigprocmask(int, const sigset_t *, sigset_t *);

diff --git a/src/internal/stdio_impl.h b/src/internal/stdio_impl.h
index 76b58be9fb6c6aaa11a2a240f66df99947f9cba5..c5f45eb1eed42df486fbe1c3bd29e7fb7a0df524 100644 (file)
--- a/src/internal/stdio_impl.h
+++ b/src/internal/stdio_impl.h
@@ -24,8 +24,8 @@
 
 #define UNGET 8
 
-#define FLOCK(f) ((libc.threads_minus_1 && (f)->lock>=0) ? (__lockfile((f)),0) : 0)
-#define FUNLOCK(f) ((f)->lockcount && (--(f)->lockcount || ((f)->lock=0)))
+#define FLOCK(f) int __need_unlock = ((f)->lock>=0 ? __lockfile((f)) : 0)
+#define FUNLOCK(f) if (__need_unlock) __unlockfile((f)); else
 
 #define F_PERM 1
 #define F_NORD 4
@@ -49,12 +49,12 @@ struct __FILE_s {
        FILE *prev, *next;
        int fd;
        int pipe_pid;
-       long dummy2;
+       long lockcount;
        short dummy3;
        signed char mode;
        signed char lbf;
        int lock;
-       int lockcount;
+       int waiters;
        void *cookie;
        off_t off;
        int (*flush)(FILE *);
@@ -86,7 +86,6 @@ FILE *__fdopen(int, const char *);
 
 #define OFLLOCK() LOCK(&libc.ofl_lock)
 #define OFLUNLOCK() UNLOCK(&libc.ofl_lock)
-#define ofl_head (libc.ofl_head)
 
 #define feof(f) ((f)->flags & F_EOF)
 #define ferror(f) ((f)->flags & F_ERR)

diff --git a/src/stdio/__fdopen.c b/src/stdio/__fdopen.c
index b13318e57a8798172f11411a997058ffd6909096..07f966a5a6556641946962082c3cab0042522f9a 100644 (file)
--- a/src/stdio/__fdopen.c
+++ b/src/stdio/__fdopen.c
@@ -39,11 +39,13 @@ FILE *__fdopen(int fd, const char *mode)
        f->seek = __stdio_seek;
        f->close = __stdio_close;
 
+       if (!libc.threaded) f->lock = -1;
+
        /* Add new FILE to open file list */
        OFLLOCK();
-       f->next = ofl_head;
-       if (ofl_head) ofl_head->prev = f;
-       ofl_head = f;
+       f->next = libc.ofl_head;
+       if (libc.ofl_head) libc.ofl_head->prev = f;
+       libc.ofl_head = f;
        OFLUNLOCK();
 
        return f;

diff --git a/src/stdio/__lockfile.c b/src/stdio/__lockfile.c
index 66a4d26d16a6134306b6d6c1dad8809ca0468882..6ebf6202bf189284b8d9a02d153259842a29a0cc 100644 (file)
--- a/src/stdio/__lockfile.c
+++ b/src/stdio/__lockfile.c
@@ -1,20 +1,18 @@
 #include "stdio_impl.h"
 #include "pthread_impl.h"
 
-void __lockfile(FILE *f)
+int __lockfile(FILE *f)
 {
-       int spins=10000;
-       int tid;
+       int owner, tid = __pthread_self()->tid;
+       if (f->lock == tid)
+               return 0;
+       while ((owner = a_cas(&f->lock, 0, tid)))
+               __wait(&f->lock, &f->waiters, owner, 1);
+       return f->lockcount = 1;
+}
 
-       if (f->lock < 0) return;
-       tid = __pthread_self()->tid;
-       if (f->lock == tid) {
-               while (f->lockcount == INT_MAX);
-               f->lockcount++;
-               return;
-       }
-       while (a_cas(&f->lock, 0, tid))
-               if (spins) spins--, a_spin();
-               else __syscall(SYS_sched_yield);
-       f->lockcount = 1;
+void __unlockfile(FILE *f)
+{
+       a_store(&f->lock, 0);
+       if (f->waiters) __wake(&f->lock, 1, 1);
 }

diff --git a/src/stdio/fclose.c b/src/stdio/fclose.c
index ee772fbb4708a6319d0911552d83ad7a27a07f66..373a2c76cc5b71b872817bf4735c7369a6bc4f82 100644 (file)
--- a/src/stdio/fclose.c
+++ b/src/stdio/fclose.c
@@ -9,7 +9,7 @@ int fclose(FILE *f)
                OFLLOCK();
                if (f->prev) f->prev->next = f->next;
                if (f->next) f->next->prev = f->prev;
-               if (ofl_head == f) ofl_head = f->next;
+               if (libc.ofl_head == f) libc.ofl_head = f->next;
                OFLUNLOCK();
        }
 

diff --git a/src/stdio/fflush.c b/src/stdio/fflush.c
index cdbd39bc3ab1ef9ef2ccaa6a9eb0c310a6f338b3..4c1647b7c9d933bf6d97c6954e45b9f68b556019 100644 (file)
--- a/src/stdio/fflush.c
+++ b/src/stdio/fflush.c
@@ -22,8 +22,8 @@ static int __fflush_unlocked(FILE *f)
 }
 
 /* stdout.c will override this if linked */
-static FILE *const __dummy = 0;
-weak_alias(__dummy, __stdout_to_flush);
+static FILE *const dummy = 0;
+weak_alias(dummy, __stdout_used);
 
 int fflush(FILE *f)
 {
@@ -37,13 +37,13 @@ int fflush(FILE *f)
                return r;
        }
 
-       r = __stdout_to_flush ? fflush(__stdout_to_flush) : 0;
+       r = __stdout_used ? fflush(__stdout_used) : 0;
 
        OFLLOCK();
-       for (f=ofl_head; f; f=next) {
+       for (f=libc.ofl_head; f; f=next) {
                FLOCK(f);
                //OFLUNLOCK();
-               r |= __fflush_unlocked(f);
+               if (f->wpos > f->wbase) r |= __fflush_unlocked(f);
                //OFLLOCK();
                next = f->next;
                FUNLOCK(f);

diff --git a/src/stdio/fgetc.c b/src/stdio/fgetc.c
index da6386842f99fc869d244f3c0b9e71c3465910ef..4d8aca371d895603653e1d06ca53c2a1f2ce726a 100644 (file)
--- a/src/stdio/fgetc.c
+++ b/src/stdio/fgetc.c
@@ -3,9 +3,10 @@
 int fgetc(FILE *f)
 {
        int c;
-       FLOCK(f);
+       if (f->lock < 0 || !__lockfile(f))
+               return getc_unlocked(f);
        c = getc_unlocked(f);
-       FUNLOCK(f);
+       __unlockfile(f);
        return c;
 }
 

diff --git a/src/stdio/fgetwc.c b/src/stdio/fgetwc.c
index 5e420594da00dbf9bce0105508f6f3ef11487f08..6f9f9ec21f8351cecf5131db636f8600364e8fab 100644 (file)
--- a/src/stdio/fgetwc.c
+++ b/src/stdio/fgetwc.c
@@ -34,7 +34,6 @@ wint_t __fgetwc_unlocked(FILE *f)
                if (l == -1) return WEOF;
        }
 
-       FUNLOCK(f);
        return wc;
 }
 

diff --git a/src/stdio/flockfile.c b/src/stdio/flockfile.c
index 0d4c92c24d9de3793f28870b60020b8bd5001d99..a196c1efe0af11663f5e0537a0de1db02c3982ef 100644 (file)
--- a/src/stdio/flockfile.c
+++ b/src/stdio/flockfile.c
@@ -3,6 +3,8 @@
 
 void flockfile(FILE *f)
 {
-       if (!libc.threaded) pthread_self();
-       __lockfile(f);
+       while (ftrylockfile(f)) {
+               int owner = f->lock;
+               if (owner) __wait(&f->lock, &f->waiters, owner, 1);
+       }
 }

diff --git a/src/stdio/fputc.c b/src/stdio/fputc.c
index 98d0a20aca814bddf897c8049aff06af9c4cf75e..6a144a5429f682fc5b79329cb309d28fc2a528e1 100644 (file)
--- a/src/stdio/fputc.c
+++ b/src/stdio/fputc.c
@@ -2,9 +2,10 @@
 
 int fputc(int c, FILE *f)
 {
-       FLOCK(f);
+       if (f->lock < 0 || !__lockfile(f))
+               return putc_unlocked(c, f);
        c = putc_unlocked(c, f);
-       FUNLOCK(f);
+       __unlockfile(f);
        return c;
 }
 

diff --git a/src/stdio/ftell.c b/src/stdio/ftell.c
index aa1f5381e9774adbc814449708465652ae990c20..3904a1d871dd9229b59ba0c97d13923dfc8d5c14 100644 (file)
--- a/src/stdio/ftell.c
+++ b/src/stdio/ftell.c
@@ -3,10 +3,8 @@
 off_t __ftello_unlocked(FILE *f)
 {
        off_t pos = f->seek(f, 0, SEEK_CUR);
-       if (pos < 0) {
-               FUNLOCK(f);
-               return pos;
-       }
+       if (pos < 0) return pos;
+
        /* Adjust for data in buffer. */
        return pos - (f->rend - f->rpos) + (f->wpos - f->wbase);
 }

diff --git a/src/stdio/ftrylockfile.c b/src/stdio/ftrylockfile.c
index 0b0e44aaf4fb6d7da1922b44e80b0926870376d1..725c4c3eacbd159dbac076a9c8e3fa4fafea22a7 100644 (file)
--- a/src/stdio/ftrylockfile.c
+++ b/src/stdio/ftrylockfile.c
@@ -5,11 +5,12 @@ int ftrylockfile(FILE *f)
 {
        int tid = pthread_self()->tid;
        if (f->lock == tid) {
-               if (f->lockcount == INT_MAX)
+               if (f->lockcount == LONG_MAX)
                        return -1;
                f->lockcount++;
                return 0;
        }
+       if (f->lock < 0) f->lock = 0;
        if (f->lock || a_cas(&f->lock, 0, tid))
                return -1;
        f->lockcount = 1;

diff --git a/src/stdio/funlockfile.c b/src/stdio/funlockfile.c
index d69f68ee8c86e32b7b27ced9088b2c75c79442b5..f8a2a071e64596b00e52940d47d94ca7ec2bee80 100644 (file)
--- a/src/stdio/funlockfile.c
+++ b/src/stdio/funlockfile.c
@@ -3,5 +3,5 @@
 
 void funlockfile(FILE *f)
 {
-       FUNLOCK(f);
+       if (!--f->lockcount) __unlockfile(f);
 }

diff --git a/src/stdio/stderr.c b/src/stdio/stderr.c
index 9a70700ceaa04b62ba88d7b97b1621df149fb6c8..3fd8f81d76ec4ed4cef8680914b822be55067831 100644 (file)
--- a/src/stdio/stderr.c
+++ b/src/stdio/stderr.c
@@ -10,5 +10,7 @@ static FILE f = {
        .write = __stdio_write,
        .seek = __stdio_seek,
        .close = __stdio_close,
+       .lock = -1,
 };
 FILE *const stderr = &f;
+FILE *const __stderr_used = &f;

diff --git a/src/stdio/stdin.c b/src/stdio/stdin.c
index 51c9923253016bd7463c8a71d9f6ffd1abb0f1f2..476dc7088f2ea46af4e95f0607f7b7eb394e3d8a 100644 (file)
--- a/src/stdio/stdin.c
+++ b/src/stdio/stdin.c
@@ -9,5 +9,7 @@ static FILE f = {
        .read = __stdio_read,
        .seek = __stdio_seek,
        .close = __stdio_close,
+       .lock = -1,
 };
 FILE *const stdin = &f;
+FILE *const __stdin_used = &f;

diff --git a/src/stdio/stdout.c b/src/stdio/stdout.c
index 552d729e48f399f3f3b6a28649a2c75691c0faf4..3855dd0be2f812906d2493ad0f47b46ee43e8233 100644 (file)
--- a/src/stdio/stdout.c
+++ b/src/stdio/stdout.c
@@ -10,8 +10,7 @@ static FILE f = {
        .write = __stdout_write,
        .seek = __stdio_seek,
        .close = __stdio_close,
+       .lock = -1,
 };
 FILE *const stdout = &f;
-
-/* overrides symbol in fflush.c, used for flushing NULL */
-FILE *const __stdout_to_flush = &f;
+FILE *const __stdout_used = &f;

diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index 856015ff01762dcc65ae12b37b06462e19987009..adef510c5298c810d09b14e8666cd0359ae8c2d3 100644 (file)
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -1,4 +1,5 @@
 #include "pthread_impl.h"
+#include "stdio_impl.h"
 
 static void dummy_0()
 {
@@ -60,6 +61,16 @@ int __uniclone(void *, int (*)(), void *);
 static const size_t dummy = 0;
 weak_alias(dummy, __pthread_tsd_size);
 
+static FILE *const dummy_file = 0;
+weak_alias(dummy_file, __stdin_used);
+weak_alias(dummy_file, __stdout_used);
+weak_alias(dummy_file, __stderr_used);
+
+static void init_file_lock(FILE *f)
+{
+       if (f && f->lock<0) f->lock = 0;
+}
+
 int pthread_create(pthread_t *res, const pthread_attr_t *attr, void *(*entry)(void *), void *arg)
 {
        int ret;
@@ -70,6 +81,11 @@ int pthread_create(pthread_t *res, const pthread_attr_t *attr, void *(*entry)(vo
 
        if (!self) return ENOSYS;
        if (!libc.threaded) {
+               for (FILE *f=libc.ofl_head; f; f=f->next)
+                       init_file_lock(f);
+               init_file_lock(__stdin_used);
+               init_file_lock(__stdout_used);
+               init_file_lock(__stderr_used);
                __syscall(SYS_rt_sigprocmask, SIG_UNBLOCK, SIGPT_SET, 0, 8);
                libc.threaded = 1;
        }

diff --git a/src/thread/pthread_key_create.c b/src/thread/pthread_key_create.c
index c9ca48ab3817f13c580d73f9120cf872b185b5bd..e51cb02325d4908a8ac91b58d6b6afde47a5af6d 100644 (file)
--- a/src/thread/pthread_key_create.c
+++ b/src/thread/pthread_key_create.c
@@ -14,7 +14,7 @@ int pthread_key_create(pthread_key_t *k, void (*dtor)(void *))
        unsigned i = (uintptr_t)&k / 16 % PTHREAD_KEYS_MAX;
        unsigned j = i;
 
-       pthread_self();
+       __pthread_self_init();
        if (!dtor) dtor = nodtor;
        do {
                if (!a_cas_p(keys+j, 0, dtor)) {

diff --git a/src/thread/pthread_self.c b/src/thread/pthread_self.c
index 55d20c9f8a9ac759a6747cac9e073a170db24c22..9f885d94a5472ce8a9b8644cf217e74f902c2881 100644 (file)
--- a/src/thread/pthread_self.c
+++ b/src/thread/pthread_self.c
@@ -35,3 +35,5 @@ pthread_t pthread_self()
        }
        return __pthread_self();
 }
+
+weak_alias(pthread_self, __pthread_self_init);