Re: [PATCH -tip] ftrace: fix trace_output

[Steven Rostedt]

Hi Lai,

On Mon, 12 Jan 2009, Lai Jiangshan wrote:

> 
> Impact: fix bug for handling partial line
> 
> trace_seq_printf(), seq_print_userip_objs(), ... return
> 0          -- partial line was written
> other(>0)  -- success
> 
> duplicate output is also removed in trace_print_raw().
> 
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> ---
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index df0c25c..c688c16 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -443,9 +443,9 @@ trace_fn_raw(struct trace_seq *s, struct trace_entry *entry, int flags)
>  	if (trace_seq_printf(s, "%x %x\n",
>  			     field->ip,
>  			     field->parent_ip))
> -		return TRACE_TYPE_PARTIAL_LINE;
> +		return 0;

Actually, could you change it to match the rest of the file.

That is to add the missing "!" instead of changing the two returns.

>  
> -	return 0;
> +	return TRACE_TYPE_PARTIAL_LINE;
>  }
>  
>  static int
> @@ -505,9 +505,9 @@ trace_ctxwake_print(struct trace_seq *s, struct trace_entry *entry, int flags,
>  			     field->next_pid,
>  			     field->next_prio,
>  			     T, comm))
> -		return TRACE_TYPE_PARTIAL_LINE;
> +		return 0;

Here too.

>  
> -	return 0;
> +	return TRACE_TYPE_PARTIAL_LINE;
>  }
>  
>  static int
> @@ -542,9 +542,9 @@ trace_ctxwake_raw(struct trace_seq *s, struct trace_entry *entry, int flags,
>  			     field->next_pid,
>  			     field->next_prio,
>  			     T))
> -		return TRACE_TYPE_PARTIAL_LINE;
> +		return 0;

and here.

>  
> -	return 0;
> +	return TRACE_TYPE_PARTIAL_LINE;
>  }
>  
>  static int
> @@ -643,9 +643,9 @@ trace_special_print(struct trace_seq *s, struct trace_entry *entry, int flags)
>  			     field->arg1,
>  			     field->arg2,
>  			     field->arg3))
> -		return TRACE_TYPE_PARTIAL_LINE;
> +		return 0;

and here.

>  
> -	return 0;
> +	return TRACE_TYPE_PARTIAL_LINE;
>  }
>  
>  static int
> @@ -697,13 +697,13 @@ trace_stack_print(struct trace_seq *s, struct trace_entry *entry, int flags)
>  
>  	for (i = 0; i < FTRACE_STACK_ENTRIES; i++) {
>  		if (i) {
> -			if (trace_seq_puts(s, " <= "))
> +			if (!trace_seq_puts(s, " <= "))
>  				goto partial;

This is good.

>  
> -			if (seq_print_ip_sym(s, field->caller[i], flags))
> +			if (!seq_print_ip_sym(s, field->caller[i], flags))
>  				goto partial;
>  		}
> -		if (trace_seq_puts(s, "\n"))
> +		if (!trace_seq_puts(s, "\n"))
>  			goto partial;
>  	}
>  
> @@ -731,10 +731,10 @@ trace_user_stack_print(struct trace_seq *s, struct trace_entry *entry,
>  
>  	trace_assign_type(field, entry);
>  
> -	if (seq_print_userip_objs(field, s, flags))
> +	if (!seq_print_userip_objs(field, s, flags))
>  		goto partial;

This is also good.

>  
> -	if (trace_seq_putc(s, '\n'))
> +	if (!trace_seq_putc(s, '\n'))
>  		goto partial;

also good.

>  
>  	return 0;
> @@ -760,10 +760,10 @@ trace_print_print(struct trace_seq *s, struct trace_entry *entry, int flags)
>  
>  	trace_assign_type(field, entry);
>  
> -	if (seq_print_ip_sym(s, field->ip, flags))
> +	if (!seq_print_ip_sym(s, field->ip, flags))
>  		goto partial;
>  
> -	if (trace_seq_printf(s, ": %s", field->buf))
> +	if (!trace_seq_printf(s, ": %s", field->buf))
>  		goto partial;
>  
>  	return 0;
> @@ -779,10 +779,7 @@ trace_print_raw(struct trace_seq *s, struct trace_entry *entry, int flags)
>  
>  	trace_assign_type(field, entry);
>  
> -	if (seq_print_ip_sym(s, field->ip, flags))
> -		goto partial;
> -
> -	if (trace_seq_printf(s, "# %lx %s", field->ip, field->buf))
> +	if (!trace_seq_printf(s, "# %lx %s", field->ip, field->buf))
>  		goto partial;

The above looks good.

>  
>  	return 0;
> diff --git a/kernel/trace/trace_output.h b/kernel/trace/trace_output.h
> index ecab4ea..b2c1461 100644
> --- a/kernel/trace/trace_output.h
> +++ b/kernel/trace/trace_output.h
> @@ -45,14 +45,14 @@ trace_nop_print(struct trace_seq *s, struct trace_entry *entry, int flags);
>  #define SEQ_PUT_FIELD_RET(s, x)				\
>  do {							\
>  	if (!trace_seq_putmem(s, &(x), sizeof(x)))	\
> -		return 0;				\
> +		return TRACE_TYPE_PARTIAL_LINE;		\
>  } while (0)
>  
>  #define SEQ_PUT_HEX_FIELD_RET(s, x)			\
>  do {							\
>  	BUILD_BUG_ON(sizeof(x) > MAX_MEMHEX_BYTES);	\
>  	if (!trace_seq_putmem_hex(s, &(x), sizeof(x)))	\
> -		return 0;				\
> +		return TRACE_TYPE_PARTIAL_LINE;		\
>  } while (0)

Ah, that is correct as well.

Thanks,

-- Steve

>  
>  #endif
> 
> 
> 
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/