From c055685f24251c70875060751c94d4bb4318cce9 Mon Sep 17 00:00:00 2001 From: Mark Callaghan Date: Fri, 5 Nov 2010 18:03:23 -0700 Subject: [PATCH] Fix race condition. Global variables were begin accessed without a mutex locked. This caused some fsync calls to use a bogus file descriptor, FATAL errors and early test failure. --- sysbench/tests/fileio/sb_fileio.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/sysbench/tests/fileio/sb_fileio.c b/sysbench/tests/fileio/sb_fileio.c index caa45a3..7253f46 100644 --- a/sysbench/tests/fileio/sb_fileio.c +++ b/sysbench/tests/fileio/sb_fileio.c @@ -153,7 +153,6 @@ static long long position; /* current position in file */ static unsigned int current_file; /* current file */ static unsigned int fsynced_file; /* file number to be fsynced (periodic) */ static unsigned int fsynced_file2; /* fsyncing in the end */ -static pthread_mutex_t fsync_mutex; /* used to sync access to counters */ static int is_dirty; /* any writes after last fsync series ? */ static int read_ops; @@ -355,8 +354,6 @@ int file_prepare(void) return 1; #endif - pthread_mutex_init(&fsync_mutex, NULL); - return 0; } @@ -385,8 +382,6 @@ int file_done(void) if (buffer != NULL) sb_free_memaligned(buffer); - pthread_mutex_destroy(&fsync_mutex); - return 0; } @@ -412,6 +407,7 @@ sb_request_t file_get_seq_request(void) sb_file_request_t *file_req = &sb_req.u.file_request; sb_req.type = SB_REQ_TYPE_FILE; + SB_THREAD_MUTEX_LOCK(); /* assume function is called with correct mode always */ if (test_mode == MODE_WRITE || test_mode == MODE_REWRITE) @@ -435,7 +431,6 @@ sb_request_t file_get_seq_request(void) } /* Advance pointers, if we're not in the fsync phase */ - pthread_mutex_lock(&fsync_mutex); if(current_file != num_files) position += file_req->size; /* scroll to the next file if not already out of bound */ @@ -444,14 +439,13 @@ sb_request_t file_get_seq_request(void) current_file++; position=0; - pthread_mutex_unlock(&fsync_mutex); - if (sb_globals.validate) { check_seq_req(&prev_req, file_req); prev_req = *file_req; } + SB_THREAD_MUTEX_UNLOCK(); return sb_req; /* This request is valid even for last file */ } @@ -471,7 +465,7 @@ sb_request_t file_get_seq_request(void) else sb_req.type = SB_REQ_TYPE_NULL; } - pthread_mutex_unlock(&fsync_mutex); + SB_THREAD_MUTEX_UNLOCK(); if (sb_globals.validate) { @@ -496,6 +490,7 @@ sb_request_t file_get_rnd_request(void) int mode = test_mode; sb_req.type = SB_REQ_TYPE_FILE; + SB_THREAD_MUTEX_LOCK(); /* Convert mode for combined tests. Locking to get consistent values @@ -503,12 +498,10 @@ sb_request_t file_get_rnd_request(void) */ if (test_mode==MODE_RND_RW) { - SB_THREAD_MUTEX_LOCK(); if ((double)(real_read_ops + 1) / (real_write_ops + 1) < file_rw_ratio) mode=MODE_RND_READ; else mode=MODE_RND_WRITE; - SB_THREAD_MUTEX_UNLOCK(); } /* fsync all files (if requested by user) as soon as we are done */ @@ -518,7 +511,6 @@ sb_request_t file_get_rnd_request(void) (real_mode == MODE_RND_WRITE || real_mode == MODE_RND_RW || real_mode == MODE_MIXED)) { - pthread_mutex_lock(&fsync_mutex); if(fsynced_file2 < num_files) { file_req->file_id = fsynced_file2; @@ -526,14 +518,14 @@ sb_request_t file_get_rnd_request(void) file_req->pos = 0; file_req->size = 0; fsynced_file2++; - pthread_mutex_unlock(&fsync_mutex); - + + SB_THREAD_MUTEX_UNLOCK(); return sb_req; } - pthread_mutex_unlock(&fsync_mutex); } sb_req.type = SB_REQ_TYPE_NULL; + SB_THREAD_MUTEX_UNLOCK(); return sb_req; } @@ -556,6 +548,7 @@ sb_request_t file_get_rnd_request(void) is_dirty = 0; } + SB_THREAD_MUTEX_UNLOCK(); return sb_req; } } @@ -576,6 +569,7 @@ sb_request_t file_get_rnd_request(void) if (file_req->operation == FILE_OP_TYPE_WRITE) is_dirty = 1; + SB_THREAD_MUTEX_UNLOCK(); return sb_req; }