lkml.org 
[lkml]   [2013]   [Nov]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Why have another variable deciding a tracepoint?
On Fri, 15 Nov 2013 10:17:41 +0200
Terje Bergström <tbergstrom@nvidia.com> wrote:

> On 15.11.2013 06:48, Steven Rostedt wrote:
> > I've been reviewing different users of tracepoints and I stumbled
> > across this:
> >
> > drivers/gpu/host1x/cdma.c: host1x_cdma_push()
> >
> > if (host1x_debug_trace_cmdbuf)
> > trace_host1x_cdma_push(dev_name(cdma_to_channel(cdma)->dev),
> > op1, op2);
> >
> > That host1x_debug_trace_cmdbuf is a variable that gets set by another
> > debugfs file "trace_cmdbuf" that is custom to this driver.
> >
> > Why?
>
> This is because it takes a lot of time to prepare for dumping the cmdbuf
> data, like mapping buffer and unmapping after tracing. We want to avoid
> all that preparation time.
>

So let me get this straight. The recording of the cdma_push() data is
slow, correct? What mapping are you talking about?

trace_host1x_cdma_push(dev_name(cdma_to_channel(cdma)->dev),
op1, op2);

#define cdma_to_channel(cdma) container_of(cdma, struct host1x_channel, cdma)

That just references the cdma field of struct host1x_channel, to get
the dev field, which does dev_name() that just gets the dev->init_name
(if defined, or kobject_name() if not). And the two u32 fields that are
passed in.

The tracepoint assigns with:

TP_fast_assign(
__entry->name = name;
__entry->op1 = op1;
__entry->op2 = op2;
),

So I still have to ask; what do you have to set up and take down here?



> > The tracepoint host1x_cdma_push is already controlled by either ftrace
> > or perf. If it gets enabled by perf or ftrace, it still wont be traced
> > unless we also enable this trace_cmdbuf. Is there some reason for this?
> > I can't figure it out from the change log: 6236451d83a720 ("gpu:
> > host1x: Add debug support").
> >
> > As tracepoints uses jump labels, there is no branch cost associated
> > with them. That is, they are either a direct jump, or a nop (in most
> > cases a nop). But here you added the overhead of a conditional branch
> > depending on this variable.
> >
> > If this is truly needed, then use TRACE_EVENT_CONDITION() for that
> > tracepoint.
>
> TRACE_EVENT_CONDITION() isn't documented, so I don't know how that would
> help.
>

It's documented in the code ;-) (kidding)

Yeah, I need to get that out a bit more. That's actually how I found
this code, I'm doing searches for all the locations that can benefit
from TRACE_EVENT_CONDITION(), and I'm trying to fix them up. I'm still
not convinced that this extra variable checking is required for this
tracepoint.

As you state though, I'll have to get time to document CONDITION() and
how and when to use it.

Thanks,

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

\
 
 \ /
  Last update: 2013-11-15 16:01    [W:0.062 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site