Re: [PATCH] ftrace: add an fsync tracer

[Jason Baron <jbaron@redhat.com>]

On Thu, Nov 06, 2008 at 09:29:03PM +0100, Peter Zijlstra wrote:
> On Thu, 2008-11-06 at 15:19 -0500, Frank Ch. Eigler wrote:
> > Arjan van de Ven <arjan@infradead.org> writes:
> > 
> > > [...]
> > > what is the real need is
> > > 1) Have a trace point in the source
> > > 2) Associate a "formatting function" with that point 
> > >    (which basically transforms the trace parameters to, say, a string)
> > > 3) A way to turn the trace point on/off.
> > 
> > For 1 and 2, it may be worth considering a plain trace_mark() in
> > do_sync().  The complication that makes this uglier than a one-liner
> > is d_path()'s buffer and error handling.
> > 
> >     {
> >     char *buffer = kzalloc (4096, GFP_KERNEL);
> >     trace_mark(fsync, "Process %s is calling fsync on %s\n",
> >                        current->comm,
> >                        ({char *err = d_path (...);
> >                          IS_ERR(err) ? "?" : err;}));
> >     kfree (buffer);
> >     }
> > 
> > With a bit of extension on the marker front, the allocation could be
> > made conditional on the marker being enabled.
> > 
> > 
> > For 3, the kernel could merge a backend that connects arbitrary
> > markers to an ftrace (or whatever) buffer.  Several compact prototypes
> > for the latter exist.
> 
> 
> I prefer we keep using trace points but do what jason has been proposing
> for a while, which is add a format and arg list to the trace point
> definition.
> 
> Something like
> 
> DEFINE_TRACE_FMT(sched_switch,
> 	TPPROTO(struct rq *rq, struct task_struct *prev,
> 		 struct task_struct *next),
> 	TPARGS(rq, prev, next),
> 	TPFMT("%d to %d\n", prev->pid, next->pid));
> 
> Which would be similar to attaching a trace_mark() to the trace point
> and can in these cases save a lot of lines of code.
> 
> Both lttng and the ftrace event tracer can use these default text
> strings.
> 

indeed...done this way I believe marker registration could then be
done directly with the tracepoint itself. That is, callers that
want the tracepoint interface register directly with the tracepoint and get 
called as before. However, callers that want the marker interface register 
directly with the tracepoint and get passed the associated marker string and 
arguments. Then, we could consolidate kernel/marker.c into kernel/tracepoint.c,
which would simlify things, and reduce code duplication.

thanks,

-Jason