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