From: Jens Axboe Date: Thu, 2 Aug 2012 06:27:41 +0000 (+0200) Subject: diskutil: ensure that we lock around disk_list access X-Git-Tag: fio-2.0.9~9 X-Git-Url: https://git.kernel.dk/?a=commitdiff_plain;h=9ec7779f89d4dde9fe1e23d68abaefda54b35de7;p=fio.git diskutil: ensure that we lock around disk_list access And also ensure that before we free the disk util structures, we wait for the disk util thread to exit. This is v2 of the patch. Commit feb41855 had a bug where it would deadlock on the disk_util_mutex, when handling drives that had slaves (lvm/md). Signed-off-by: Jens Axboe --- diff --git a/backend.c b/backend.c index e1dc0ac8..9cc8dbc3 100644 --- a/backend.c +++ b/backend.c @@ -50,6 +50,7 @@ #include "server.h" static pthread_t disk_util_thread; +static struct fio_mutex *disk_thread_mutex; static struct fio_mutex *startup_mutex; static struct fio_mutex *writeout_mutex; static struct flist_head *cgroup_list; @@ -1588,20 +1589,28 @@ static void run_threads(void) fio_unpin_memory(); } +void wait_for_disk_thread_exit(void) +{ + fio_mutex_down(disk_thread_mutex); +} + static void *disk_thread_main(void *data) { + int ret = 0; + fio_mutex_up(startup_mutex); - while (threads) { + while (threads && !ret) { usleep(DISK_UTIL_MSEC * 1000); if (!threads) break; - update_io_ticks(); + ret = update_io_ticks(); if (!is_backend) print_thread_status(); } + fio_mutex_up(disk_thread_mutex); return NULL; } @@ -1609,14 +1618,20 @@ static int create_disk_util_thread(void) { int ret; + setup_disk_util(); + + disk_thread_mutex = fio_mutex_init(0); + ret = pthread_create(&disk_util_thread, NULL, disk_thread_main, NULL); if (ret) { + fio_mutex_remove(disk_thread_mutex); log_err("Can't create disk util thread: %s\n", strerror(ret)); return 1; } ret = pthread_detach(disk_util_thread); if (ret) { + fio_mutex_remove(disk_thread_mutex); log_err("Can't detatch disk util thread: %s\n", strerror(ret)); return 1; } @@ -1680,5 +1695,6 @@ int fio_backend(void) fio_mutex_remove(startup_mutex); fio_mutex_remove(writeout_mutex); + fio_mutex_remove(disk_thread_mutex); return exit_value; } diff --git a/diskutil.c b/diskutil.c index feb88526..f51d07b3 100644 --- a/diskutil.c +++ b/diskutil.c @@ -14,6 +14,9 @@ static int last_majdev, last_mindev; static struct disk_util *last_du; +static struct fio_mutex *disk_util_mutex; +static int disk_util_exit; + FLIST_HEAD(disk_list); static struct disk_util *__init_per_file_disk_util(struct thread_data *td, @@ -102,17 +105,26 @@ static void update_io_tick_disk(struct disk_util *du) memcpy(ldus, &__dus, sizeof(__dus)); } -void update_io_ticks(void) +int update_io_ticks(void) { struct flist_head *entry; struct disk_util *du; + int ret = 0; dprint(FD_DISKUTIL, "update io ticks\n"); - flist_for_each(entry, &disk_list) { - du = flist_entry(entry, struct disk_util, list); - update_io_tick_disk(du); - } + fio_mutex_down(disk_util_mutex); + + if (!disk_util_exit) { + flist_for_each(entry, &disk_list) { + du = flist_entry(entry, struct disk_util, list); + update_io_tick_disk(du); + } + } else + ret = 1; + + fio_mutex_up(disk_util_mutex); + return ret; } static struct disk_util *disk_util_exists(int major, int minor) @@ -120,13 +132,18 @@ static struct disk_util *disk_util_exists(int major, int minor) struct flist_head *entry; struct disk_util *du; + fio_mutex_down(disk_util_mutex); + flist_for_each(entry, &disk_list) { du = flist_entry(entry, struct disk_util, list); - if (major == du->major && minor == du->minor) + if (major == du->major && minor == du->minor) { + fio_mutex_up(disk_util_mutex); return du; + } } + fio_mutex_up(disk_util_mutex); return NULL; } @@ -276,6 +293,8 @@ static struct disk_util *disk_util_add(struct thread_data *td, int majdev, du->lock = fio_mutex_init(1); du->users = 0; + fio_mutex_down(disk_util_mutex); + flist_for_each(entry, &disk_list) { __du = flist_entry(entry, struct disk_util, list); @@ -283,6 +302,7 @@ static struct disk_util *disk_util_add(struct thread_data *td, int majdev, if (!strcmp((char *) du->dus.name, (char *) __du->dus.name)) { disk_util_free(du); + fio_mutex_up(disk_util_mutex); return __du; } } @@ -293,6 +313,8 @@ static struct disk_util *disk_util_add(struct thread_data *td, int majdev, get_io_ticks(du, &du->last_dus); flist_add_tail(&du->list, &disk_list); + fio_mutex_up(disk_util_mutex); + find_add_disk_slaves(td, path, du); return du; } @@ -521,6 +543,11 @@ void free_disk_util(void) { struct disk_util *du; + disk_util_exit = 1; + wait_for_disk_thread_exit(); + + fio_mutex_down(disk_util_mutex); + while (!flist_empty(&disk_list)) { du = flist_entry(disk_list.next, struct disk_util, list); flist_del(&du->list); @@ -528,6 +555,8 @@ void free_disk_util(void) } last_majdev = last_mindev = -1; + fio_mutex_up(disk_util_mutex); + fio_mutex_remove(disk_util_mutex); } void print_disk_util(struct disk_util_stat *dus, struct disk_util_agg *agg, @@ -573,8 +602,12 @@ void show_disk_util(int terse) struct flist_head *entry; struct disk_util *du; - if (flist_empty(&disk_list)) + fio_mutex_down(disk_util_mutex); + + if (flist_empty(&disk_list)) { + fio_mutex_up(disk_util_mutex); return; + } if (!terse) log_info("\nDisk stats (read/write):\n"); @@ -585,4 +618,11 @@ void show_disk_util(int terse) aggregate_slaves_stats(du); print_disk_util(&du->dus, &du->agg, terse); } + + fio_mutex_up(disk_util_mutex); +} + +void setup_disk_util(void) +{ + disk_util_mutex = fio_mutex_init(1); } diff --git a/diskutil.h b/diskutil.h index 5a9b0796..88dde555 100644 --- a/diskutil.h +++ b/diskutil.h @@ -94,6 +94,8 @@ static inline void disk_util_dec(struct disk_util *du) extern struct flist_head disk_list; +extern void wait_for_disk_thread_exit(void); + /* * disk util stuff */ @@ -102,13 +104,18 @@ extern void print_disk_util(struct disk_util_stat *, struct disk_util_agg *, int extern void show_disk_util(int terse); extern void free_disk_util(void); extern void init_disk_util(struct thread_data *); -extern void update_io_ticks(void); +extern int update_io_ticks(void); +extern void setup_disk_util(void); #else #define print_disk_util(dus, agg, terse) #define show_disk_util(terse) #define free_disk_util() #define init_disk_util(td) -#define update_io_ticks() +#define setup_disk_util() +static inline int update_io_ticks(void) +{ + return 0; +} #endif #endif diff --git a/iolog.h b/iolog.h index 95617fcc..1853846e 100644 --- a/iolog.h +++ b/iolog.h @@ -107,7 +107,6 @@ extern void add_bw_sample(struct thread_data *, enum fio_ddir, unsigned int, extern void add_iops_sample(struct thread_data *, enum fio_ddir, struct timeval *); extern void init_disk_util(struct thread_data *); extern void update_rusage_stat(struct thread_data *); -extern void update_io_ticks(void); extern void setup_log(struct io_log **, unsigned long); extern void finish_log(struct thread_data *, struct io_log *, const char *); extern void finish_log_named(struct thread_data *, struct io_log *, const char *, const char *);