From: Frederic Weisbecker on
On Wed, Feb 10, 2010 at 02:52:47PM -0800, Tim Bird wrote:
> int trace_graph_entry(struct ftrace_graph_ent *trace)
> {
> struct trace_array *tr = graph_array;
> @@ -221,6 +226,10 @@ int trace_graph_entry(struct ftrace_graph_ent *trace)
> if (!ftrace_graph_addr(trace->func))
> return 0;
>
> + /* if tracing duration, only save function exits (omit function entries in log) */
> + if (tracing_thresh)
> + return 1;
> +


Instead of having yet another check here, may be should we
have a dedicated stub trace_graph_entry?



> @@ -254,6 +263,10 @@ static void __trace_graph_return(struct trace_array *tr,
> if (unlikely(__this_cpu_read(per_cpu_var(ftrace_cpu_disabled))))
> return;
>
> + if (tracing_thresh &&
> + (trace->rettime - trace->calltime < tracing_thresh))
> + return;
> +



And perhaps we can do the same for the return handler?
We could have a trace_graph_return_threshold that
performs the above check and then relies on trace_graph_return.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on
On Mon, Feb 22, 2010 at 09:57:43AM -0500, Steven Rostedt wrote:
> On Sat, 2010-02-20 at 15:43 +0100, Frederic Weisbecker wrote:
> >
> > Instead of having yet another check here, may be should we
> > have a dedicated stub trace_graph_entry?
> >
> >
> >
> > > @@ -254,6 +263,10 @@ static void __trace_graph_return(struct trace_array *tr,
> > > if (unlikely(__this_cpu_read(per_cpu_var(ftrace_cpu_disabled))))
> > > return;
> > >
> > > + if (tracing_thresh &&
> > > + (trace->rettime - trace->calltime < tracing_thresh))
> > > + return;
> > > +
> >
> >
> >
> > And perhaps we can do the same for the return handler?
> > We could have a trace_graph_return_threshold that
> > performs the above check and then relies on trace_graph_return.
>
> So you mean to register a different type of function to the graph tracer
> if trace_thresh is enabled? That does sound like a better idea.


Yeah, this is going to optimize both types of tracing. And I would
also like to prevent from adding new checks in the common graph
tracing if possible. User's cpus and cachelines deserve better :)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on
On Mon, Feb 22, 2010 at 09:48:03AM -0500, Steven Rostedt wrote:
> On Sat, 2010-02-20 at 15:21 +0100, Frederic Weisbecker wrote:
>
> >
> > Actually why do we encumber with both tracing_thresh and the funcgraph-exit
> > option?
> >
> > We could just have the output and the record check tracing_thresh instead
> > of the funcgraph-exit option.
>
> You mean just use tracing_thresh during the tracing? We could perhaps
> also change the code (and I think this would be beneficial even without
> this change) to print the function on exit if it did not have a entry.
>
> That is, in the reading of the trace, keep a depth pointer. For every
> "entry" we hit, we add one, and for every "exit" we subtract one (per
> cpu). If we go negative, we keep the counter at zero, but write
> something like:
>
>
> } (sys_write)



Why not, looks like a good idea.


>
> Because I always hate it when a trace starts with a bunch of "}" and I
> have no idea what functions they are. This would fix that.


Agreed.



> Yeah, Tim's trace would have only "} (sys_write)" type syntax but that's
> probably fine.



May be. Or we can couple your above solution and check if tracing_thresh > 0,
in which case we just zap the "}".

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