Blob Blame History Raw
From bf24623765817ede84ea47f3265f5e6c2ae17ee7 Mon Sep 17 00:00:00 2001
From: Mohit Agrawal <moagrawal@redhat.com>
Date: Tue, 16 Jul 2019 20:36:57 +0530
Subject: [PATCH 279/284] posix: In brick_mux brick is crashed while start/stop
 volume in loop

Problem: In brick_mux environment sometime brick is crashed while
         volume stop/start in a loop.Brick is crashed in janitor task
         at the time of accessing priv.If posix priv is cleaned up before
         call janitor task then janitor task is crashed.

Solution: To avoid the crash in brick_mux environment introduce a new
          flag janitor_task_stop in posix_private and before send CHILD_DOWN event
          wait for update the flag by janitor_task_done

> Change-Id: Id9fa5d183a463b2b682774ab5cb9868357d139a4
> fixes: bz#1730409
> Signed-off-by: Mohit Agrawal <moagrawal@redhat.com>
> (Cherry picked from commit f138d3fa2237e7fa940ecf17153fd700350c4138)
> (Reviewed on upstream link https://review.gluster.org/#/c/glusterfs/+/23060/)

Change-Id: Id9fa5d183a463b2b682774ab5cb9868357d139a4
fixex: bz#1729971
Signed-off-by: Mohit Agrawal <moagrawal@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/178934
Tested-by: Mohit Agrawal <moagrawa@redhat.com>
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
 libglusterfs/src/glusterfs/xlator.h        |  3 +++
 xlators/mgmt/glusterd/src/glusterd-utils.c |  5 ++--
 xlators/protocol/server/src/server.c       |  6 ++++-
 xlators/storage/posix/src/posix-common.c   | 40 +++++++++++++++++++++++++++++-
 xlators/storage/posix/src/posix-helpers.c  | 16 ++++++++++++
 xlators/storage/posix/src/posix.h          |  3 +++
 6 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/libglusterfs/src/glusterfs/xlator.h b/libglusterfs/src/glusterfs/xlator.h
index b78daad..da551e9 100644
--- a/libglusterfs/src/glusterfs/xlator.h
+++ b/libglusterfs/src/glusterfs/xlator.h
@@ -861,6 +861,9 @@ struct _xlator {
 
     /* Flag to notify got CHILD_DOWN event for detach brick */
     uint32_t notify_down;
+
+    /* Flag to avoid throw duplicate PARENT_DOWN event */
+    uint32_t parent_down;
 };
 
 /* This would be the only structure which needs to be exported by
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index 2aa975b..812c698 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -4082,8 +4082,9 @@ out:
     if (msg[0]) {
         gf_msg("glusterd", GF_LOG_ERROR, 0, GD_MSG_BRICK_IMPORT_FAIL, "%s",
                msg);
-        gf_event(EVENT_IMPORT_BRICK_FAILED, "peer=%s;brick=%s",
-                 new_brickinfo->hostname, new_brickinfo->path);
+        if (new_brickinfo)
+            gf_event(EVENT_IMPORT_BRICK_FAILED, "peer=%s;brick=%s",
+                     new_brickinfo->hostname, new_brickinfo->path);
     }
     gf_msg_debug("glusterd", 0, "Returning with %d", ret);
     return ret;
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index 6ae63ba..a5f09fe 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -580,6 +580,7 @@ server_graph_janitor_threads(void *data)
     gf_boolean_t victim_found = _gf_false;
     xlator_list_t **trav_p = NULL;
     xlator_t *top = NULL;
+    uint32_t parent_down = 0;
 
     GF_ASSERT(data);
 
@@ -598,7 +599,10 @@ server_graph_janitor_threads(void *data)
         victim = (*trav_p)->xlator;
         if (victim->cleanup_starting &&
             strcmp(victim->name, victim_name) == 0) {
-            victim_found = _gf_true;
+            parent_down = victim->parent_down;
+            victim->parent_down = 1;
+            if (!parent_down)
+                victim_found = _gf_true;
             break;
         }
     }
diff --git a/xlators/storage/posix/src/posix-common.c b/xlators/storage/posix/src/posix-common.c
index d738692..69857d9 100644
--- a/xlators/storage/posix/src/posix-common.c
+++ b/xlators/storage/posix/src/posix-common.c
@@ -146,10 +146,15 @@ int32_t
 posix_notify(xlator_t *this, int32_t event, void *data, ...)
 {
     xlator_t *victim = data;
+    struct posix_private *priv = this->private;
+    int ret = 0;
+    struct timespec sleep_till = {
+        0,
+    };
 
     switch (event) {
         case GF_EVENT_PARENT_UP: {
-            /* Tell the parent that posix xlator is up */
+            /* the parent that posix xlator is up */
             default_notify(this, GF_EVENT_CHILD_UP, data);
         } break;
 
@@ -158,6 +163,31 @@ posix_notify(xlator_t *this, int32_t event, void *data, ...)
                 break;
             gf_log(this->name, GF_LOG_INFO, "Sending CHILD_DOWN for brick %s",
                    victim->name);
+
+            if (priv->janitor) {
+                pthread_mutex_lock(&priv->janitor_mutex);
+                {
+                    priv->janitor_task_stop = _gf_true;
+                    ret = gf_tw_del_timer(this->ctx->tw->timer_wheel,
+                                          priv->janitor);
+                    if (!ret) {
+                        clock_gettime(CLOCK_REALTIME, &sleep_till);
+                        sleep_till.tv_sec += 1;
+                        /* Wait to set janitor_task flag to _gf_false by
+                         * janitor_task_done */
+                        while (priv->janitor_task_stop) {
+                            (void)pthread_cond_timedwait(&priv->janitor_cond,
+                                                         &priv->janitor_mutex,
+                                                         &sleep_till);
+                            clock_gettime(CLOCK_REALTIME, &sleep_till);
+                            sleep_till.tv_sec += 1;
+                        }
+                    }
+                }
+                pthread_mutex_unlock(&priv->janitor_mutex);
+                GF_FREE(priv->janitor);
+            }
+            priv->janitor = NULL;
             default_notify(this->parents->xlator, GF_EVENT_CHILD_DOWN, data);
         } break;
         default:
@@ -1008,6 +1038,8 @@ posix_init(xlator_t *this)
 
     pthread_mutex_init(&_private->fsync_mutex, NULL);
     pthread_cond_init(&_private->fsync_cond, NULL);
+    pthread_mutex_init(&_private->janitor_mutex, NULL);
+    pthread_cond_init(&_private->janitor_cond, NULL);
     INIT_LIST_HEAD(&_private->fsyncs);
     ret = posix_spawn_ctx_janitor_thread(this);
     if (ret)
@@ -1128,6 +1160,7 @@ posix_fini(xlator_t *this)
         (void)gf_thread_cleanup_xint(priv->disk_space_check);
         priv->disk_space_check = 0;
     }
+
     if (priv->janitor) {
         /*TODO: Make sure the synctask is also complete */
         ret = gf_tw_del_timer(this->ctx->tw->timer_wheel, priv->janitor);
@@ -1135,8 +1168,10 @@ posix_fini(xlator_t *this)
             gf_msg(this->name, GF_LOG_ERROR, errno, P_MSG_TIMER_DELETE_FAILED,
                    "Failed to delete janitor timer");
         }
+        GF_FREE(priv->janitor);
         priv->janitor = NULL;
     }
+
     if (priv->fsyncer) {
         (void)gf_thread_cleanup_xint(priv->fsyncer);
         priv->fsyncer = 0;
@@ -1148,6 +1183,9 @@ posix_fini(xlator_t *this)
     GF_FREE(priv->base_path);
     LOCK_DESTROY(&priv->lock);
     pthread_mutex_destroy(&priv->fsync_mutex);
+    pthread_cond_destroy(&priv->fsync_cond);
+    pthread_mutex_destroy(&priv->janitor_mutex);
+    pthread_cond_destroy(&priv->janitor_cond);
     GF_FREE(priv->hostname);
     GF_FREE(priv->trash_path);
     GF_FREE(priv);
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
index 07169b5..ef5bfd5 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -1432,12 +1432,24 @@ posix_janitor_task_done(int ret, call_frame_t *frame, void *data)
     this = data;
     priv = this->private;
 
+    pthread_mutex_lock(&priv->janitor_mutex);
+    {
+        if (priv->janitor_task_stop) {
+            priv->janitor_task_stop = _gf_false;
+            pthread_cond_signal(&priv->janitor_cond);
+            pthread_mutex_unlock(&priv->janitor_mutex);
+            goto out;
+        }
+    }
+    pthread_mutex_unlock(&priv->janitor_mutex);
+
     LOCK(&priv->lock);
     {
         __posix_janitor_timer_start(this);
     }
     UNLOCK(&priv->lock);
 
+out:
     return 0;
 }
 
@@ -1456,6 +1468,9 @@ posix_janitor_task(void *data)
     old_this = THIS;
     THIS = this;
 
+    if (!priv)
+        goto out;
+
     time(&now);
     if ((now - priv->last_landfill_check) > priv->janitor_sleep_duration) {
         if (priv->disable_landfill_purge) {
@@ -1475,6 +1490,7 @@ posix_janitor_task(void *data)
 
     THIS = old_this;
 
+out:
     return 0;
 }
 
diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h
index b0935a7..64288a7 100644
--- a/xlators/storage/posix/src/posix.h
+++ b/xlators/storage/posix/src/posix.h
@@ -203,6 +203,8 @@ struct posix_private {
     struct list_head fsyncs;
     pthread_mutex_t fsync_mutex;
     pthread_cond_t fsync_cond;
+    pthread_mutex_t janitor_mutex;
+    pthread_cond_t janitor_cond;
     int fsync_queue_count;
 
     enum {
@@ -257,6 +259,7 @@ struct posix_private {
 
     gf_boolean_t fips_mode_rchecksum;
     gf_boolean_t ctime;
+    gf_boolean_t janitor_task_stop;
 };
 
 typedef struct {
-- 
1.8.3.1