Re: [Patch 11/11] ftrace plugin for kernel symbol tracing using HW Breakpoint interfaces - v2

["K.Prasad" <prasad@linux.vnet.ibm.com>]

On Tue, Mar 24, 2009 at 11:03:18PM -0400, Steven Rostedt wrote:
> 
> Hi Prasad,
> 
> On Tue, 24 Mar 2009, K.Prasad wrote:
> 
> > +
> > +static void ksym_trace_print_header(struct seq_file *m)
> > +{
> > +
> > +	seq_puts(m,
> > +		 "#       TASK-PID      CPU#      Symbol         Type    "
> > +		 "Function         \n");
> > +	seq_puts(m,
> > +		 "#          |           |          |              |         "
> > +		 "|            \n");
> > +}
> > +
> > +static enum print_line_t ksym_trace_output(struct trace_iterator *iter)
> > +{
> > +	struct trace_entry *entry = iter->ent;
> > +	struct trace_seq *s = &iter->seq;
> > +	struct trace_ksym *field;
> > +	char str[KSYM_SYMBOL_LEN];
> > +	int ret;
> > +
> > +	trace_assign_type(field, entry);
> 
> Looking at this, you do not test the iter->ent->type. If events or
> trace_printk is running, this will fail your tracer.
> 
> You want to test for your types first, because all trace types will be 
> called for "print_line" function below.
> 
> Thus you want something like
> 
> 	if (entry->type != TRACE_KSYM)
> 		return TRACE_TYPE_UNHANDLED;
> 
> Then the tracer infrastructure could print out the trace event or 
> trace_printk.
> 
> If you only want to print your events and ignore all others. Return
> TRACE_TYPE_HANDLED.
> 
> Note, the if needs to be before the trace_assign_type.
> 
> -- Steve
>

Thanks for pointing that out. I will make those changes and send out a
new patchset.

However, looking at some of the existing plugins I find that many of
them do this check after trace_assign_type() or not do at all (e.g.
trace_branch_print()). They may need similar changes too.

Thanks,
K.Prasad