Blob Blame History Raw
From f856824bcbe2ced66a848cdb1a66600ae15158c9 Mon Sep 17 00:00:00 2001
From: Pranith Kumar K <pkarampu@redhat.com>
Date: Fri, 18 Nov 2016 13:30:08 +0530
Subject: [PATCH 191/206] 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>
 >(cherry picked from commit 25817a8c868b6c1b8149117f13e4216a99e453aa)

BUG: 1393526
Change-Id: Id55e7c2f3e90c013d40e59bfbfb3f1583b8c4061
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/91096
Reviewed-by: Raghavendra Gowdappa <rgowdapp@redhat.com>
Tested-by: Raghavendra Gowdappa <rgowdapp@redhat.com>
---
 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(-)

diff --git a/xlators/performance/io-threads/src/io-threads.c b/xlators/performance/io-threads/src/io-threads.c
index 394be8a..72a8208 100644
--- a/xlators/performance/io-threads/src/io-threads.c
+++ b/xlators/performance/io-threads/src/io-threads.c
@@ -964,6 +964,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,
@@ -971,6 +972,7 @@ init (xlator_t *this)
                         "pthread_mutex_init failed (%d)", ret);
                 goto out;
         }
+        conf->mutex_inited = _gf_true;
 
         set_stack_size (conf);
 
@@ -1025,22 +1027,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);
 
@@ -1052,6 +1059,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 cb984f0..fa955b5 100644
--- a/xlators/performance/io-threads/src/io-threads.h
+++ b/xlators/performance/io-threads/src/io-threads.h
@@ -80,6 +80,8 @@ struct iot_conf {
         size_t              stack_size;
 
         gf_boolean_t         down; /*PARENT_DOWN event is notified*/
+        gf_boolean_t         mutex_inited;
+        gf_boolean_t         cond_inited;
 	struct iot_least_throttle throttle;
 };
 
-- 
2.9.3