linux-kernel - Re: [PATCH] ftrace: add an fsync tracer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081106211856.GC3167@redhat.com>
Date:	Thu, 6 Nov 2008 16:18:56 -0500
From:	Jason Baron <jbaron@...hat.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	"Frank Ch. Eigler" <fche@...hat.com>,
	Arjan van de Ven <arjan@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	linux-kernel@...r.kernel.org, mingo@...e.hu, alan@...hat.com,
	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
Subject: Re: [PATCH] ftrace: add an fsync tracer

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@...radead.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/