stack: use list_head for managing frames

PROBLEM
--------

statedump requests that traverse call frames of all call stacks in
execution may race with a STACK_RESET on a stack.  This could crash the
corresponding glusterfs process. For e.g, recently we observed this in a
regression test case tests/basic/afr/sparse-self-heal.t.

FIX
---

gf_proc_dump_pending_frames takes a (TRY_LOCK) call_pool->lock before
iterating through call frames of all call stacks in progress.  With this
fix, STACK_RESET removes its call frames under the same lock.

Additional info
----------------

This fix makes call_stack_t to use struct list_head in place of custom
doubly-linked list implementation. This makes call_frame_t manipulation
easier to maintain in the context of STACK_WIND et al.

BUG: 1229658
Change-Id: I7e43bccd3994cd9184ab982dba3dbc10618f0d94
Signed-off-by: Krishnan Parthasarathi <kparthas@redhat.com>
Reviewed-on: http://review.gluster.org/11095
Reviewed-by: Niels de Vos <ndevos@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Tested-by: NetBSD Build System <jenkins@build.gluster.org>

1 files changed, 32 insertions, 43 deletions

diff --git a/libglusterfs/src/stack.c b/libglusterfs/src/stack.c
index a7fb9510399..2488cc7f0ba 100644
--- a/libglusterfs/src/stack.c
+++ b/libglusterfs/src/stack.c
@@ -11,25 +11,11 @@
 #include "statedump.h"
 #include "stack.h"
 
-static inline
-int call_frames_count (call_frame_t *call_frame)
-{
-        call_frame_t *pos;
-        int32_t count = 0;
-
-        if (!call_frame)
-                return count;
-
-        for (pos = call_frame; pos != NULL; pos = pos->next)
-                count++;
-
-        return count;
-}
-
 call_frame_t *
 create_frame (xlator_t *xl, call_pool_t *pool)
 {
         call_stack_t    *stack = NULL;
+        call_frame_t    *frame = NULL;
 
         if (!xl || !pool) {
                 return NULL;
@@ -39,18 +25,31 @@ create_frame (xlator_t *xl, call_pool_t *pool)
         if (!stack)
                 return NULL;
 
+        INIT_LIST_HEAD (&stack->myframes);
+
+        frame = mem_get0 (pool->frame_mem_pool);
+        if (!frame) {
+                mem_put (stack);
+                return NULL;
+        }
+
+        frame->root = stack;
+        frame->this = xl;
+        LOCK_INIT (&frame->lock);
+        INIT_LIST_HEAD (&frame->frames);
+        list_add (&frame->frames, &stack->myframes);
+
         stack->pool = pool;
-        stack->frames.root = stack;
-        stack->frames.this = xl;
         stack->ctx = xl->ctx;
 
         if (stack->ctx->measure_latency) {
                 if (gettimeofday (&stack->tv, NULL) == -1)
                         gf_log ("stack", GF_LOG_ERROR, "gettimeofday () failed."
                                 " (%s)", strerror (errno));
-                memcpy (&stack->frames.begin, &stack->tv, sizeof (stack->tv));
+                memcpy (&frame->begin, &stack->tv, sizeof (stack->tv));
         }
 
+
         LOCK (&pool->lock);
         {
                 list_add (&stack->all_frames, &pool->all_frames);
@@ -58,10 +57,9 @@ create_frame (xlator_t *xl, call_pool_t *pool)
         }
         UNLOCK (&pool->lock);
 
-        LOCK_INIT (&stack->frames.lock);
         LOCK_INIT (&stack->stack_lock);
 
-        return &stack->frames;
+        return frame;
 }
 
 void
@@ -136,7 +134,7 @@ gf_proc_dump_call_stack (call_stack_t *call_stack, const char *key_buf,...)
         char prefix[GF_DUMP_MAX_BUF_LEN];
         va_list ap;
         call_frame_t *trav;
-        int32_t cnt, i;
+        int32_t i = 1, cnt = 0;
         char timestr[256] = {0,};
 
         if (!call_stack)
@@ -144,13 +142,12 @@ gf_proc_dump_call_stack (call_stack_t *call_stack, const char *key_buf,...)
 
         GF_ASSERT (key_buf);
 
-        cnt = call_frames_count(&call_stack->frames);
-
         memset(prefix, 0, sizeof(prefix));
         va_start(ap, key_buf);
         vsnprintf(prefix, GF_DUMP_MAX_BUF_LEN, key_buf, ap);
         va_end(ap);
 
+        cnt = call_frames_count (call_stack);
         if (call_stack->ctx->measure_latency) {
                 gf_time_fmt (timestr, sizeof timestr, call_stack->tv.tv_sec,
                              gf_timefmt_FT);
@@ -176,14 +173,10 @@ gf_proc_dump_call_stack (call_stack_t *call_stack, const char *key_buf,...)
         gf_proc_dump_write("type", "%d", call_stack->type);
         gf_proc_dump_write("cnt", "%d", cnt);
 
-        trav = &call_stack->frames;
-
-        for (i = 1; i <= cnt; i++) {
-                if (trav) {
-                        gf_proc_dump_add_section("%s.frame.%d", prefix, i);
-                        gf_proc_dump_call_frame(trav, "%s.frame.%d", prefix, i);
-                        trav = trav->next;
-                }
+        list_for_each_entry (trav, &call_stack->myframes, frames) {
+                gf_proc_dump_add_section("%s.frame.%d", prefix, i);
+                gf_proc_dump_call_frame(trav, "%s.frame.%d", prefix, i);
+                i++;
         }
 }
 
@@ -317,14 +310,13 @@ gf_proc_dump_call_stack_to_dict (call_stack_t *call_stack,
         int             ret = -1;
         char            key[GF_DUMP_MAX_BUF_LEN] = {0,};
         call_frame_t    *trav = NULL;
-        int             count = 0;
         int             i = 0;
+        int             count = 0;
 
         if (!call_stack || !dict)
                 return;
 
-        count = call_frames_count (&call_stack->frames);
-
+        count = call_frames_count (call_stack);
         memset (key, 0, sizeof (key));
         snprintf (key, sizeof (key), "%s.uid", prefix);
         ret = dict_set_int32 (dict, key, call_stack->uid);
@@ -372,15 +364,12 @@ gf_proc_dump_call_stack_to_dict (call_stack_t *call_stack,
         if (ret)
                 return;
 
-        trav = &call_stack->frames;
-        for (i = 0; i < count; i++) {
-                if (trav) {
-                        memset (key, 0, sizeof (key));
-                        snprintf (key, sizeof (key), "%s.frame%d",
-                                  prefix, i);
-                        gf_proc_dump_call_frame_to_dict (trav, key, dict);
-                        trav = trav->next;
-                }
+        list_for_each_entry (trav, &call_stack->myframes, frames) {
+                memset (key, 0, sizeof (key));
+                snprintf (key, sizeof (key), "%s.frame%d",
+                          prefix, i);
+                gf_proc_dump_call_frame_to_dict (trav, key, dict);
+                i++;
         }
 
         return;