From 3fd89450610679c6b777767632d2af5751773295 Mon Sep 17 00:00:00 2001 From: sr Date: Wed, 17 Jul 2013 12:10:01 +0200 Subject: Replace all pthread_spin_* calls with spin_*, so that all locking can be tracked and debugged Fix compilation of kernel module --- src/kernel/dnbd3.h | 1 + src/kernel/serialize_kmod.c | 1 + src/server/image.c | 47 ++++++++++++++++--------------- src/server/locks.c | 22 +++++++++++++-- src/server/locks.h | 4 +-- src/server/memlog.c | 11 ++++---- src/server/net.c | 5 ++-- src/server/server.c | 68 +++++++++++++++++++++++++++------------------ src/server/uplink.c | 5 ++-- src/types.h | 2 ++ 10 files changed, 102 insertions(+), 64 deletions(-) diff --git a/src/kernel/dnbd3.h b/src/kernel/dnbd3.h index e7911cf..915c8b9 100644 --- a/src/kernel/dnbd3.h +++ b/src/kernel/dnbd3.h @@ -27,6 +27,7 @@ #include #include +#define KERNEL_MODULE #include "config.h" #include "types.h" #include "serialize.h" diff --git a/src/kernel/serialize_kmod.c b/src/kernel/serialize_kmod.c index a6a9b03..50746df 100644 --- a/src/kernel/serialize_kmod.c +++ b/src/kernel/serialize_kmod.c @@ -1,4 +1,5 @@ #include #include +#define KERNEL_MODULE #include "serialize.c" diff --git a/src/server/image.c b/src/server/image.c index 4d066ad..8a01c9a 100644 --- a/src/server/image.c +++ b/src/server/image.c @@ -2,6 +2,7 @@ #include "helper.h" #include "memlog.h" #include "uplink.h" +#include "locks.h" #include #include @@ -101,7 +102,7 @@ dnbd3_image_t* image_get(char *name, uint16_t revision) // Always use lowercase name strtolower( name ); // Go through array - pthread_spin_lock( &_images_lock ); + spin_lock( &_images_lock ); for (i = 0; i < _num_images; ++i) { dnbd3_image_t * const image = _images[i]; if ( image == NULL || strcmp( image->lower_name, name ) != 0 ) continue; @@ -114,12 +115,12 @@ dnbd3_image_t* image_get(char *name, uint16_t revision) } if ( candidate == NULL ) { - pthread_spin_unlock( &_images_lock ); + spin_unlock( &_images_lock ); return NULL ; } - pthread_spin_lock( &candidate->lock ); - pthread_spin_unlock( &_images_lock ); + spin_lock( &candidate->lock ); + spin_unlock( &_images_lock ); // Found, see if it works struct stat st; @@ -128,7 +129,7 @@ dnbd3_image_t* image_get(char *name, uint16_t revision) candidate->working = FALSE; // No file? OUT! } candidate->users++; - pthread_spin_unlock( &candidate->lock ); + spin_unlock( &candidate->lock ); return candidate; // Success :-) } @@ -141,30 +142,30 @@ dnbd3_image_t* image_get(char *name, uint16_t revision) void image_release(dnbd3_image_t *image) { assert( image != NULL ); - pthread_spin_lock( &image->lock ); + spin_lock( &image->lock ); assert( image->users > 0 ); image->users--; if ( image->users > 0 ) { // Still in use, do nothing - pthread_spin_unlock( &image->lock ); + spin_unlock( &image->lock ); return; } - pthread_spin_unlock( &image->lock ); - pthread_spin_lock( &_images_lock ); - pthread_spin_lock( &image->lock ); + spin_unlock( &image->lock ); + spin_lock( &_images_lock ); + spin_lock( &image->lock ); // Check active users again as we unlocked if ( image->users == 0 ) { // Not in use anymore, see if it's in the images array for (int i = 0; i < _num_images; ++i) { if ( _images[i] == image ) { // Found, do nothing - pthread_spin_unlock( &image->lock ); - pthread_spin_unlock( &_images_lock ); + spin_unlock( &image->lock ); + spin_unlock( &_images_lock ); return; } } } // Not found, free - pthread_spin_unlock( &image->lock ); - pthread_spin_unlock( &_images_lock ); + spin_unlock( &image->lock ); + spin_unlock( &_images_lock ); image_free( image ); } @@ -174,16 +175,16 @@ void image_release(dnbd3_image_t *image) */ void image_remove(dnbd3_image_t *image) { - pthread_spin_lock( &_images_lock ); - pthread_spin_lock( &image->lock ); + spin_lock( &_images_lock ); + spin_lock( &image->lock ); for (int i = _num_images - 1; i >= 0; --i) { if ( _images[i] != image ) continue; _images[i] = NULL; if ( i + 1 == _num_images ) _num_images--; } - pthread_spin_unlock( &image->lock ); + spin_unlock( &image->lock ); if ( image->users <= 0 ) image = image_free( image ); - pthread_spin_unlock( &_images_lock ); + spin_unlock( &_images_lock ); } /** @@ -402,7 +403,7 @@ static int image_try_load(char *base, char *path) image->atime = time( NULL ); } image->working = (image->cache_map == NULL ); - pthread_spin_init( &image->lock, PTHREAD_PROCESS_PRIVATE ); + spin_init( &image->lock, PTHREAD_PROCESS_PRIVATE ); // Get rid of cache map if image is complete if ( image->cache_map != NULL && image_is_complete( image ) ) { remove( mapFile ); @@ -414,7 +415,7 @@ static int image_try_load(char *base, char *path) cache_map = NULL; crc32list = NULL; // Add to images array - pthread_spin_lock( &_images_lock ); + spin_lock( &_images_lock ); for (i = 0; i < _num_images; ++i) { if ( _images[i] != NULL ) continue; _images[i] = image; @@ -423,14 +424,14 @@ static int image_try_load(char *base, char *path) if ( i >= _num_images ) { if ( _num_images >= SERVER_MAX_IMAGES ) { memlogf( "[ERROR] Cannot load image '%s': maximum number of images reached.", path ); - pthread_spin_unlock( &_images_lock ); + spin_unlock( &_images_lock ); image = image_free( image ); goto load_error; } _images[_num_images++] = image; printf( "[DEBUG] Loaded image '%s'\n", image->lower_name ); } - pthread_spin_unlock( &_images_lock ); + spin_unlock( &_images_lock ); function_return = TRUE; // Clean exit: load_error: ; @@ -480,7 +481,7 @@ static dnbd3_image_t* image_free(dnbd3_image_t *image) free( image->path ); free( image->lower_name ); uplink_shutdown( image->uplink ); - pthread_spin_destroy( &image->lock ); + spin_destroy( &image->lock ); // memset( image, 0, sizeof(dnbd3_image_t) ); free( image ); diff --git a/src/server/locks.c b/src/server/locks.c index 5a68747..050fa4e 100644 --- a/src/server/locks.c +++ b/src/server/locks.c @@ -15,6 +15,7 @@ #include #include #include "globals.h" +#include "memlog.h" #define MAXLOCKS 500 #define MAXTHREADS 500 @@ -44,6 +45,9 @@ static debug_thread_t threads[MAXTHREADS]; static int init_done = 0; static pthread_spinlock_t initdestory; static volatile int lockId = 0; +static pthread_t watchdog = 0; + +static void *debug_thread_watchdog(void *something); int debug_spin_init(const char *name, const char *file, int line, pthread_spinlock_t *lock, int shared) { @@ -60,7 +64,7 @@ int debug_spin_init(const char *name, const char *file, int line, pthread_spinlo printf( "[ERROR] Lock %p (%s) already initialized (%s:%d)\n", lock, name, file, line ); exit( 4 ); } - if ( first == -1 ) first = i; + if ( first == -1 && locks[i].lock == NULL ) first = i; } if ( first == -1 ) { printf( "[ERROR] No more free debug locks (%s:%d)\n", file, line ); @@ -87,6 +91,7 @@ int debug_spin_lock(const char *name, const char *file, int line, pthread_spinlo pthread_spin_unlock( &initdestory ); if ( l == NULL ) { printf( "[ERROR] Tried to lock uninitialized lock %p (%s) at %s:%d\n", lock, name, file, line ); + debug_dump_lock_stats(); exit( 4 ); } debug_thread_t *t = NULL; @@ -98,11 +103,13 @@ int debug_spin_lock(const char *name, const char *file, int line, pthread_spinlo snprintf( threads[i].name, LOCKLEN, "%s", name ); snprintf( threads[i].where, LOCKLEN, "%s:%d", file, line ); t = &threads[i]; + break; } pthread_spin_unlock( &initdestory ); int retval = pthread_spin_lock( lock ); pthread_spin_lock( &initdestory ); t->tid = 0; + t->time = 0; pthread_spin_unlock( &initdestory ); if ( l->locked ) { printf( "[ERROR] Lock sanity check: lock %p (%s) already locked at %s:%d\n", lock, name, file, line ); @@ -145,7 +152,7 @@ int debug_spin_unlock(const char *name, const char *file, int line, pthread_spin return retval; } -int debug_spin_destory(const char *name, const char *file, int line, pthread_spinlock_t *lock) +int debug_spin_destroy(const char *name, const char *file, int line, pthread_spinlock_t *lock) { pthread_spin_lock( &initdestory ); for (int i = 0; i < MAXLOCKS; ++i) { @@ -196,7 +203,7 @@ void debug_dump_lock_stats() pthread_spin_unlock( &initdestory ); } -void *debug_thread_watchdog(void *something) +static void *debug_thread_watchdog(void *something) { while (!_shutdown) { time_t now = time(NULL); @@ -217,4 +224,13 @@ void *debug_thread_watchdog(void *something) return NULL; } +void debug_locks_start_watchdog() +{ + if ( 0 != pthread_create( &watchdog, NULL, &debug_thread_watchdog, (void *)NULL ) ) { + memlogf( "[ERROR] Could not start debug-lock watchdog." ); + return; + } + pthread_detach( watchdog ); +} + #endif diff --git a/src/server/locks.h b/src/server/locks.h index 9a0a024..6558558 100644 --- a/src/server/locks.h +++ b/src/server/locks.h @@ -13,10 +13,10 @@ int debug_spin_init(const char *name, const char *file, int line, pthread_spinlock_t *lock, int shared); int debug_spin_lock(const char *name, const char *file, int line, pthread_spinlock_t *lock); int debug_spin_unlock(const char *name, const char *file, int line, pthread_spinlock_t *lock); -int debug_spin_destory(const char *name, const char *file, int line, pthread_spinlock_t *lock); +int debug_spin_destroy(const char *name, const char *file, int line, pthread_spinlock_t *lock); +void debug_locks_start_watchdog(); void debug_dump_lock_stats(); -void *debug_thread_watchdog(void *something); #else diff --git a/src/server/memlog.c b/src/server/memlog.c index 6d4ec09..f159d96 100644 --- a/src/server/memlog.c +++ b/src/server/memlog.c @@ -25,6 +25,7 @@ #include #include #include +#include "locks.h" #define MAX(a,b) (a > b ? a : b) @@ -48,7 +49,7 @@ void initmemlog() { // Use main spinlock to make sure we really init only once if (logBuffer) return; - pthread_spin_init(&logLock, PTHREAD_PROCESS_PRIVATE); + spin_init(&logLock, PTHREAD_PROCESS_PRIVATE); logBuffer = (LogLine *)calloc(LINE_COUNT, sizeof(LogLine)); } @@ -61,7 +62,7 @@ void memlogf(const char *fmt, ...) struct tm *timeinfo; time(&rawtime); timeinfo = localtime(&rawtime); - pthread_spin_lock(&logLock); + spin_lock(&logLock); LogLine *const line = (LogLine *)&(logBuffer[bufferPos % LINE_COUNT]); const size_t offset = strftime(line->text, LINE_LEN, "[%d.%m. %H:%M:%S] ", timeinfo); if (offset == 0) *line->text = '\0'; @@ -75,7 +76,7 @@ void memlogf(const char *fmt, ...) // so to be safe either way, let strlen do the job line->len = strlen(line->text); if (ret > 0 || line->len > 0) ++bufferPos; - pthread_spin_unlock(&logLock); + spin_unlock(&logLock); puts(line->text); } @@ -86,7 +87,7 @@ char *fetchlog(int maxlines) const int start = MAX(0, bufferPos - maxlines); int len = 1, i; //printf("Outputting log from %d to %d\n", start, bufferPos); - pthread_spin_lock(&logLock); + spin_lock(&logLock); // Determine required buffer space for all log lines for (i = start; i < bufferPos; ++i) { @@ -113,6 +114,6 @@ char *fetchlog(int maxlines) } *pos = '\0'; endFunction: - pthread_spin_unlock(&logLock); + spin_unlock(&logLock); return retval; } diff --git a/src/server/net.c b/src/server/net.c index 214f1cd..4efdc00 100644 --- a/src/server/net.c +++ b/src/server/net.c @@ -36,6 +36,7 @@ #include "memlog.h" #include "../serialize.h" #include "../config.h" +#include "locks.h" static inline char recv_request_header(int sock, dnbd3_request_t *request) { @@ -347,12 +348,12 @@ void *net_client_handler(void *dnbd3_client) // Check for messages that have been queued from another thread while ( client->sendqueue != NULL ) { dnbd3_binstring_t *message = NULL; - pthread_spin_lock( &client->lock ); + spin_lock( &client->lock ); if ( client->sendqueue != NULL ) { message = client->sendqueue->data; client->sendqueue = g_slist_remove( client->sendqueue, message ); } - pthread_spin_unlock( &client->lock ); + spin_unlock( &client->lock ); send_data( client->sock, message->data, message->len ); free( message ); } diff --git a/src/server/server.c b/src/server/server.c index 19dd18d..a4aaa1c 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -32,6 +32,7 @@ #include "../types.h" #include "../version.h" +#include "locks.h" #include "sockhelper.h" #include "server.h" #include "image.h" @@ -57,6 +58,7 @@ static int dnbd3_add_client(dnbd3_client_t *client); static void dnbd3_load_config(); static void dnbd3_handle_sigpipe(int signum); static void dnbd3_handle_sigterm(int signum); +static void dnbd3_handle_sigusr1(int signum); /** * Print help text for usage instructions @@ -67,7 +69,7 @@ void dnbd3_print_help(char *argv_0) printf( "Start the DNBD3 server\n" ); printf( "-f or --file Configuration file (default /etc/dnbd3-server.conf)\n" ); #ifdef _DEBUG - printf("-d or --delay Add a fake network delay of X µs\n"); + printf( "-d or --delay Add a fake network delay of X µs\n" ); #endif printf( "-n or --nodaemon Start server in foreground\n" ); printf( "-r or --reload Reload configuration file\n" ); @@ -104,24 +106,24 @@ void dnbd3_cleanup() socket_count = 0; // Clean up clients - pthread_spin_lock( &_clients_lock ); + spin_lock( &_clients_lock ); for (i = 0; i < _num_clients; ++i) { dnbd3_client_t * const client = _clients[i]; - pthread_spin_lock( &client->lock ); + spin_lock( &client->lock ); if ( client->sock >= 0 ) shutdown( client->sock, SHUT_RDWR ); if ( client->thread != 0 ) pthread_join( client->thread, NULL ); _clients[i] = NULL; - pthread_spin_unlock( &client->lock ); + spin_unlock( &client->lock ); free( client ); } _num_clients = 0; - pthread_spin_unlock( &_clients_lock ); + spin_unlock( &_clients_lock ); // Clean up images - pthread_spin_lock( &_images_lock ); + spin_lock( &_images_lock ); for (i = 0; i < _num_images; ++i) { dnbd3_image_t *image = _images[i]; - pthread_spin_lock( &image->lock ); + spin_lock( &image->lock ); // save cache maps to files image_save_cache_map( image ); // free uplink connection @@ -131,11 +133,11 @@ void dnbd3_cleanup() free( image->path ); free( image->lower_name ); _images[i] = NULL; - pthread_spin_unlock( &image->lock ); + spin_unlock( &image->lock ); free( image ); } _num_images = 0; - pthread_spin_unlock( &_images_lock ); + spin_unlock( &_images_lock ); exit( EXIT_SUCCESS ); } @@ -162,7 +164,7 @@ int main(int argc, char *argv[]) break; case 'd': #ifdef _DEBUG - _fake_delay = atoi(optarg); + _fake_delay = atoi( optarg ); break; #else printf( "This option is only available in debug builds.\n\n" ); @@ -198,12 +200,16 @@ int main(int argc, char *argv[]) if ( demonize ) daemon( 1, 0 ); - _basePath = strdup("/home/sr/vmware/"); + _basePath = strdup( "/home/sr/vmware/" ); _vmdkLegacyMode = TRUE; - pthread_spin_init( &_clients_lock, PTHREAD_PROCESS_PRIVATE ); - pthread_spin_init( &_images_lock, PTHREAD_PROCESS_PRIVATE ); - pthread_spin_init( &_alts_lock, PTHREAD_PROCESS_PRIVATE ); + spin_init( &_clients_lock, PTHREAD_PROCESS_PRIVATE ); + spin_init( &_images_lock, PTHREAD_PROCESS_PRIVATE ); + spin_init( &_alts_lock, PTHREAD_PROCESS_PRIVATE ); + +#ifdef _DEBUG + debug_locks_start_watchdog(); +#endif initmemlog(); memlogf( "DNBD3 server starting.... Machine type: " ENDIAN_MODE ); @@ -215,10 +221,12 @@ int main(int argc, char *argv[]) signal( SIGPIPE, dnbd3_handle_sigpipe ); signal( SIGTERM, dnbd3_handle_sigterm ); signal( SIGINT, dnbd3_handle_sigterm ); + signal( SIGUSR1, dnbd3_handle_sigusr1 ); + printf( "Loading images....\n" ); // Load all images in base path - if (!image_load_all(NULL)) { - printf("[ERROR] Could not load images.\n"); + if ( !image_load_all( NULL ) ) { + printf( "[ERROR] Could not load images.\n" ); return EXIT_FAILURE; } @@ -310,7 +318,7 @@ dnbd3_client_t* dnbd3_init_client(struct sockaddr_storage *client, int fd) return NULL ; } dnbd3_client->sock = fd; - pthread_spin_init( &dnbd3_client->lock, PTHREAD_PROCESS_PRIVATE ); + spin_init( &dnbd3_client->lock, PTHREAD_PROCESS_PRIVATE ); return dnbd3_client; } @@ -321,13 +329,13 @@ dnbd3_client_t* dnbd3_init_client(struct sockaddr_storage *client, int fd) void dnbd3_remove_client(dnbd3_client_t *client) { int i; - pthread_spin_lock( &_clients_lock ); + spin_lock( &_clients_lock ); for (i = _num_clients - 1; i >= 0; --i) { if ( _clients[i] != client ) continue; _clients[i] = NULL; if ( i + 1 == _num_clients ) --_num_clients; } - pthread_spin_unlock( &_clients_lock ); + spin_unlock( &_clients_lock ); } /** @@ -338,7 +346,7 @@ void dnbd3_remove_client(dnbd3_client_t *client) dnbd3_client_t* dnbd3_free_client(dnbd3_client_t *client) { GSList *it; - pthread_spin_lock(&client->lock); + spin_lock( &client->lock ); for (it = client->sendqueue; it; it = it->next) { free( it->data ); } @@ -347,10 +355,10 @@ dnbd3_client_t* dnbd3_free_client(dnbd3_client_t *client) client->sock = -1; if ( client->image != NULL ) image_release( client->image ); client->image = NULL; - pthread_spin_unlock(&client->lock); - pthread_spin_destroy(&client->lock); + spin_unlock( &client->lock ); + spin_destroy( &client->lock ); free( client ); - return NULL; + return NULL ; } //###// @@ -362,20 +370,20 @@ dnbd3_client_t* dnbd3_free_client(dnbd3_client_t *client) static int dnbd3_add_client(dnbd3_client_t *client) { int i; - pthread_spin_lock( &_clients_lock ); + spin_lock( &_clients_lock ); for (i = 0; i < _num_clients; ++i) { if ( _clients[i] != NULL ) continue; _clients[i] = client; - pthread_spin_unlock( &_clients_lock ); + spin_unlock( &_clients_lock ); return TRUE; } if ( _num_clients >= SERVER_MAX_CLIENTS ) { - pthread_spin_unlock( &_clients_lock ); + spin_unlock( &_clients_lock ); memlogf( "[ERROR] Maximum number of clients reached!" ); return FALSE; } _clients[_num_clients++] = client; - pthread_spin_unlock( &_clients_lock ); + spin_unlock( &_clients_lock ); return TRUE; } @@ -394,3 +402,9 @@ static void dnbd3_handle_sigterm(int signum) memlogf( "INFO: SIGTERM or SIGINT received (%s)", strsignal( signum ) ); dnbd3_cleanup(); } + +void dnbd3_handle_sigusr1(int signum) +{ + memlogf( "INFO: SIGUSR1 (%s) received, re-scanning image directory", strsignal( signum ) ); + image_load_all(NULL); +} diff --git a/src/server/uplink.c b/src/server/uplink.c index 797b287..07fe35b 100644 --- a/src/server/uplink.c +++ b/src/server/uplink.c @@ -1,4 +1,5 @@ #include "uplink.h" +#include "locks.h" #include #include #include @@ -17,7 +18,7 @@ int uplink_get_matching_alt_servers(dnbd3_host_t *host, dnbd3_server_entry_t *ou int i, j; int count = 0; int distance[size]; - pthread_spin_lock( &_alts_lock ); + spin_lock( &_alts_lock ); for (i = 0; i < _num_alts; ++i) { if ( host->type != _alt_servers[i]->host.type ) continue; // Wrong address family if ( count == 0 ) { @@ -48,7 +49,7 @@ int uplink_get_matching_alt_servers(dnbd3_host_t *host, dnbd3_server_entry_t *ou } } } - pthread_spin_unlock( &_alts_lock ); + spin_unlock( &_alts_lock ); return count; } diff --git a/src/types.h b/src/types.h index 6b074b7..a8d567d 100644 --- a/src/types.h +++ b/src/types.h @@ -22,7 +22,9 @@ #define TYPES_H_ #include "config.h" +#ifndef KERNEL_MODULE #include +#endif // ioctl #define DNBD3_MAGIC 'd' -- cgit v1.2.3-55-g7522