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, 9 insertions, 12 deletions

diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index 48a1c41efcf..1b02ca5f217 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -568,6 +568,7 @@ gf_print_trace (int32_t signum, glusterfs_ctx_t *ctx)
 {
         char         msg[1024] = {0,};
         char         timestr[64] = {0,};
+        call_stack_t *stack = NULL;
 
         /* Now every gf_log call will just write to a buffer and when the
          * buffer becomes full, its written to the log-file. Suppose the process
@@ -583,23 +584,19 @@ gf_print_trace (int32_t signum, glusterfs_ctx_t *ctx)
         /* Pending frames, (if any), list them in order */
         gf_msg_plain_nomem (GF_LOG_ALERT, "pending frames:");
         {
-                struct list_head *trav =
-                                ((call_pool_t *)ctx->pool)->all_frames.next;
-                while (trav != (&((call_pool_t *)ctx->pool)->all_frames)) {
-                        call_frame_t *tmp =
-                                (call_frame_t *)(&((call_stack_t *)trav)->frames);
-                        if (tmp->root->type == GF_OP_TYPE_FOP)
+                /* FIXME: traversing stacks outside pool->lock */
+                list_for_each_entry (stack, &ctx->pool->all_frames,
+                                     all_frames) {
+                        if (stack->type == GF_OP_TYPE_FOP)
                                 sprintf (msg,"frame : type(%d) op(%s)",
-                                         tmp->root->type,
-                                         gf_fop_list[tmp->root->op]);
+                                         stack->type,
+                                         gf_fop_list[stack->op]);
                         else
                                 sprintf (msg,"frame : type(%d) op(%d)",
-                                         tmp->root->type,
-                                         tmp->root->op);
+                                         stack->type,
+                                         stack->op);
 
                         gf_msg_plain_nomem (GF_LOG_ALERT, msg);
-
-                        trav = trav->next;
                 }
         }