diff options
| author | Pranith Kumar K <pkarampu@redhat.com> | 2016-11-18 13:30:08 +0530 | 
|---|---|---|
| committer | Raghavendra G <rgowdapp@redhat.com> | 2016-11-21 23:01:18 -0800 | 
| commit | 25817a8c868b6c1b8149117f13e4216a99e453aa (patch) | |
| tree | fe42ae33a20de76c7e5022c479a7a026268c6c55 | |
| parent | 3e405b546e8b9fe15ae477613474e9cd2d2df4e7 (diff) | |
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 <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/15888
Smoke: Gluster Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
| -rw-r--r-- | xlators/performance/io-threads/src/io-threads.c | 41 | ||||
| -rw-r--r-- | xlators/performance/io-threads/src/io-threads.h | 2 | 
2 files changed, 32 insertions, 11 deletions
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;  | 
