libc: speed-up flockfile()/funlockfile()
For Honeycomb, we added proper file thread-safety for
all FILE* operations. However, we did implement that by
using an out-of-band hash table to map FILE* pointers
to phtread_mutex_t mutexes, because we couldn't change
the size of 'struct _sFILE' without breaking the ABI.
It turns out that our BSD-derived code already has
some support code to extend FILE* objects, so use it
instead. See libc/stdio/fileext.h
This patch gets rid of the hash table, and put the
mutex directly into the sFILE extension.
Change-Id: If1c3fe0a0a89da49c568e9a7560b7827737ff4d0
diff --git a/libc/include/pthread.h b/libc/include/pthread.h
index 9d05769..2015ac0 100644
--- a/libc/include/pthread.h
+++ b/libc/include/pthread.h
@@ -42,9 +42,13 @@
int volatile value;
} pthread_mutex_t;
-#define PTHREAD_MUTEX_INITIALIZER {0}
-#define PTHREAD_RECURSIVE_MUTEX_INITIALIZER {0x4000}
-#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER {0x8000}
+#define __PTHREAD_MUTEX_INIT_VALUE 0
+#define __PTHREAD_RECURSIVE_MUTEX_INIT_VALUE 0x4000
+#define __PTHREAD_ERRORCHECK_MUTEX_INIT_VALUE 0x8000
+
+#define PTHREAD_MUTEX_INITIALIZER {__PTHREAD_MUTEX_INIT_VALUE}
+#define PTHREAD_RECURSIVE_MUTEX_INITIALIZER {__PTHREAD_RECURSIVE_MUTEX_INIT_VALUE}
+#define PTHREAD_ERRORCHECK_MUTEX_INITIALIZER {__PTHREAD_ERRORCHECK_MUTEX_INIT_VALUE}
enum {
PTHREAD_MUTEX_NORMAL = 0,
diff --git a/libc/stdio/fileext.h b/libc/stdio/fileext.h
index 2d07043..b36a448 100644
--- a/libc/stdio/fileext.h
+++ b/libc/stdio/fileext.h
@@ -29,24 +29,41 @@
* $Citrus$
*/
+#include <pthread.h>
+#include "wcio.h"
+
/*
* file extension
*/
struct __sfileext {
struct __sbuf _ub; /* ungetc buffer */
struct wchar_io_data _wcio; /* wide char io status */
+ pthread_mutex_t _lock; /* file lock */
};
+#define _FILEEXT_INITIALIZER {{NULL,0},{0},PTHREAD_RECURSIVE_MUTEX_INITIALIZER}
+
#define _EXT(fp) ((struct __sfileext *)((fp)->_ext._base))
#define _UB(fp) _EXT(fp)->_ub
+#define _FLOCK(fp) _EXT(fp)->_lock
#define _FILEEXT_INIT(fp) \
do { \
_UB(fp)._base = NULL; \
_UB(fp)._size = 0; \
WCIO_INIT(fp); \
+ _FLOCK_INIT(fp); \
} while (0)
+/* Helper macros to avoid a function call when you know that fp is not NULL.
+ * Notice that we keep _FLOCK_INIT() fast by slightly breaking our pthread
+ * encapsulation.
+ */
+#define _FLOCK_INIT(fp) _FLOCK(fp).value = __PTHREAD_RECURSIVE_MUTEX_INIT_VALUE
+#define _FLOCK_LOCK(fp) pthread_mutex_lock(&_FLOCK(fp))
+#define _FLOCK_TRYLOCK(fp) pthread_mutex_trylock(&_FLOCK(fp))
+#define _FLOCK_UNLOCK(fp) pthread_mutex_unlock(&_FLOCK(fp))
+
#define _FILEEXT_SETUP(f, fext) \
do { \
(f)->_ext._base = (unsigned char *)(fext); \
diff --git a/libc/stdio/findfp.c b/libc/stdio/findfp.c
index a659c87..76ed5ee 100644
--- a/libc/stdio/findfp.c
+++ b/libc/stdio/findfp.c
@@ -58,7 +58,12 @@
static struct glue *lastglue = &uglue;
_THREAD_PRIVATE_MUTEX(__sfp_mutex);
-static struct __sfileext __sFext[3];
+static struct __sfileext __sFext[3] = {
+ _FILEEXT_INITIALIZER,
+ _FILEEXT_INITIALIZER,
+ _FILEEXT_INITIALIZER,
+};
+
FILE __sF[3] = {
std(__SRD, STDIN_FILENO), /* stdin */
std(__SWR, STDOUT_FILENO), /* stdout */
diff --git a/libc/stdio/flockfile.c b/libc/stdio/flockfile.c
index e8c74c5..368fb15 100644
--- a/libc/stdio/flockfile.c
+++ b/libc/stdio/flockfile.c
@@ -31,122 +31,23 @@
* we can't use the OpenBSD implementation which uses kernel-specific
* APIs not available on Linux.
*
- * Ideally, this would be trivially implemented by adding a
- * pthread_mutex_t field to struct __sFILE as defined in
- * <stdio.h>.
- *
- * However, since we don't want to bring pthread into the mix
- * as well as change the size of a public API/ABI structure,
- * we're going to store the data out-of-band.
- *
- * we use a hash-table to map FILE* pointers to recursive mutexes
- * fclose() will call __fremovelock() defined below to remove
- * a pointer from the table.
+ * Instead, we use a pthread_mutex_t within the FILE* internal state.
+ * See fileext.h for details.
*
* the behaviour, if fclose() is called while the corresponding
* file is locked is totally undefined.
*/
#include <stdio.h>
-#include <pthread.h>
#include <string.h>
+#include <errno.h>
+#include "fileext.h"
-/* a node in the hash table */
-typedef struct FileLock {
- struct FileLock* next;
- FILE* file;
- pthread_mutex_t mutex;
-} FileLock;
-
-/* use a static hash table. We assume that we're not going to
- * lock a really large number of FILE* objects on an embedded
- * system.
- */
-#define FILE_LOCK_BUCKETS 32
-
-typedef struct {
- pthread_mutex_t lock;
- FileLock* buckets[ FILE_LOCK_BUCKETS ];
-} LockTable;
-
-static LockTable* _lockTable;
-static pthread_once_t _lockTable_once = PTHREAD_ONCE_INIT;
-
-static void
-lock_table_init( void )
-{
- _lockTable = malloc(sizeof(*_lockTable));
- if (_lockTable != NULL) {
- pthread_mutex_init(&_lockTable->lock, NULL);
- memset(_lockTable->buckets, 0, sizeof(_lockTable->buckets));
- }
-}
-
-static LockTable*
-lock_table_lock( void )
-{
- pthread_once( &_lockTable_once, lock_table_init );
- pthread_mutex_lock( &_lockTable->lock );
- return _lockTable;
-}
-
-static void
-lock_table_unlock( LockTable* t )
-{
- pthread_mutex_unlock( &t->lock );
-}
-
-static FileLock**
-lock_table_lookup( LockTable* t, FILE* f )
-{
- uint32_t hash = (uint32_t)(void*)f;
- FileLock** pnode;
-
- hash = (hash >> 2) ^ (hash << 17);
- pnode = &t->buckets[hash % FILE_LOCK_BUCKETS];
- for (;;) {
- FileLock* node = *pnode;
- if (node == NULL || node->file == f)
- break;
- pnode = &node->next;
- }
- return pnode;
-}
void
flockfile(FILE * fp)
{
- LockTable* t = lock_table_lock();
-
- if (t != NULL) {
- FileLock** lookup = lock_table_lookup(t, fp);
- FileLock* lock = *lookup;
-
- if (lock == NULL) {
- pthread_mutexattr_t attr;
-
- /* create a new node in the hash table */
- lock = malloc(sizeof(*lock));
- if (lock == NULL) {
- lock_table_unlock(t);
- return;
- }
- lock->next = NULL;
- lock->file = fp;
-
- pthread_mutexattr_init(&attr);
- pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
- pthread_mutex_init( &lock->mutex, &attr );
-
- *lookup = lock;
- }
- lock_table_unlock(t);
-
- /* we assume that another thread didn't destroy 'lock'
- * by calling fclose() on the FILE*. This can happen if
- * the client is *really* buggy, but we don't care about
- * such code here.
- */
- pthread_mutex_lock(&lock->mutex);
+ if (fp != NULL) {
+ _FLOCK_LOCK(fp);
}
}
@@ -154,21 +55,13 @@
int
ftrylockfile(FILE *fp)
{
- int ret = -1;
- LockTable* t = lock_table_lock();
+ /* The specification for ftrylockfile() says it returns 0 on success,
+ * or non-zero on error. So return an errno code directly on error.
+ */
+ int ret = EINVAL;
- if (t != NULL) {
- FileLock** lookup = lock_table_lookup(t, fp);
- FileLock* lock = *lookup;
-
- lock_table_unlock(t);
-
- /* see above comment about why we assume that 'lock' can
- * be accessed from here
- */
- if (lock != NULL && !pthread_mutex_trylock(&lock->mutex)) {
- ret = 0; /* signal success */
- }
+ if (fp != NULL) {
+ ret = _FLOCK_TRYLOCK(fp);
}
return ret;
}
@@ -176,35 +69,7 @@
void
funlockfile(FILE * fp)
{
- LockTable* t = lock_table_lock();
-
- if (t != NULL) {
- FileLock** lookup = lock_table_lookup(t, fp);
- FileLock* lock = *lookup;
-
- if (lock != NULL)
- pthread_mutex_unlock(&lock->mutex);
-
- lock_table_unlock(t);
- }
-}
-
-
-/* called from fclose() to remove the file lock */
-__LIBC_HIDDEN__ void
-__fremovelock(FILE* fp)
-{
- LockTable* t = lock_table_lock();
-
- if (t != NULL) {
- FileLock** lookup = lock_table_lookup(t, fp);
- FileLock* lock = *lookup;
-
- if (lock != NULL) {
- *lookup = lock->next;
- lock->file = NULL;
- }
- lock_table_unlock(t);
- free(lock);
+ if (fp != NULL) {
+ _FLOCK_UNLOCK(fp);
}
}