From 25817a8c868b6c1b8149117f13e4216a99e453aa Mon Sep 17 00:00:00 2001 From: Pranith Kumar K Date: Fri, 18 Nov 2016 13:30:08 +0530 Subject: performance/io-threads: Exit threads in fini() as well Problem: io-threads starts the thread in 'init()' but doesn't clean them up on 'fini()'. It relies on PARENT_DOWN to exit threads but there can be cases where event before PARENT_UP the graph init code can think of issuing fini(). This code path is hit when glfs_init() is called on a volume that is in 'stopped' state. It leads to a crash in ganesha process, because the io-thread tries to access freed memory. Fix: Ideal fix would be to wait for all fops in io-thread list to be completed on PARENT_DOWN, and have fini() do cleanup of threads. Because there is no proper documentation about how PARENT_DOWN/fini are supposed to be used, we are getting different kinds of sequences in different higher level protocols. So for now cleaning up in both PARENT_DOWN and fini(). Fuse doesn't call fini() gfapi is not calling PARENT_DOWN in some cases, so for now I don't see another way out. BUG: 1396793 Change-Id: I9c9154e7d57198dbaff0f30d3ffc25f6d8088aec Signed-off-by: Pranith Kumar K Reviewed-on: http://review.gluster.org/15888 Smoke: Gluster Build System CentOS-regression: Gluster Build System NetBSD-regression: NetBSD Build System Reviewed-by: Raghavendra G --- xlators/performance/io-threads/src/io-threads.c | 41 ++++++++++++++++++------- xlators/performance/io-threads/src/io-threads.h | 2 ++ 2 files changed, 32 insertions(+), 11 deletions(-) (limited to 'xlators/performance/io-threads') diff --git a/xlators/performance/io-threads/src/io-threads.c b/xlators/performance/io-threads/src/io-threads.c index a30de1572b9..fd860d6a133 100644 --- a/xlators/performance/io-threads/src/io-threads.c +++ b/xlators/performance/io-threads/src/io-threads.c @@ -992,6 +992,7 @@ init (xlator_t *this) "pthread_cond_init failed (%d)", ret); goto out; } + conf->cond_inited = _gf_true; if ((ret = pthread_mutex_init(&conf->mutex, NULL)) != 0) { gf_msg (this->name, GF_LOG_ERROR, 0, @@ -999,6 +1000,7 @@ init (xlator_t *this) "pthread_mutex_init failed (%d)", ret); goto out; } + conf->mutex_inited = _gf_true; set_stack_size (conf); @@ -1046,22 +1048,27 @@ out: return ret; } +static void +iot_exit_threads (iot_conf_t *conf) +{ + pthread_mutex_lock (&conf->mutex); + { + conf->down = _gf_true; + /*Let all the threads know that xl is going down*/ + pthread_cond_broadcast (&conf->cond); + while (conf->curr_count)/*Wait for threads to exit*/ + pthread_cond_wait (&conf->cond, &conf->mutex); + } + pthread_mutex_unlock (&conf->mutex); +} + int notify (xlator_t *this, int32_t event, void *data, ...) { iot_conf_t *conf = this->private; - if (GF_EVENT_PARENT_DOWN == event) { - pthread_mutex_lock (&conf->mutex); - { - conf->down = _gf_true; - /*Let all the threads know that xl is going down*/ - pthread_cond_broadcast (&conf->cond); - while (conf->curr_count)/*Wait for threads to exit*/ - pthread_cond_wait (&conf->cond, &conf->mutex); - } - pthread_mutex_unlock (&conf->mutex); - } + if (GF_EVENT_PARENT_DOWN == event) + iot_exit_threads (conf); default_notify (this, event, data); @@ -1073,6 +1080,18 @@ fini (xlator_t *this) { iot_conf_t *conf = this->private; + if (!conf) + return; + + if (conf->mutex_inited && conf->cond_inited) + iot_exit_threads (conf); + + if (conf->cond_inited) + pthread_cond_destroy (&conf->cond); + + if (conf->mutex_inited) + pthread_mutex_destroy (&conf->mutex); + GF_FREE (conf); this->private = NULL; diff --git a/xlators/performance/io-threads/src/io-threads.h b/xlators/performance/io-threads/src/io-threads.h index ae548cbcd44..cd28e2d40ad 100644 --- a/xlators/performance/io-threads/src/io-threads.h +++ b/xlators/performance/io-threads/src/io-threads.h @@ -82,6 +82,8 @@ struct iot_conf { xlator_t *this; size_t stack_size; gf_boolean_t down; /*PARENT_DOWN event is notified*/ + gf_boolean_t mutex_inited; + gf_boolean_t cond_inited; }; typedef struct iot_conf iot_conf_t; -- cgit