diff --git a/core/depend.mk b/core/depend.mk index 6c0955f3d..b8d8841f6 100644 --- a/core/depend.mk +++ b/core/depend.mk @@ -395,7 +395,8 @@ config.o: config.c /usr/include/stdio.h /usr/include/features.h \ ../include/service.h ../include/spinlock.h ../include/thread.h \ /usr/include/pthread.h /usr/include/sched.h /usr/include/bits/sched.h \ /usr/include/bits/setjmp.h ../include/dcb.h ../include/buffer.h \ - ../include/server.h ../include/users.h ../include/hashtable.h + ../include/server.h ../include/users.h ../include/hashtable.h \ + ../include/atomic.h users.o: users.c /usr/include/stdlib.h /usr/include/features.h \ /usr/include/sys/cdefs.h /usr/include/bits/wordsize.h \ /usr/include/gnu/stubs.h /usr/include/gnu/stubs-64.h \ @@ -408,7 +409,9 @@ users.o: users.c /usr/include/stdlib.h /usr/include/features.h \ /usr/include/bits/sigset.h /usr/include/bits/time.h \ /usr/include/sys/sysmacros.h /usr/include/bits/pthreadtypes.h \ /usr/include/alloca.h /usr/include/string.h /usr/include/xlocale.h \ - ../include/users.h ../include/hashtable.h ../include/atomic.h + ../include/users.h ../include/hashtable.h ../include/spinlock.h \ + ../include/thread.h /usr/include/pthread.h /usr/include/sched.h \ + /usr/include/bits/sched.h /usr/include/bits/setjmp.h ../include/atomic.h hashtable.o: hashtable.c /usr/include/stdlib.h /usr/include/features.h \ /usr/include/sys/cdefs.h /usr/include/bits/wordsize.h \ /usr/include/gnu/stubs.h /usr/include/gnu/stubs-64.h \ @@ -421,4 +424,6 @@ hashtable.o: hashtable.c /usr/include/stdlib.h /usr/include/features.h \ /usr/include/bits/sigset.h /usr/include/bits/time.h \ /usr/include/sys/sysmacros.h /usr/include/bits/pthreadtypes.h \ /usr/include/alloca.h /usr/include/string.h /usr/include/xlocale.h \ - ../include/hashtable.h + ../include/hashtable.h ../include/spinlock.h ../include/thread.h \ + /usr/include/pthread.h /usr/include/sched.h /usr/include/bits/sched.h \ + /usr/include/bits/setjmp.h ../include/atomic.h diff --git a/core/hashtable.c b/core/hashtable.c index 52b15a802..2455d1281 100644 --- a/core/hashtable.c +++ b/core/hashtable.c @@ -22,6 +22,29 @@ /** * @file hashtable.c General purpose hashtable routines * + * The hashtable can be create with a custom number of hash buckets, + * a hash function and optional functions to call make copies of the key + * and value and to free them. + * + * The hashtable is arrange as a set of linked lists, the number of linked + * lists beign the hashsize as requested by the user. Entries are hashed by + * calling the hash function that is passed in by the user, this is used as + * an index into the array of linked lists, usign modulo hashsize. + * + * The linked lists are searched using the key comparison function that is + * passed into the hash table creation routine. + * + * By default the hash table keeps the original pointers that are passed in + * for the keys and values, however two functions can be supplied to copy these + * a copy function and a free function. Please note the same function is used for + * the key and the value, if the actions required are different the called functions + * must understand how to differenate the key and value. + * + * The hash table implements a single write, multiple reader locking policy by + * using a pair of counters and a spinlock. The spinlock is used to protect the + * number of readers and writers counters when taking out locks. Releasing of + * locks uses pure atomic actions and thus does not require spinlock protection. + * * @verbatim * Revision History * @@ -31,9 +54,14 @@ * @endverbatim */ +static void hashtable_read_lock(HASHTABLE *table); +static void hashtable_read_unlock(HASHTABLE *table); +static void hashtable_write_lock(HASHTABLE *table); +static void hashtable_write_unlock(HASHTABLE *table); + /** * Special null function used as default memory allfunctions in the hashtable - * implementation. This avoids havign to special case the code that manipulates + * implementation. This avoids having to special case the code that manipulates * the keys and values * * @param data The data pointer @@ -65,6 +93,9 @@ HASHTABLE *rval; rval->cmpfn = cmpfn; rval->copyfn = nullfn; rval->freefn = nullfn; + rval->n_readers = 0; + rval->writelock = 0; + spinlock_init(&rval->spin); if ((rval->entries = calloc(size, sizeof(HASHENTRIES))) == NULL) { free(rval); @@ -86,6 +117,7 @@ hashtable_free(HASHTABLE *table) int i; HASHENTRIES *entry, *ptr; + hashtable_write_lock(table); for (i = 0; i < table->hashsize; i++) { entry = table->entries[i]; @@ -135,9 +167,10 @@ hashtable_memory_fns(HASHTABLE *table, HASHMEMORYFN copyfn, HASHMEMORYFN freefn) int hashtable_add(HASHTABLE *table, void *key, void *value) { -int hashkey = table->hashfn(key); +int hashkey = table->hashfn(key) % table->hashsize; HASHENTRIES *entry; + hashtable_write_lock(table); entry = table->entries[hashkey % table->hashsize]; while (entry->next && entry->key && table->cmpfn(key, entry->key) != 0) { @@ -152,18 +185,23 @@ HASHENTRIES *entry; else if (table->cmpfn(key, entry->key) == 0) { /* Duplicate key value */ + hashtable_write_unlock(table); return 0; } else { HASHENTRIES *ptr = (HASHENTRIES *)malloc(sizeof(HASHENTRIES)); if (ptr == NULL) + { + hashtable_write_unlock(table); return 0; + } ptr->key = table->copyfn(key); ptr->value = table->copyfn(value); ptr->next = NULL; entry->next = ptr; } + hashtable_write_unlock(table); return 1; } @@ -177,9 +215,10 @@ HASHENTRIES *entry; int hashtable_delete(HASHTABLE *table, void *key) { -int hashkey = table->hashfn(key); +int hashkey = table->hashfn(key) % table->hashsize; HASHENTRIES *entry, *ptr; + hashtable_write_lock(table); entry = table->entries[hashkey % table->hashsize]; while (entry && entry->key && table->cmpfn(key, entry->key) != 0) { @@ -188,6 +227,7 @@ HASHENTRIES *entry, *ptr; if (entry == NULL || entry->key == NULL) { /* Not found */ + hashtable_write_unlock(table); return 0; } @@ -218,12 +258,16 @@ HASHENTRIES *entry, *ptr; while (ptr && ptr->next != entry) ptr = ptr->next; if (ptr == NULL) + { + hashtable_write_unlock(table); return 0; /* This should never happen */ + } ptr->next = entry->next; table->freefn(entry->key); table->freefn(entry->value); free(entry); } + hashtable_write_unlock(table); return 1; } @@ -237,9 +281,10 @@ HASHENTRIES *entry, *ptr; void * hashtable_fetch(HASHTABLE *table, void *key) { -int hashkey = table->hashfn(key); +int hashkey = table->hashfn(key) % table->hashsize; HASHENTRIES *entry; + hashtable_read_lock(table); entry = table->entries[hashkey % table->hashsize]; while (entry && entry->key && table->cmpfn(key, entry->key) != 0) { @@ -247,11 +292,102 @@ HASHENTRIES *entry; } if (entry == NULL || entry->key == NULL) { + hashtable_read_unlock(table); return NULL; } else { + hashtable_read_unlock(table); return entry->value; } } + +/** + * Take a read lock on the hashtable. + * + * The hashtable support multiple readers and a single writer, + * we have a spinlock to protect the two counts, n_readers and + * writelock. + * + * We take the hashtable spinlock and then check that writelock + * is set to zero. If not we release the spinlock and do dirty + * reads of writelock until it goes to 0. Once it is zero we + * acquire the spinlock again and test that writelock is still + * 0. + * + * With writelock set to zero we increment n_readers with the + * spinlock still held. + * + * @param table The hashtable to lock. + */ +static void +hashtable_read_lock(HASHTABLE *table) +{ + spinlock_acquire(&table->spin); + while (table->writelock) + { + spinlock_release(&table->spin); + while (table->writelock) + ; + spinlock_acquire(&table->spin); + } + table->n_readers++; + spinlock_release(&table->spin); +} + +/** + * Release a previously obtained readlock. + * + * Simply decrement the n_readers value for the hash table + * + * @param table The hash table to unlock + */ +static void +hashtable_read_unlock(HASHTABLE *table) +{ + atomic_add(&table->n_readers, -1); +} + +/** + * Obtain an exclusive write lock for the hash table. + * + * We acquire the hashtable spinlock, check for the number of + * readers beign zero. If it is not we hold the spinlock and + * loop waiting for the n_readers to reach zero. This will prevent + * any new readers beign granted access but will not prevent current + * readers releasing the read lock. + * + * Once we have no readers we increment writelock and test if we are + * the only writelock holder, if not we repeat the process. We hold + * the spinlock throughout the process since both read and write + * locks do not require the spinlock to be acquired. + * + * @param table The table to lock for updates + */ +static void +hashtable_write_lock(HASHTABLE *table) +{ +int available; + + spinlock_acquire(&table->spin); + do { + while (table->n_readers) + ; + available = atomic_add(&table->writelock, 1); + if (available != 0) + atomic_add(&table->writelock, -1); + } while (available != 0); + spinlock_release(&table->spin); +} + +/** + * Release the write lock on the hash table. + * + * @param table The hash table to unlock + */ +static void +hashtable_write_unlock(HASHTABLE *table) +{ + atomic_add(&table->writelock, -1); +} diff --git a/include/hashtable.h b/include/hashtable.h index 2c04ff064..b14764978 100644 --- a/include/hashtable.h +++ b/include/hashtable.h @@ -30,6 +30,8 @@ * * @endverbatim */ +#include +#include /** * The entries within a hashtable. @@ -58,6 +60,9 @@ typedef struct hashtable { int (*cmpfn)(void *, void *); /**< The key comparison function */ HASHMEMORYFN copyfn; /**< Optional copy function */ HASHMEMORYFN freefn; /**< Optional free function */ + SPINLOCK spin; /**< Internal spinlock for the hashtable */ + int n_readers; /**< Number of clients reading the table */ + int writelock; /**< The table is locked by a writer */ } HASHTABLE; extern HASHTABLE *hashtable_alloc(int, int (*hashfn)(), int (*cmpfn)()); diff --git a/include/spinlock.h b/include/spinlock.h index 66706d3fd..5957a0f47 100644 --- a/include/spinlock.h +++ b/include/spinlock.h @@ -31,7 +31,7 @@ #include typedef struct spinlock { - int lock; + volatile int lock; #if DEBUG int spins; int acquired; diff --git a/include/users.h b/include/users.h index 4b9a4f86f..4f209ba4a 100644 --- a/include/users.h +++ b/include/users.h @@ -18,6 +18,7 @@ * Copyright SkySQL Ab 2013 */ #include + /** * @file users.h The functions to manipulate the table of users maintained * for each service @@ -40,6 +41,7 @@ typedef struct { int n_deletes; /**< The number of deletes */ int n_fetches; /**< The number of fetchs */ } USERS_STATS; + /** * The user table, this contains the username and authentication data required * for the authentication implementation within the gateway.