2010-12-02 22:48:18

by Steven Rostedt

[permalink] [raw]
Subject: [RFC][PATCH 0/2] tracing: Add conditional to tracepoints

This is an RFC that adds the TP_CONDITION() to the TRACE_EVENT()
code.

There are certain cases that a tracepoint only makes sense if
a specific condition is met. But because we do not want to dirty
the fast path (the non tracing case) with if statements that are
there only to avoid tracing, we just pass the condition variables
to the tracepoint, and let the user filter them out if needed.

A perfect example is the tracepoint sched_wakeup. It traces all
calls to try_to_wake_up() even if it fails to wake up. But if we add:

if (success)
trace_sched_wakeup(p);

We have that "if (success)" tested for every time we call try_to_wake_up().
Even when tracing is not (or never will be) enabled.

This patch set adds a variant TRACE_EVENT_CONDITIONAL()
(and DECLARE_EVENT_CLASS_CONDITIONAL()) that has a "cond" argument.
This argument is encapsulated with "TP_CONDITIONAL()" which turns into:

if (!cond)
return;

This is placed inside the called tracepoint routine, and is only tested
when the trace is enabled. Otherwise it is a nop as tracepoints normally
are when disabled.

Note, another variant of this, is to move the test directly into the
_DO_TRACE() macro, and not call any registered event callbacks. This would
even speed it up faster when tracing is enabled. I did not do this
orginially because I just thought of it now as I wrote this change log ;-)
I'm posting this version now just in case people prefer it instead.

The following patches are in:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

branch: rfc/trace-conditional


Steven Rostedt (2):
tracing: Add TRACE_EVENT_CONDITIONAL()
tracing: Only trace sched_wakeup if it actually work something up

----
include/linux/tracepoint.h | 3 ++
include/trace/define_trace.h | 6 +++
include/trace/events/sched.h | 12 +++---
include/trace/ftrace.h | 70 +++++++++++++++++++++++++++++++----------
4 files changed, 68 insertions(+), 23 deletions(-)


2010-12-02 23:21:34

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [RFC][PATCH 0/2] tracing: Add conditional to tracepoints

On Thu, Dec 02, 2010 at 05:36:54PM -0500, Steven Rostedt wrote:
> This is an RFC that adds the TP_CONDITION() to the TRACE_EVENT()
> code.
>
> There are certain cases that a tracepoint only makes sense if
> a specific condition is met. But because we do not want to dirty
> the fast path (the non tracing case) with if statements that are
> there only to avoid tracing, we just pass the condition variables
> to the tracepoint, and let the user filter them out if needed.
>
> A perfect example is the tracepoint sched_wakeup. It traces all
> calls to try_to_wake_up() even if it fails to wake up. But if we add:
>
> if (success)
> trace_sched_wakeup(p);
>
> We have that "if (success)" tested for every time we call try_to_wake_up().
> Even when tracing is not (or never will be) enabled.
>
> This patch set adds a variant TRACE_EVENT_CONDITIONAL()
> (and DECLARE_EVENT_CLASS_CONDITIONAL()) that has a "cond" argument.
> This argument is encapsulated with "TP_CONDITIONAL()" which turns into:
>
> if (!cond)
> return;
>
> This is placed inside the called tracepoint routine, and is only tested
> when the trace is enabled. Otherwise it is a nop as tracepoints normally
> are when disabled.
>
> Note, another variant of this, is to move the test directly into the
> _DO_TRACE() macro, and not call any registered event callbacks. This would
> even speed it up faster when tracing is enabled. I did not do this
> orginially because I just thought of it now as I wrote this change log ;-)

Hehe :)

Yeah indeed. And that looks fairly possible.


> I'm posting this version now just in case people prefer it instead.
>
> The following patches are in:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
>
> branch: rfc/trace-conditional
>
>
> Steven Rostedt (2):
> tracing: Add TRACE_EVENT_CONDITIONAL()
> tracing: Only trace sched_wakeup if it actually work something up
>
> ----
> include/linux/tracepoint.h | 3 ++
> include/trace/define_trace.h | 6 +++
> include/trace/events/sched.h | 12 +++---
> include/trace/ftrace.h | 70 +++++++++++++++++++++++++++++++----------
> 4 files changed, 68 insertions(+), 23 deletions(-)

2010-12-03 01:42:42

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [RFC][PATCH 0/2] tracing: Add conditional to tracepoints

* Frederic Weisbecker ([email protected]) wrote:
> On Thu, Dec 02, 2010 at 05:36:54PM -0500, Steven Rostedt wrote:
> >
> > Note, another variant of this, is to move the test directly into the
> > _DO_TRACE() macro, and not call any registered event callbacks. This would
> > even speed it up faster when tracing is enabled. I did not do this
> > orginially because I just thought of it now as I wrote this change log ;-)
>
> Hehe :)
>
> Yeah indeed. And that looks fairly possible.

I'd very much prefer if the test is performed before the call, within
the block that contains the stack setup and the tracepoint function
call. Having an utterly low performance impact for the events that are
filtered out is very important for my client's use-cases. Also, moving
it outside of the tracepoint probe function allows us to filter only
once for all the registered handlers.

All it would require is to skip over the function call rather than doing
a "return".

For the rest, it looks nice. :-)

Thanks,

Mathieu

--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com