Message-ID: <1272550009.9739.136.camel@gandalf.stny.rr.com>
Date:	Thu, 29 Apr 2010 10:06:49 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Mathieu Desnoyers <compudj@...stal.dyndns.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Li Zefan <lizf@...fujitsu.com>,
	Masami Hiramatsu <mhiramat@...hat.com>,
	Christoph Hellwig <hch@....de>
Subject: Re: [PATCH 04/10][RFC] tracing: Remove per event trace registering

On Thu, 2010-04-29 at 09:36 -0400, Mathieu Desnoyers wrote:

> > Instead of calling register_trace_##name that is created for each
> > tracepoint, we now call the tracepoint_probe_register() directly in the
> > C file with the generated probe.
> > 
> > Both the probe and the tracepoint are created from the same data. I'm
> > not seeing where you want to add this check.
> 
> So if they are created from the same data, we can expect this test to
> always pass, which is good (and expected).
> 
> I'd add this extra check before casting the callback to (void *) before
> it is passed to tracepoint_probe_register(). Let's just call this
> internal preprocessing macro integrity check. As long as it does not add
> a runtime cost, there is no reason not to put this extra check.

The problem is, the cast is now performed in a C file for all events.
There's no way to know what to cast it to there. This is out of the
automation of the macro.

We use to have the cast check by creating code that would create the
"register_trace_##call", and the typecheck was doing by passing the data
to this function. But we removed this code out of the per event, it was
adding lots of text footprint, and moved it to one single function that
handles all events. It is just expected that the callback created
matches the function it was done.

If you are overly paranoid, we could create a special function that
tests that the callback format that is created matches the tracepoint
that is created, and make it so GCC sees that nothing calls it and
removes it at final link. But I still see this as a waste.


The tracepoint is created in include/linux/tracepoint.h:

#define TRACE_EVENT(name, proto, args, struct, assign, print)	\
	DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))

The callback is created in include/trace/ftrace.h:

#undef TRACE_EVENT
#define TRACE_EVENT(name, proto, args, tstuct, assign, print)	\
	DECLARE_EVENT_CLASS(name,				\
				PARAMS(proto),			\
				PARAMS(args),			\
				PARAMS(tstruct),		\
				PARAMS(assign),			\
				PARAMS(print));			\
	DEFINE_EVENT(name, name, PARAMS(proto), PARAMS(args));

#undef DECLARE_EVENT_CLASS
#define DECLARE_EVENT_CLASS(call, proto, args, tstruct, assign, print)	\
									\
static notrace void							\
ftrace_raw_event_##call(proto,						\
			struct ftrace_event_call *event_call)		\
[...]


Thus the "proto" field of the TRACE_EVENT() is used to make both the
tracepoint and the callback. We add the struct ftrace_event_call
*event_call which is the data we pass to the callback.

Now, where this gets called is in kernel/trace/trace_events.c:

	tracepoint_probe_register(call->name,
				  call->class->probe,
				  call);

This is where we lose the typecheck. So my question is... where do you
want to put in a check?

-- Steve

	


--
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/

Powered by blists - more mailing lists