From d7df1d133b0c3daad4ae4c731e0dae7b0181fd62 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Wed, 20 Mar 2013 19:57:01 -0600 Subject: [PATCH] Rework lockfile= file lock handling Get rid of the hand rolled rw semaphores, just use pthread rwlocks instead. Kill the batching too, it was broken by default, so nobody could have been using it. Signed-off-by: Jens Axboe --- HOWTO | 5 ---- file.h | 8 +++--- filesetup.c | 61 ++++++++++++++++++-------------------------- fio.1 | 5 ---- fio.h | 1 + mutex.c | 73 +++++++++++++++++++++++++---------------------------- mutex.h | 24 ++++++++---------- options.c | 15 ----------- smalloc.c | 14 +++++----- 9 files changed, 81 insertions(+), 125 deletions(-) diff --git a/HOWTO b/HOWTO index fbe8f791..cf6d4274 100644 --- a/HOWTO +++ b/HOWTO @@ -302,11 +302,6 @@ lockfile=str Fio defaults to not locking any files before it does same time, but writes get exclusive access. - The option may be post-fixed with a lock batch number. If - set, then each thread/process may do that amount of IOs to - the file before giving up the lock. Since lock acquisition is - expensive, batching the lock/unlocks will speed up IO. - readwrite=str rw=str Type of io pattern. Accepted values are: diff --git a/file.h b/file.h index eb0688c5..d7e05f4f 100644 --- a/file.h +++ b/file.h @@ -102,10 +102,10 @@ struct fio_file { /* * if io is protected by a semaphore, this is set */ - struct fio_mutex *lock; - void *lock_owner; - unsigned int lock_batch; - enum fio_ddir lock_ddir; + union { + struct fio_mutex *lock; + struct fio_rwlock *rwlock; + }; /* * block map for random io diff --git a/filesetup.c b/filesetup.c index 57553c96..e456186b 100644 --- a/filesetup.c +++ b/filesetup.c @@ -456,9 +456,6 @@ int file_lookup_open(struct fio_file *f, int flags) * racy, need the __f->lock locked */ f->lock = __f->lock; - f->lock_owner = __f->lock_owner; - f->lock_batch = __f->lock_batch; - f->lock_ddir = __f->lock_ddir; from_hash = 1; } else { dprint(FD_FILE, "file not found in hash %s\n", f->file_name); @@ -1028,8 +1025,10 @@ void close_and_free_files(struct thread_data *td) td->o.filename = NULL; free(td->files); + free(td->file_locks); td->files_index = 0; td->files = NULL; + td->file_locks = NULL; td->o.nr_files = 0; } @@ -1086,6 +1085,14 @@ int add_file(struct thread_data *td, const char *fname) log_err("fio: realloc OOM\n"); assert(0); } + if (td->o.file_lock_mode != FILE_LOCK_NONE) { + td->file_locks = realloc(td->file_locks, new_size); + if (!td->file_locks) { + log_err("fio: realloc OOM\n"); + assert(0); + } + td->file_locks[cur_files] = FILE_LOCK_NONE; + } td->files_size = new_size; } td->files[cur_files] = f; @@ -1113,7 +1120,7 @@ int add_file(struct thread_data *td, const char *fname) case FILE_LOCK_NONE: break; case FILE_LOCK_READWRITE: - f->lock = fio_mutex_rw_init(); + f->rwlock = fio_rwlock_init(); break; case FILE_LOCK_EXCLUSIVE: f->lock = fio_mutex_init(FIO_MUTEX_UNLOCKED); @@ -1188,57 +1195,34 @@ void lock_file(struct thread_data *td, struct fio_file *f, enum fio_ddir ddir) if (!f->lock || td->o.file_lock_mode == FILE_LOCK_NONE) return; - if (f->lock_owner == td && f->lock_batch--) - return; - if (td->o.file_lock_mode == FILE_LOCK_READWRITE) { if (ddir == DDIR_READ) - fio_mutex_down_read(f->lock); + fio_rwlock_read(f->rwlock); else - fio_mutex_down_write(f->lock); + fio_rwlock_write(f->rwlock); } else if (td->o.file_lock_mode == FILE_LOCK_EXCLUSIVE) fio_mutex_down(f->lock); - f->lock_owner = td; - f->lock_batch = td->o.lockfile_batch; - f->lock_ddir = ddir; + td->file_locks[f->fileno] = td->o.file_lock_mode; } void unlock_file(struct thread_data *td, struct fio_file *f) { if (!f->lock || td->o.file_lock_mode == FILE_LOCK_NONE) return; - if (f->lock_batch) - return; - - if (td->o.file_lock_mode == FILE_LOCK_READWRITE) { - const int is_read = f->lock_ddir == DDIR_READ; - int val = fio_mutex_getval(f->lock); - - if ((is_read && val == 1) || (!is_read && val == -1)) - f->lock_owner = NULL; - - if (is_read) - fio_mutex_up_read(f->lock); - else - fio_mutex_up_write(f->lock); - } else if (td->o.file_lock_mode == FILE_LOCK_EXCLUSIVE) { - int val = fio_mutex_getval(f->lock); - - if (val == 0) - f->lock_owner = NULL; + if (td->o.file_lock_mode == FILE_LOCK_READWRITE) + fio_rwlock_unlock(f->rwlock); + else if (td->o.file_lock_mode == FILE_LOCK_EXCLUSIVE) fio_mutex_up(f->lock); - } + + td->file_locks[f->fileno] = FILE_LOCK_NONE; } void unlock_file_all(struct thread_data *td, struct fio_file *f) { - if (f->lock_owner != td) - return; - - f->lock_batch = 0; - unlock_file(td, f); + if (td->file_locks[f->fileno] != FILE_LOCK_NONE) + unlock_file(td, f); } static int recurse_dir(struct thread_data *td, const char *dirname) @@ -1311,6 +1295,9 @@ void dup_files(struct thread_data *td, struct thread_data *org) td->files = malloc(org->files_index * sizeof(f)); + if (td->o.file_lock_mode != FILE_LOCK_NONE) + td->file_locks = malloc(org->files_index); + for_each_file(org, f, i) { struct fio_file *__f; diff --git a/fio.1 b/fio.1 index bacc634e..fe8ab762 100644 --- a/fio.1 +++ b/fio.1 @@ -170,11 +170,6 @@ Read-write locking on the file. Many readers may access the file at the same time, but writes get exclusive access. .RE .P -The option may be post-fixed with a lock batch number. If set, then each -thread/process may do that amount of IOs to the file before giving up the lock. -Since lock acquisition is expensive, batching the lock/unlocks will speed up IO. -.RE -.P .BI opendir \fR=\fPstr Recursively open any files below directory \fIstr\fR. .TP diff --git a/fio.h b/fio.h index c42c4e1a..67388ae8 100644 --- a/fio.h +++ b/fio.h @@ -359,6 +359,7 @@ struct thread_data { struct rusage ru_end; struct fio_file **files; + unsigned char *file_locks; unsigned int files_size; unsigned int files_index; unsigned int nr_open_files; diff --git a/mutex.c b/mutex.c index 3b94beff..332e9f9e 100644 --- a/mutex.c +++ b/mutex.c @@ -106,10 +106,8 @@ int fio_mutex_down_timeout(struct fio_mutex *mutex, unsigned int seconds) * way too early, double check. */ ret = pthread_cond_timedwait(&mutex->cond, &mutex->lock, &t); - if (ret == ETIMEDOUT && !mutex_timed_out(&tv_s, seconds)) { - pthread_mutex_lock(&mutex->lock); + if (ret == ETIMEDOUT && !mutex_timed_out(&tv_s, seconds)) ret = 0; - } mutex->waiters--; } @@ -146,50 +144,49 @@ void fio_mutex_up(struct fio_mutex *mutex) pthread_mutex_unlock(&mutex->lock); } -void fio_mutex_down_write(struct fio_mutex *mutex) +void fio_rwlock_write(struct fio_rwlock *lock) { - pthread_mutex_lock(&mutex->lock); - - while (mutex->value != 0) { - mutex->waiters++; - pthread_cond_wait(&mutex->cond, &mutex->lock); - mutex->waiters--; - } - - mutex->value--; - pthread_mutex_unlock(&mutex->lock); + pthread_rwlock_wrlock(&lock->lock); } -void fio_mutex_down_read(struct fio_mutex *mutex) +void fio_rwlock_read(struct fio_rwlock *lock) { - pthread_mutex_lock(&mutex->lock); - - while (mutex->value < 0) { - mutex->waiters++; - pthread_cond_wait(&mutex->cond, &mutex->lock); - mutex->waiters--; - } + pthread_rwlock_rdlock(&lock->lock); +} - mutex->value++; - pthread_mutex_unlock(&mutex->lock); +void fio_rwlock_unlock(struct fio_rwlock *lock) +{ + pthread_rwlock_unlock(&lock->lock); } -void fio_mutex_up_read(struct fio_mutex *mutex) +void fio_rwlock_remove(struct fio_rwlock *lock) { - pthread_mutex_lock(&mutex->lock); - mutex->value--; - read_barrier(); - if (mutex->value >= 0 && mutex->waiters) - pthread_cond_signal(&mutex->cond); - pthread_mutex_unlock(&mutex->lock); + munmap((void *) lock, sizeof(*lock)); } -void fio_mutex_up_write(struct fio_mutex *mutex) +struct fio_rwlock *fio_rwlock_init(void) { - pthread_mutex_lock(&mutex->lock); - mutex->value++; - read_barrier(); - if (mutex->value >= 0 && mutex->waiters) - pthread_cond_signal(&mutex->cond); - pthread_mutex_unlock(&mutex->lock); + struct fio_rwlock *lock; + int ret; + + lock = (void *) mmap(NULL, sizeof(struct fio_rwlock), + PROT_READ | PROT_WRITE, + OS_MAP_ANON | MAP_SHARED, -1, 0); + if (lock == MAP_FAILED) { + perror("mmap rwlock"); + lock = NULL; + goto err; + } + + ret = pthread_rwlock_init(&lock->lock, NULL); + if (ret) { + log_err("pthread_rwlock_init: %s\n", strerror(ret)); + goto err; + } + + return lock; +err: + if (lock) + fio_rwlock_remove(lock); + return NULL; } diff --git a/mutex.h b/mutex.h index 6fdf7c6d..49a66e36 100644 --- a/mutex.h +++ b/mutex.h @@ -10,6 +10,10 @@ struct fio_mutex { int waiters; }; +struct fio_rwlock { + pthread_rwlock_t lock; +}; + enum { FIO_MUTEX_LOCKED = 0, FIO_MUTEX_UNLOCKED = 1, @@ -17,22 +21,14 @@ enum { extern struct fio_mutex *fio_mutex_init(int); extern void fio_mutex_remove(struct fio_mutex *); +extern void fio_mutex_up(struct fio_mutex *); extern void fio_mutex_down(struct fio_mutex *); extern int fio_mutex_down_timeout(struct fio_mutex *, unsigned int); -extern void fio_mutex_down_read(struct fio_mutex *); -extern void fio_mutex_down_write(struct fio_mutex *); -extern void fio_mutex_up(struct fio_mutex *); -extern void fio_mutex_up_read(struct fio_mutex *); -extern void fio_mutex_up_write(struct fio_mutex *); - -static inline struct fio_mutex *fio_mutex_rw_init(void) -{ - return fio_mutex_init(0); -} -static inline int fio_mutex_getval(struct fio_mutex *mutex) -{ - return mutex->value; -} +extern void fio_rwlock_read(struct fio_rwlock *); +extern void fio_rwlock_write(struct fio_rwlock *); +extern void fio_rwlock_unlock(struct fio_rwlock *); +extern struct fio_rwlock *fio_rwlock_init(void); +extern void fio_rwlock_remove(struct fio_rwlock *); #endif diff --git a/options.c b/options.c index 42a2ea07..3eb5fdc9 100644 --- a/options.c +++ b/options.c @@ -986,20 +986,6 @@ static int str_verify_pattern_cb(void *data, const char *input) return 0; } -static int str_lockfile_cb(void *data, const char *str) -{ - struct thread_data *td = data; - char *nr = get_opt_postfix(str); - - td->o.lockfile_batch = 1; - if (nr) { - td->o.lockfile_batch = atoi(nr); - free(nr); - } - - return 0; -} - static int str_write_bw_log_cb(void *data, const char *str) { struct thread_data *td = data; @@ -1157,7 +1143,6 @@ static struct fio_option options[FIO_MAX_OPTS] = { { .name = "lockfile", .type = FIO_OPT_STR, - .cb = str_lockfile_cb, .off1 = td_var_offset(file_lock_mode), .help = "Lock file when doing IO to it", .parent = "filename", diff --git a/smalloc.c b/smalloc.c index b0173739..5dae7e7c 100644 --- a/smalloc.c +++ b/smalloc.c @@ -52,7 +52,7 @@ struct block_hdr { static struct pool mp[MAX_POOLS]; static unsigned int nr_pools; static unsigned int last_pool; -static struct fio_mutex *lock; +static struct fio_rwlock *lock; static inline void pool_lock(struct pool *pool) { @@ -66,22 +66,22 @@ static inline void pool_unlock(struct pool *pool) static inline void global_read_lock(void) { - fio_mutex_down_read(lock); + fio_rwlock_read(lock); } static inline void global_read_unlock(void) { - fio_mutex_up_read(lock); + fio_rwlock_unlock(lock); } static inline void global_write_lock(void) { - fio_mutex_down_write(lock); + fio_rwlock_write(lock); } static inline void global_write_unlock(void) { - fio_mutex_up_write(lock); + fio_rwlock_unlock(lock); } static inline int ptr_valid(struct pool *pool, void *ptr) @@ -223,7 +223,7 @@ void sinit(void) { int ret; - lock = fio_mutex_rw_init(); + lock = fio_rwlock_init(); ret = add_pool(&mp[0], INITIAL_SIZE); assert(!ret); } @@ -248,7 +248,7 @@ void scleanup(void) cleanup_pool(&mp[i]); if (lock) - fio_mutex_remove(lock); + fio_rwlock_remove(lock); } #ifdef SMALLOC_REDZONE -- 2.25.1