From 8ad92bbde3a17ce9aa44e32ae42df5db259fa2ce Mon Sep 17 00:00:00 2001 From: Krishnan Parthasarathi Date: Fri, 5 Jun 2015 10:33:11 +0530 Subject: 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: 1234408 Change-Id: I7e43bccd3994cd9184ab982dba3dbc10618f0d94 Signed-off-by: Krishnan Parthasarathi Reviewed-on: http://review.gluster.org/11095 Reviewed-by: Niels de Vos Tested-by: Gluster Build System Reviewed-by: Pranith Kumar Karampuri Tested-by: NetBSD Build System (cherry picked from commit 79e4c7b2fad6db15863efb4e979525b1bd4862ea) Reviewed-on: http://review.gluster.org/11352 --- libglusterfs/src/common-utils.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'libglusterfs/src/common-utils.c') diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c index 08694e483cf..851edad6667 100644 --- a/libglusterfs/src/common-utils.c +++ b/libglusterfs/src/common-utils.c @@ -573,6 +573,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 @@ -588,23 +589,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; } } -- cgit