[EMAIL PROTECTED] wrote on Thu, 14 Feb 2008 00:19 -0500:
> [EMAIL PROTECTED] wrote on Wed, 13 Feb 2008 21:25 -0600:
> > What happens when you restart the client daemon? Does the segfault occur
> > with bmi_tcp?
>
> Yeah, I'm getting the same sort of thing, with TCP. 1 client, 1
> md+data server. 2.6.24-rc6. A few "ls -la /pvfs" will crash
> client-core, and it automatically is restarted. Similar sort of
> backtrace. Valgrind doesn't show anything before where it all goes
> bad in Troy's traces.
>
> ==7517== Invalid read of size 8
> ==7517== at 0x4C6989E: qlist_empty (quicklist.h:117)
> ==7517== by 0x4C697DD: PINT_sm_frame (state-machine-fns.c:595)
> ==7517== by 0x4C270AB: completion_list_retrieve_completed
> (client-state-machine.c:140)
> ==7517== by 0x4C281DB: PINT_client_state_machine_testsome
> (client-state-machine.c:694)
> ==7517== by 0x4C285EB: PVFS_sys_testsome (client-state-machine.c:907)
> ==7517== by 0x407BED: process_vfs_requests (pvfs2-client-core.c:2943)
> ==7517== by 0x40A284: main (pvfs2-client-core.c:3379)
> ==7517== Address 0x53BD870 is 80 bytes inside a block of size 176 free'd
> ==7517== at 0x4A0560B: free (vg_replace_malloc.c:233)
> ==7517== by 0x4C696D7: PINT_smcb_free (state-machine-fns.c:551)
> ==7517== by 0x4C2768A: PINT_client_state_machine_post
> (client-state-machine.c:395)
> ==7517== by 0x4C29FE5: PVFS_isys_getattr (sys-getattr.sm:211)
> ==7517== by 0x403D94: post_getattr_request (pvfs2-client-core.c:558)
> ==7517== by 0x408648: handle_unexp_vfs_request (pvfs2-client-core.c:2708)
> ==7517== by 0x407D70: process_vfs_requests (pvfs2-client-core.c:2990)
> ==7517== by 0x40A284: main (pvfs2-client-core.c:3379)
>
> (Parse the second half of this first:)
>
> handle_unexp_vfs_request goes off to post a getattr.
> PVFS_isys_getattr allocs a new smcb. PINT_client_state_machine_post
> starts the SM and it must have finished immediately through a
> successful acache lookup. PINT_client_state_machine_post frees the
> smcb.
>
> (Now the top half:)
>
> Later testsome decides it has a completed smcb. The same one that
> had been freed as above. Although maybe not related.
>
> This is the CVS head _before_ the big cleanup Sam did today. Are
> we forgetting to initialize smcb->frames somewhere related? Looking
> back for suspicious changes.
>
> There's the memmove() fix on s_completion_list[] by Phil back on 15
> jan, but that's obviously a big fix, and it's probably not getting
> triggered either. And a bunch of locking changes that are harmless.
>
> Needs more debugging.
It's definitely this commit that causes the problem. Gotta love
git-bisect. Does it offer any clues?
-- Pete
commit 3d0ba5f2ee107c9cec913d6ba1a56b571c87c2b7
Author: slang <slang>
Date: Thu Feb 7 16:16:35 2008 +0000
fixes to sm code for parallel jumps.
diff --git a/src/common/misc/state-machine-fns.c
b/src/common/misc/state-machine-fns.c
index 6098ea0..c512db1 100644
--- a/src/common/misc/state-machine-fns.c
+++ b/src/common/misc/state-machine-fns.c
@@ -70,30 +70,40 @@ int PINT_state_machine_halt(void)
*/
int PINT_state_machine_terminate(struct PINT_smcb *smcb, job_status_s *r)
{
- struct PINT_frame_s *current_frame;
+ struct PINT_frame_s *my_frame, *f;
job_id_t id;
/* notify parent */
if (smcb->parent_smcb)
{
- assert(smcb->parent_smcb->children_running > 0);
-
- current_frame = qlist_entry(
- &smcb->frames.next, struct PINT_frame_s, link);
- current_frame->error = r->error_code;
-
- if (--smcb->parent_smcb->children_running > 0)
- {
- /* SM is still deferred */
- return SM_ACTION_DEFERRED;
- }
- else
+ gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
+ "[SM Terminating Child]: (%p) %s:%s (error_code: %d)\n",
+ smcb,
+ /* skip pvfs2_ */
+ PINT_state_machine_current_machine_name(smcb),
+ PINT_state_machine_current_state_name(smcb),
+ (int32_t)r->error_code);
+ assert(smcb->parent_smcb->children_running > 0);
+
+ my_frame = qlist_entry(
+ smcb->frames.next, struct PINT_frame_s, link);
+ qlist_for_each_entry(f, &smcb->parent_smcb->frames, link)
+ {
+ if(my_frame->frame == f->frame)
+ {
+ f->error = r->error_code;
+ break;
+ }
+ }
+
+ if (--smcb->parent_smcb->children_running <= 0)
{
/* no more child state machines running, so we can
* start up the parent state machine again
*/
job_null(0, smcb->parent_smcb, 0, r, &id, smcb->context);
}
+ return SM_ACTION_DEFERRED;
}
/* call state machine completion function */
if (smcb->terminate_fn)
@@ -127,10 +137,6 @@ PINT_sm_action PINT_state_machine_invoke(struct PINT_smcb
*smcb,
return SM_ERROR;
}
- /* print pre-call debugging info */
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM invoke smcb %p op %d\n",smcb,(smcb)->op);
-
state_name = PINT_state_machine_current_state_name(smcb);
machine_name = PINT_state_machine_current_machine_name(smcb);
@@ -148,17 +154,10 @@ PINT_sm_action PINT_state_machine_invoke(struct PINT_smcb
*smcb,
switch (retval)
{
case SM_ACTION_TERMINATE :
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM Terminates (%p)\n", smcb);
smcb->op_terminate = 1;
break;
case SM_ACTION_COMPLETE :
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM Returns Complete (%p)\n", smcb);
- break;
case SM_ACTION_DEFERRED :
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM Returns Deferred (%p)\n", smcb);
break;
default :
/* error */
@@ -169,7 +168,7 @@ PINT_sm_action PINT_state_machine_invoke(struct PINT_smcb
*smcb,
/* print post-call debugging info */
gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "[SM Exiting]: (%p) %s:%s (error code: %d), (sm action:
%s)\n",
+ "[SM Exiting]: (%p) %s:%s (error code: %d), (action: %s)\n",
smcb,
/* skip pvfs2_ */
machine_name,
@@ -181,9 +180,6 @@ PINT_sm_action PINT_state_machine_invoke(struct PINT_smcb
*smcb,
{
/* start child SMs */
PINT_sm_start_child_frames(smcb);
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM (%p) started %d child frames\n",
- smcb, smcb->children_running);
if (smcb->children_running > 0)
retval = SM_ACTION_DEFERRED;
else
@@ -210,7 +206,7 @@ PINT_sm_action PINT_state_machine_start(struct PINT_smcb
*smcb, job_status_s *r)
if (ret == SM_ACTION_COMPLETE || ret == SM_ACTION_TERMINATE)
{
/* keep running until state machine deferrs or terminates */
- ret = PINT_state_machine_next(smcb, r);
+ ret = PINT_state_machine_continue(smcb, r);
/* note that if ret == SM_ACTION_TERMINATE, we _don't_ call
* PINT_state_machine_terminate here because that adds the smcb
@@ -239,8 +235,6 @@ PINT_sm_action PINT_state_machine_next(struct PINT_smcb
*smcb, job_status_s *r)
gossip_err("SM next called on invald smcb\n");
return -1;
}
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM next smcb %p op %d\n",smcb,(smcb)->op);
/* loop while invoke of new state returns COMPLETED */
do {
/* loop while returning from nested SM */
@@ -254,12 +248,12 @@ PINT_sm_action PINT_state_machine_next(struct PINT_smcb
*smcb, job_status_s *r)
return -1;
}
transtbl = smcb->current_state->trtbl;
-
- /* for each entry in the transition table there is a return
- * code followed by a next state pointer to the new state.
- * This loops through each entry, checking for a match on the
- * return address, and then sets the new current_state and calls
- * the new state action function */
+
+ /* for each entry in the transition table there is a return
+ * code followed by a next state pointer to the new state.
+ * This loops through each entry, checking for a match on the
+ * return address, and then sets the new current_state and calls
+ * the new state action function */
for (i = 0; transtbl[i].return_value != DEFAULT_ERROR; i++)
{
if (transtbl[i].return_value == r->error_code)
@@ -296,10 +290,8 @@ PINT_sm_action PINT_state_machine_next(struct PINT_smcb
*smcb, job_status_s *r)
if(!smcb->current_state ||
smcb->current_state->trtbl[0].flag == SM_TERM)
{
- /* assume nested machine was invoked without
- * a parent, or nested machine completion results
- * in immediate termination
- */
+ /* assume nested state machine was invoked without
+ * a parent */
return SM_ACTION_TERMINATE;
}
}
@@ -355,6 +347,8 @@ int PINT_state_machine_locate(struct PINT_smcb *smcb)
{
struct PINT_state_s *current_tmp;
struct PINT_state_machine_s *op_sm;
+ const char *state_name;
+ const char *machine_name;
/* check for valid inputs */
if (!smcb || smcb->op < 0 || !smcb->op_get_state_machine)
@@ -363,7 +357,7 @@ int PINT_state_machine_locate(struct PINT_smcb *smcb)
return -PVFS_EINVAL;
}
gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM locate smcb %p op %d\n",smcb,(smcb)->op);
+ "[SM Locating]: (%p) op-id: %d\n",smcb,(smcb)->op);
/* this is a the usage dependant routine to look up the SM */
op_sm = (*smcb->op_get_state_machine)(smcb->op);
if (op_sm != NULL)
@@ -379,6 +373,14 @@ int PINT_state_machine_locate(struct PINT_smcb *smcb)
current_tmp->action.nested)->first_state;
}
smcb->current_state = current_tmp;
+
+ state_name = PINT_state_machine_current_state_name(smcb);
+ machine_name = PINT_state_machine_current_machine_name(smcb);
+
+ gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
+ "[SM Locating]: (%p) located: %s:%s\n",
+ smcb, machine_name, state_name);
+
return 1; /* indicates successful locate */
}
gossip_err("State machine not found for operation %d\n",smcb->op);
@@ -393,8 +395,6 @@ int PINT_state_machine_locate(struct PINT_smcb *smcb)
*/
int PINT_smcb_set_op(struct PINT_smcb *smcb, int op)
{
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM set op smcb %p op %d\n",smcb,op);
smcb->op = op;
return PINT_state_machine_locate(smcb);
}
@@ -499,8 +499,6 @@ int PINT_smcb_alloc(
{
return -PVFS_ENOMEM;
}
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM allocate smcb %p op %d\n",*smcb,op);
/* zero out all members */
memset(*smcb, 0, sizeof(struct PINT_smcb));
@@ -540,8 +538,6 @@ void PINT_smcb_free(struct PINT_smcb *smcb)
{
struct PINT_frame_s *frame_entry, *tmp;
assert(smcb);
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "SM free smcb %p op %d\n", smcb, smcb->op);
qlist_for_each_entry_safe(frame_entry, tmp, &smcb->frames, link)
{
if (frame_entry->frame && frame_entry->task_id == 0)
@@ -564,8 +560,10 @@ void PINT_smcb_free(struct PINT_smcb *smcb)
*/
static struct PINT_state_s *PINT_pop_state(struct PINT_smcb *smcb)
{
- assert(smcb->stackptr > 0);
-
+ if(smcb->stackptr == 0)
+ {
+ return NULL;
+ }
return smcb->state_stack[--smcb->stackptr];
}
@@ -611,9 +609,6 @@ void *PINT_sm_frame(struct PINT_smcb *smcb, int index)
next = next->next;
}
frame_entry = qlist_entry(next, struct PINT_frame_s, link);
- gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "FRAME GET smcb %p index %d -> frame: %p\n",
- smcb, index, frame_entry->frame);
return frame_entry->frame;
}
}
@@ -627,9 +622,8 @@ int PINT_sm_push_frame(struct PINT_smcb *smcb, int task_id,
void *frame_p)
{
struct PINT_frame_s *newframe;
gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "PUSH FRAME %p onto smcb %p\n",
- frame_p, smcb);
-
+ "[SM Frame PUSH]: (%p) frame: %p\n",
+ smcb, frame_p);
newframe = malloc(sizeof(struct PINT_frame_s));
if(!newframe)
{
@@ -638,15 +632,22 @@ int PINT_sm_push_frame(struct PINT_smcb *smcb, int
task_id, void *frame_p)
newframe->task_id = task_id;
newframe->frame = frame_p;
qlist_add(&newframe->link, &smcb->frames);
+ smcb->frame_count++;
return 0;
}
/* Function: PINT_sm_pop_frame
- * Params: pointer to an smcb pointer
+ * Params: smcb - pointer to an smcb pointer
+ * task_id - the task id of this frame
+ * error_code - the frame's error if there was one.
+ * remaining - count of remaining frames on the smcb.
* Returns: frame pointer
* Synopsis: pops a frame pointer from the frame_stack and returns it
*/
-void *PINT_sm_pop_frame(struct PINT_smcb *smcb, int *error_code)
+void *PINT_sm_pop_frame(struct PINT_smcb *smcb,
+ int *task_id,
+ int *error_code,
+ int *remaining)
{
struct PINT_frame_s *frame_entry;
void *frame;
@@ -656,17 +657,24 @@ void *PINT_sm_pop_frame(struct PINT_smcb *smcb, int
*error_code)
return NULL;
}
- frame_entry = qlist_entry(&smcb->frames.next, struct PINT_frame_s, link);
+ frame_entry = qlist_entry(smcb->frames.next, struct PINT_frame_s, link);
qlist_del(smcb->frames.next);
+ smcb->frame_count--;
+
+ if(remaining)
+ {
+ *remaining = smcb->frame_count;
+ }
frame = frame_entry->frame;
*error_code = frame_entry->error;
+ *task_id = frame_entry->task_id;
free(frame_entry);
gossip_debug(GOSSIP_STATE_MACHINE_DEBUG,
- "POP FRAME %p from smcb %p\n",
- frame, smcb);
+ "[SM Frame POP]: (%p) frame: %p\n",
+ smcb, frame);
return frame;
}
@@ -710,11 +718,22 @@ static void PINT_sm_start_child_frames(struct PINT_smcb
*smcb)
struct PINT_smcb *new_sm;
struct PINT_frame_s *frame_entry;
job_status_s r;
+ struct qlist_head *f;
assert(smcb);
- qlist_for_each_entry(frame_entry, &smcb->frames, link)
+ memset(&r, 0, sizeof(job_status_s));
+
+ qlist_for_each(f, &smcb->frames)
{
+ /* skip the last since its the parent frame */
+ if(f->next == &smcb->frames)
+ {
+ break;
+ }
+
+ frame_entry = qlist_entry(f, struct PINT_frame_s, link);
+
/* allocate smcb */
PINT_smcb_alloc(&new_sm, smcb->op, 0, NULL,
child_sm_frame_terminate, smcb->context);
diff --git a/src/common/misc/state-machine.h b/src/common/misc/state-machine.h
index 86f3220..cddaead 100644
--- a/src/common/misc/state-machine.h
+++ b/src/common/misc/state-machine.h
@@ -68,6 +68,7 @@ typedef struct PINT_smcb
struct PINT_state_s *state_stack[PINT_STATE_STACK_SIZE];
struct qlist_head frames;
+ int frame_count;
/* usage specific routinet to look up SM from OP */
struct PINT_state_machine_s *(*op_get_state_machine)(int);
@@ -152,14 +153,6 @@ enum {
#define SM_STATE_RETURN -1
#define SM_NESTED_STATE 1
-#define SM_NONE 0
-#define SM_NEXT 1
-#define SM_RETURN 2
-#define SM_EXTERN 3
-#define SM_NESTED 5
-#define SM_JUMP 6
-#define SM_TERMINATE 7
-
/* Prototypes for functions provided by user */
int PINT_state_machine_complete(void *);
@@ -196,9 +189,10 @@ int PINT_smcb_alloc(struct PINT_smcb **, int, int,
void PINT_smcb_free(struct PINT_smcb *);
void *PINT_sm_frame(struct PINT_smcb *, int);
int PINT_sm_push_frame(struct PINT_smcb *smcb, int task_id, void *frame_p);
-void *PINT_sm_pop_frame(struct PINT_smcb *smcb, int *error_code);
-
-int PINT_sm_pop_error(PINT_smcb *smcb, PVFS_error ret);
+void *PINT_sm_pop_frame(struct PINT_smcb *smcb,
+ int *task_id,
+ int *error_code,
+ int *remaining);
/* This macro is used in calls to PINT_sm_fram() */
#define PINT_FRAME_CURRENT 0
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
