io-stats: Terminate dump thread when dump interval is set to 0

_ios_dump_thread is not terminated by the function
_ios_destroy_dump_thread when the diagnostic interval is set
to 0 (which means disable auto dumping).

During reconfigure, if the value changes from 0 to another
then the thread is started, but on reconfiguring this to 0
the thread is not being terminated.

Further, if the value is changed from 0 to X to 0 to Y, where
X and Y are 2 arbitrary duration numbers, the reconfigure
code ends up starting one more thread (for each change from
0 to a valid interval).

This patch fixes the same by terminating the thread when the
value changes from non-zero to 0.

NOTE: It would seem nicer to use conf->dump_thread and check
its value for thread presence etc. but there is no documented
invalid value for the same, and hence an invalid check is not
feasible, thus introducing a new running bool to determine the
same.

Fixes: bz#1598548
Change-Id: I3e7d2ce8f033879542932ac730d57bfcaf14af73
Signed-off-by: ShyamsundarR <srangana@redhat.com>

1 files changed, 11 insertions, 1 deletions

diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c
index f5d86e1bc6a..28e6e362b6b 100644
--- a/xlators/debug/io-stats/src/io-stats.c
+++ b/xlators/debug/io-stats/src/io-stats.c
@@ -153,6 +153,7 @@ struct ios_conf {
         int32_t                   ios_dump_interval;
         pthread_t                 dump_thread;
         gf_boolean_t              dump_thread_should_die;
+        gf_boolean_t              dump_thread_running;
         gf_lock_t                 ios_sampling_lock;
         int32_t                   ios_sample_interval;
         int32_t                   ios_sample_buf_size;
@@ -3129,7 +3130,7 @@ conditional_dump (dict_t *dict, char *key, data_t *value, void *data)
 int
 _ios_destroy_dump_thread (struct ios_conf *conf) {
         conf->dump_thread_should_die = _gf_true;
-        if (conf->ios_dump_interval > 0) {
+        if (conf->dump_thread_running) {
                 (void) pthread_cancel (conf->dump_thread);
                 (void) pthread_join (conf->dump_thread, NULL);
         }
@@ -3248,6 +3249,7 @@ _ios_dump_thread (xlator_t *this) {
                 }
         }
 out:
+        conf->dump_thread_running = _gf_false;
         gf_log (this->name, GF_LOG_INFO, "IO stats dump thread terminated");
         return NULL;
 }
@@ -3883,14 +3885,19 @@ reconfigure (xlator_t *this, dict_t *options)
         GF_OPTION_RECONF ("ios-dump-interval", conf->ios_dump_interval, options,
                          int32, out);
         if ((old_dump_interval <= 0) && (conf->ios_dump_interval > 0)) {
+                conf->dump_thread_running = _gf_true;
+                conf->dump_thread_should_die = _gf_false;
                 ret = gf_thread_create (&conf->dump_thread, NULL,
                                   (void *) &_ios_dump_thread, this, "iosdump");
                 if (ret) {
+                        conf->dump_thread_running = _gf_false;
                         gf_log (this ? this->name : "io-stats",
                                 GF_LOG_ERROR, "Failed to start thread"
                                 "while reconfigure. Returning %d", ret);
                         goto out;
                 }
+        } else if ((old_dump_interval > 0) && (conf->ios_dump_interval == 0)) {
+                _ios_destroy_dump_thread (conf);
         }
 
         GF_OPTION_RECONF ("ios-sample-interval", conf->ios_sample_interval,
@@ -4114,10 +4121,13 @@ init (xlator_t *this)
 
         this->private = conf;
         if (conf->ios_dump_interval > 0) {
+                conf->dump_thread_running = _gf_true;
+                conf->dump_thread_should_die = _gf_false;
                 ret = gf_thread_create (&conf->dump_thread, NULL,
                                         (void *) &_ios_dump_thread,
                                         this, "iosdump");
                 if (ret) {
+                        conf->dump_thread_running = _gf_false;
                         gf_log (this ? this->name : "io-stats",
                                 GF_LOG_ERROR, "Failed to start thread"
                                 "in init. Returning %d", ret);