From 7dfeb1c1963596edd36193ede5ce8f82c2bf76a5 Mon Sep 17 00:00:00 2001 From: Simon Rettberg Date: Sun, 22 Feb 2015 15:26:21 +0100 Subject: Get rid of unneccessary volatile Some minor debugging code, mostly commented out --- CMakeLists.txt | 8 ++++---- src/kernel/dnbd3.h | 4 ++-- src/server/globals.c | 2 +- src/server/globals.h | 22 +++++++++++----------- src/server/integrity.c | 6 +++--- src/server/locks.c | 6 +++--- src/server/memlog.c | 4 ++-- src/server/net.c | 2 +- src/server/threadpool.c | 2 +- src/server/uplink.c | 35 ++++++++++++++++++++++++++++------- 10 files changed, 56 insertions(+), 35 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e0425ee..0e265f7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,12 +12,12 @@ message( "Build Type selected: ${CMAKE_BUILD_TYPE}" ) if(CMAKE_C_COMPILER MATCHES "clang") message( "Using clang flags." ) - SET(CMAKE_C_FLAGS_DEBUG "-std=c99 -fsanitize=address -O1 -fno-omit-frame-pointer -g -Wall -Wno-unused-result -D_GNU_SOURCE -D_DEBUG -Wno-multichar") - SET(CMAKE_C_FLAGS_RELEASE "-std=c99 -O2 -Wno-unused-result -D_GNU_SOURCE -DNDEBUG -Wno-multichar") + SET(CMAKE_C_FLAGS_DEBUG "-std=c99 -fsanitize=address -O1 -fno-omit-frame-pointer -g -Wall -Wno-unused-result -D_GNU_SOURCE -D_DEBUG -Wno-multichar -fno-strict-aliasing") + SET(CMAKE_C_FLAGS_RELEASE "-std=c99 -O2 -Wno-unused-result -D_GNU_SOURCE -DNDEBUG -Wno-multichar -fno-strict-aliasing") elseif (CMAKE_C_COMPILER MATCHES "(cc-)|(cc$)") message( "Using (g)cc flags." ) - SET(CMAKE_C_FLAGS_DEBUG "-std=c99 -O0 -g -pedantic -Wall -Wextra -D_GNU_SOURCE -D_DEBUG -Wno-multichar") - SET(CMAKE_C_FLAGS_RELEASE "-std=c99 -O2 -Wno-unused-result -D_GNU_SOURCE -DNDEBUG -Wno-multichar") + SET(CMAKE_C_FLAGS_DEBUG "-std=c99 -O0 -g -Wall -Wextra -Wpedantic -D_GNU_SOURCE -D_DEBUG -Wno-multichar -fno-strict-aliasing") + SET(CMAKE_C_FLAGS_RELEASE "-std=c99 -O2 -Wno-unused-result -D_GNU_SOURCE -DNDEBUG -Wno-multichar -fno-strict-aliasing") else() message( FATAL_ERROR "Could not determine compiler type." ) endif() diff --git a/src/kernel/dnbd3.h b/src/kernel/dnbd3.h index c703019..c0c7f4d 100644 --- a/src/kernel/dnbd3.h +++ b/src/kernel/dnbd3.h @@ -68,8 +68,8 @@ typedef struct struct socket *better_sock; // process - struct task_struct * volatile thread_send; - struct task_struct * volatile thread_receive; + struct task_struct * thread_send; + struct task_struct * thread_receive; struct task_struct *thread_discover; struct timer_list hb_timer; wait_queue_head_t process_queue_send; diff --git a/src/server/globals.c b/src/server/globals.c index e5f844d..441b2d1 100644 --- a/src/server/globals.c +++ b/src/server/globals.c @@ -19,7 +19,7 @@ int _uplinkTimeout = 1250; int _clientTimeout = 15000; #define SAVE_TO_VAR_STR(ss, kk) do { if (strcmp(section, #ss) == 0 && strcmp(key, #kk) == 0) { if (_ ## kk != NULL) free(_ ## kk); _ ## kk = strdup(value); } } while (0) -#define SAVE_TO_VAR_BOOL(ss, kk) do { if (strcmp(section, #ss) == 0 && strcmp(key, #kk) == 0) _ ## kk = atoi(value) != 0 || strcmp(value, "true") == 0 || strcmp(value, "True") == 0 || strcmp(value, "true") == 0; } while (0) +#define SAVE_TO_VAR_BOOL(ss, kk) do { if (strcmp(section, #ss) == 0 && strcmp(key, #kk) == 0) _ ## kk = atoi(value) != 0 || strcmp(value, "true") == 0 || strcmp(value, "True") == 0 || strcmp(value, "TRUE") == 0; } while (0) #define SAVE_TO_VAR_INT(ss, kk) do { if (strcmp(section, #ss) == 0 && strcmp(key, #kk) == 0) _ ## kk = atoi(value); } while (0) static int ini_handler(void *custom UNUSED, const char* section, const char* key, const char* value) diff --git a/src/server/globals.h b/src/server/globals.h index f9a1add..3c800aa 100644 --- a/src/server/globals.h +++ b/src/server/globals.h @@ -26,11 +26,11 @@ typedef struct _dnbd3_client dnbd3_client_t; #define ULR_PROCESSING 3 typedef struct { - uint64_t handle; // Client defined handle to pass back in reply - uint64_t from; // First byte offset of requested block (ie. 4096) - volatile uint64_t to; // Last byte + 1 of requested block (ie. 8192, if request len is 4096, resulting in bytes 4096-8191) - dnbd3_client_t * volatile client; // Client to send reply to - volatile int status; // status of this entry: ULR_* + uint64_t handle; // Client defined handle to pass back in reply + uint64_t from; // First byte offset of requested block (ie. 4096) + uint64_t to; // Last byte + 1 of requested block (ie. 8192, if request len is 4096, resulting in bytes 4096-8191) + dnbd3_client_t * client; // Client to send reply to + int status; // status of this entry: ULR_* time_t entered; // When this request entered the queue (for debugging) } dnbd3_queued_request_t; @@ -41,15 +41,15 @@ typedef struct #define RTT_NOT_REACHABLE 4 // No uplink was reachable struct _dnbd3_connection { - volatile int fd; // socket fd to remote server + int fd; // socket fd to remote server int signal; // eventfd used to wake up the process pthread_t thread; // thread holding the connection pthread_spinlock_t queueLock; // lock for synchronization on request queue etc. dnbd3_queued_request_t queue[SERVER_MAX_UPLINK_QUEUE]; - volatile int queueLen; // length of queue + int queueLen; // length of queue dnbd3_image_t *image; // image that this uplink is used for; do not call get/release for this pointer dnbd3_host_t currentServer; // Current server we're connected to - volatile int rttTestResult; // RTT_* + int rttTestResult; // RTT_* dnbd3_host_t betterServer; // The better server int betterFd; // Active connection to better server, ready to use uint8_t *recvBuffer; // Buffer for receiving payload @@ -98,17 +98,17 @@ struct _dnbd3_image { char *path; // absolute path of the image char *lower_name; // relative path, all lowercase, minus revision ID - uint8_t * volatile cache_map; // cache map telling which parts are locally cached, NULL if complete + uint8_t *cache_map; // cache map telling which parts are locally cached, NULL if complete uint32_t *crc32; // list of crc32 checksums for each 16MiB block in image uint32_t masterCrc32; // CRC-32 of the crc-32 list - dnbd3_connection_t * volatile uplink; // pointer to a server connection + dnbd3_connection_t *uplink; // pointer to a server connection uint64_t filesize; // size of image int readFd; // used to read the image. Used from multiple threads, so use atomic operations (pread et al) int cacheFd; // used to write to the image, in case it is relayed. ONLY USE FROM UPLINK THREAD! int rid; // revision of image int users; // clients currently using this image time_t atime; // last access time - volatile bool working; // true if image exists and completeness is == 100% or a working upstream proxy is connected + bool working; // true if image exists and completeness is == 100% or a working upstream proxy is connected pthread_spinlock_t lock; }; diff --git a/src/server/integrity.c b/src/server/integrity.c index e9b8ff3..352b1a5 100644 --- a/src/server/integrity.c +++ b/src/server/integrity.c @@ -20,15 +20,15 @@ typedef struct { - dnbd3_image_t * volatile image; - int volatile block; + dnbd3_image_t *image; + int block; } queue_entry; static pthread_t thread; static queue_entry checkQueue[CHECK_QUEUE_SIZE]; static pthread_mutex_t integrityQueueLock; static pthread_cond_t queueSignal; -static volatile int queueLen = -1; +static int queueLen = -1; static volatile bool bRunning = false; static void* integrity_main(void *data); diff --git a/src/server/locks.c b/src/server/locks.c index cf35986..de37734 100644 --- a/src/server/locks.c +++ b/src/server/locks.c @@ -25,8 +25,8 @@ typedef struct { void *lock; - volatile time_t locktime; - volatile char locked; + time_t locktime; + char locked; pthread_t thread; int lockId; char name[LOCKLEN]; @@ -48,7 +48,7 @@ static debug_lock_t locks[MAXLOCKS]; static debug_thread_t threads[MAXTHREADS]; static int init_done = 0; static pthread_spinlock_t initdestory; -static volatile int lockId = 0; +static int lockId = 0; static pthread_t watchdog = 0; static int watchdogSignal = -1; diff --git a/src/server/memlog.c b/src/server/memlog.c index 12de858..1d95c5a 100644 --- a/src/server/memlog.c +++ b/src/server/memlog.c @@ -41,9 +41,9 @@ typedef struct } LogLine; // This will be used as a ring buffer -static volatile LogLine *logBuffer = NULL; +static LogLine *logBuffer = NULL; // bufferPos counts up, use modulo LINE_COUNT to get array index -static volatile int bufferPos = 0; +static int bufferPos = 0; void initmemlog() { diff --git a/src/server/net.c b/src/server/net.c index 1497bbc..cde5f36 100644 --- a/src/server/net.c +++ b/src/server/net.c @@ -302,7 +302,7 @@ void *net_client_handler(void *dnbd3_client) const ssize_t ret = sendfile( client->sock, image_file, &offset, request.size - done ); if ( ret <= 0 ) { if ( lock ) pthread_mutex_unlock( &client->sendMutex ); - if ( ret < 0 && errno != 32 ) + if ( ret < 0 && errno != 32 && errno != 104 ) printf( "[DEBUG] sendfile failed (image to net. sent %d/%d, errno=%d)\n", (int)done, (int)request.size, (int)errno ); if ( errno == EBADF || errno == EINVAL || errno == EIO ) image->working = false; diff --git a/src/server/threadpool.c b/src/server/threadpool.c index 2d621c5..44649e4 100644 --- a/src/server/threadpool.c +++ b/src/server/threadpool.c @@ -11,7 +11,7 @@ typedef struct _entry_t { pthread_t thread; int signalFd; void *(*startRoutine)(void *); - void * volatile arg; + void * arg; } entry_t; static void *threadpool_worker(void *entryPtr); diff --git a/src/server/uplink.c b/src/server/uplink.c index bd776d1..2459171 100644 --- a/src/server/uplink.c +++ b/src/server/uplink.c @@ -188,14 +188,25 @@ bool uplink_request(dnbd3_client_t *client, uint64_t handle, uint64_t start, uin // is currently traversing the request queue. As it is processing the queue from highest to lowest index, it might // already have passed the index of the free slot we determined, but not reached the existing request we just found above. if ( foundExisting != -1 && existingType != ULR_NEW && freeSlot > foundExisting ) foundExisting = -1; +#ifdef _DEBUG + if ( foundExisting != -1 ) { + printf( "%p (%s) Found existing request of type %s at slot %d, attaching in slot %d.\n", (void*)uplink, uplink->image->lower_name, existingType == ULR_NEW ? "ULR_NEW" : "ULR_PENDING", foundExisting, freeSlot ); + printf( "Original %" PRIu64 "-%" PRIu64 " (%p)\n" + "New %" PRIu64 "-%" PRIu64 " (%p)\n", + uplink->queue[foundExisting].from, uplink->queue[foundExisting].to, (void*)uplink->queue[foundExisting].client, + start, end, (void*)client ); + } +#endif // Fill structure uplink->queue[freeSlot].from = start; uplink->queue[freeSlot].to = end; uplink->queue[freeSlot].handle = handle; uplink->queue[freeSlot].client = client; + //int old = uplink->queue[freeSlot].status; uplink->queue[freeSlot].status = (foundExisting == -1 ? ULR_NEW : ULR_PENDING); #ifdef _DEBUG uplink->queue[freeSlot].entered = time( NULL ); + //printf( "[V %p] Inserting request at slot %d, was %d, now %d, handle %" PRIu64 ", Range: %" PRIu64 "-%" PRIu64 "\n", (void*)uplink, freeSlot, old, uplink->queue[freeSlot].status, uplink->queue[freeSlot].handle, start, end ); #endif spin_unlock( &uplink->queueLock ); @@ -370,15 +381,15 @@ static void* uplink_mainloop(void *data) #ifdef _DEBUG if ( link->fd != -1 && !link->shutdown ) { bool resend = false; - const time_t deadline = now - 15; + const time_t deadline = now - 8; spin_lock( &link->queueLock ); for (i = 0; i < link->queueLen; ++i) { if ( link->queue[i].status != ULR_FREE && link->queue[i].entered < deadline ) { - snprintf( buffer, sizeof(buffer), "[DEBUG WARNING] Starving request detected:\n" - "%s\n(from %" PRIu64 " to %" PRIu64 ", status: %d)\n", link->queue[i].client->image->lower_name, + snprintf( buffer, sizeof(buffer), "[DEBUG %p] Starving request slot %d detected:\n" + "%s\n(from %" PRIu64 " to %" PRIu64 ", status: %d)\n", (void*)link, i, link->queue[i].client->image->lower_name, link->queue[i].from, link->queue[i].to, link->queue[i].status ); - //link->queue[i].status = ULR_NEW; - //resend = true; + link->queue[i].status = ULR_NEW; + resend = true; spin_unlock( &link->queueLock ); printf("%s", buffer); spin_lock( &link->queueLock ); @@ -426,6 +437,7 @@ static void uplink_sendRequests(dnbd3_connection_t *link, bool newOnly) spin_lock( &link->queueLock ); for (j = 0; j < link->queueLen; ++j) { if ( link->queue[j].status != ULR_NEW && (newOnly || link->queue[j].status != ULR_PENDING) ) continue; + //printf( "[V %p] Sending slot %d, now %d, handle %" PRIu64 ", Range: %" PRIu64 "-%" PRIu64 "\n", (void*)link, j, link->queue[j].status, link->queue[j].handle, link->queue[j].from, link->queue[j].to ); link->queue[j].status = ULR_PENDING; const uint64_t offset = link->queue[j].from; const uint32_t size = link->queue[j].to - link->queue[j].from; @@ -545,8 +557,9 @@ static void uplink_handleReceive(dnbd3_connection_t *link) spin_lock( &link->queueLock ); for (i = 0; i < link->queueLen; ++i) { dnbd3_queued_request_t * const req = &link->queue[i]; - assert( req->status != ULR_PROCESSING || req->status != ULR_NEW ); - if ( req->status != ULR_PENDING ) continue; + assert( req->status != ULR_PROCESSING ); + if ( req->status != ULR_PENDING && req->status != ULR_NEW ) continue; + assert( req->client != NULL ); if ( req->from >= start && req->to <= end ) { // Match :-) req->status = ULR_PROCESSING; } @@ -555,9 +568,11 @@ static void uplink_handleReceive(dnbd3_connection_t *link) // so we can decrease queueLen on the fly while iterating. Should you ever change this to start // from 0, you also need to change the "attach to existing request"-logic in uplink_request() outReply.magic = dnbd3_packet_magic; + bool served = false; for (i = link->queueLen - 1; i >= 0; --i) { dnbd3_queued_request_t * const req = &link->queue[i]; if ( req->status == ULR_PROCESSING ) { + //printf( "[V %p] Reply slot %d, handle %" PRIu64 ", Range: %" PRIu64 "-%" PRIu64 "\n", (void*)link, i, req->handle, req->from, req->to ); assert( req->from >= start && req->to <= end ); dnbd3_client_t * const client = req->client; outReply.cmd = CMD_GET_BLOCK; @@ -569,6 +584,8 @@ static void uplink_handleReceive(dnbd3_connection_t *link) iov[1].iov_len = outReply.size; fixup_reply( outReply ); req->status = ULR_FREE; + req->client = NULL; + served = true; pthread_mutex_lock( &client->sendMutex ); spin_unlock( &link->queueLock ); if ( client->sock != -1 ) writev( client->sock, iov, 2 ); @@ -578,6 +595,10 @@ static void uplink_handleReceive(dnbd3_connection_t *link) if ( req->status == ULR_FREE && i == link->queueLen - 1 ) link->queueLen--; } spin_unlock( &link->queueLock ); +#ifdef _DEBUG + if ( !served && start != link->replicationHandle ) + printf( "[DEBUG %p] %s -- Unmatched reply: %" PRIu64 " to %" PRIu64 "\n", (void*)link, link->image->lower_name, start, end ); +#endif if ( start == link->replicationHandle ) link->replicationHandle = 0; } if ( link->queueLen == 0 ) uplink_sendReplicationRequest( link ); -- cgit v1.2.3-55-g7522