Re: Why have another variable deciding a tracepoint?

["Terje Bergström" <tbergstrom@nvidia.com>]

On 15.11.2013 16:52, Steven Rostedt wrote:
> 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?

Oops, there's a piece missing that I didn't mention. What we want is a
full trace of what we're sending to the push buffer. See
hw/channel_hw.c/trace_write_gather(). It maps the buffer handed from
user space, dumps it to ftrace and unmaps it. That costs a lot of time
that we don't want to spend in normal operation and hence the special
condition.

trace_host1x_cdma_push() in the code you refer to just makes the dump
complete by adding some overhead opcodes that kernel needs to add. It
doesn't make sense to output that if we don't get the bulk from
trace_write_gather() and that's why it's also checking the same
conditional. It'd just make the trace look corrupted.

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

Sounds good. :-)

Terje