[ltt-dev] [PATCH] Fix initial state bug

[compudj@krystal.dyndns.org (Mathieu Desnoyers)]

* francis.giraldeau at usherbrooke.ca (francis.giraldeau at usherbrooke.ca) wrote:
> From: Francis Giraldeau <francis.giraldeau@usherbrooke.ca>
> 
> This patch fixes a bug with the initial state in the control flow viewer. When
> a process was started before starting the trace, in some situation all the
> states of this process was wrong, because of a unknown initial state.
> 
> The patch adds string tables that matches the enums sequence in lttng-modules,
> to convert enum value in the state dump to quark.
> 
> Update :
>     * more specific macro name
>     * move static string tables to state.c
> ---
>  lttv/lttv/state.c |   55 +++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 41 insertions(+), 14 deletions(-)
> 
> diff --git a/lttv/lttv/state.c b/lttv/lttv/state.c
> index bd3f64d..32d68f2 100644
> --- a/lttv/lttv/state.c
> +++ b/lttv/lttv/state.c
> @@ -46,6 +46,43 @@
>  
>  #define PREALLOCATED_EXECUTION_STACK 10
>  
> +/* get a given quark from lttng module enum value */
> +#define ltt_enum_quark(e, f, names) (g_quark_from_string(names[ltt_event_get_unsigned(e, f)]))

Please use a static inline rather than a preprocessor macro for this. It
comes with type checking and removes odd macro corner-cases.

> +
> +/*
> + * Quark strings that matches lttng module enums found in statedump
> + * Matching enums are from lttng-modules/ltt-statedump.c
> + */
> +static const char *const lttng_thread_type_names[] = {
> +        "USER_THREAD",     // LTTNG_USER_THREAD
> +        "KERNEL_THREAD"    // LTTNG_KERNEL_THREAD
> +};
> +
> +static const char *const lttng_execution_mode_names[] = {
> +        "USER_MODE",       // LTTNG_USER_MODE
> +        "SYSCALL",         // LTTNG_SYSCALL
> +        "TRAP",            // LTTNG_TRAP
> +        "IRQ",             // LTTNG_IRQ
> +        "SOFTIRQ",         // LTTNG_SOFTIRQ
> +        "UNKNOWN"          // LTTNG_MODE_UNKNOWN
> +};
> +
> +static const char *const lttng_execution_submode_names[] = {
> +        "UNKNOWN",         // LTTNG_NONE
> +        "NONE"             // LTTNG_UNKNOWN
> +};
> +
> +static const char *const lttng_process_status_names[] = {
> +        "",                 // LTTNG_UNNAMED
> +        "WAIT_FORK",        // LTTNG_WAIT_FORK
> +        "WAIT_CPU",         // LTTNG_WAIT_CPU
> +        "EXIT",             // LTTNG_EXIT
> +        "ZOMBIE",           // LTTNG_ZOMBIE
> +        "WAIT",             // LTTNG_WAIT
> +        "RUN",              // LTTNG_RUN
> +        "DEAD"              // LTTNG_DEAD
> +};
> +
>  /* Channel Quarks */
>  
>  GQuark
> @@ -3440,21 +3477,19 @@ static gboolean enum_process_state(void *hook_data, void *call_data)
>  
>  	/* type */
>  	f = lttv_trace_get_hook_field(th, 3);
> -	type = ltt_enum_string_get(f, ltt_event_get_unsigned(e, f));
> -
> -	//FIXME: type is rarely used, enum must match possible types.
> +	type = ltt_enum_quark(e, f, lttng_thread_type_names);
>  
>  	/* mode */
>  	f = lttv_trace_get_hook_field(th, 4);
> -	mode = ltt_enum_string_get(f,ltt_event_get_unsigned(e, f));
> +	mode = ltt_enum_quark(e, f, lttng_execution_mode_names);
>  
>  	/* submode */
>  	f = lttv_trace_get_hook_field(th, 5);
> -	submode = ltt_enum_string_get(f, ltt_event_get_unsigned(e, f));
> +	submode = ltt_enum_quark(e, f, lttng_execution_submode_names);
>  
>  	/* status */
>  	f = lttv_trace_get_hook_field(th, 6);
> -	status = ltt_enum_string_get(f, ltt_event_get_unsigned(e, f));
> +	status = ltt_enum_quark(e, f, lttng_process_status_names);
>  
>  	/* TGID */
>  	f = lttv_trace_get_hook_field(th, 7);
> @@ -3503,11 +3538,9 @@ static gboolean enum_process_state(void *hook_data, void *call_data)
>  				es->t = LTTV_STATE_MODE_UNKNOWN;
>  				es->s = LTTV_STATE_UNNAMED;
>  				es->n = LTTV_STATE_SUBMODE_UNKNOWN;
> -#if 0
>  				es->t = LTTV_STATE_SYSCALL;
>  				es->s = status;
>  				es->n = submode;
> -#endif //0

Hrm ? you're leaving the unknown and then changing the state ? What for ?

How do you know the thread is not in interrupt or trap ? (see comment
above)

>  			} else {
>  				/* User space process :
>  				 * bottom : user mode
> @@ -3528,13 +3561,10 @@ static gboolean enum_process_state(void *hook_data, void *call_data)
>  				es->t = LTTV_STATE_MODE_UNKNOWN;
>  				es->s = LTTV_STATE_UNNAMED;
>  				es->n = LTTV_STATE_SUBMODE_UNKNOWN;
> -	#if 0
>  				es->t = LTTV_STATE_USER_MODE;
>  				es->s = status;
>  				es->n = submode;
> -	#endif //0

Same here (about double-change).

Also, how would a thread, running in user-mode, and interrupted, be
listed by the state dump ? I think marking it as "user mode" is wrong
here. We don't know this for sure.

Julien looked at these things in great length lately, you might want to
discuss with him.

Thanks,

Mathieu

>  			}
> -	#if 0
>  			/* UNKNOWN STATE */
>  			{
>  				es = process->state = &g_array_index(process->execution_stack,
> @@ -3543,7 +3573,6 @@ static gboolean enum_process_state(void *hook_data, void *call_data)
>  				es->s = LTTV_STATE_UNNAMED;
>  				es->n = LTTV_STATE_SUBMODE_UNKNOWN;
>  			}
> -	#endif //0
>  		} else {
>  			/* The process has already been created :
>  			 * Probably was forked while dumping the process state or
> @@ -3554,14 +3583,12 @@ static gboolean enum_process_state(void *hook_data, void *call_data)
>  			process->name = g_quark_from_string(command);
>  			process->type = type;
>  			es = &g_array_index(process->execution_stack, LttvExecutionState, 0);
> -#if 0
>  			if(es->t == LTTV_STATE_MODE_UNKNOWN) {
>  				if(type == LTTV_STATE_KERNEL_THREAD)
>  					es->t = LTTV_STATE_SYSCALL;
>  				else
>  					es->t = LTTV_STATE_USER_MODE;
>  			}
> -#endif //0
>  			/* Don't mess around with the stack, it will eventually become
>  			 * ok after the end of state dump. */
>  		}
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com