Re: [PATCH] perf script: Print callchains and symbols if they exist - v2

[Frederic Weisbecker <fweisbec@gmail.com>]

On Tue, Nov 19, 2013 at 09:07:37PM -0700, David Ahern wrote:
> The intent of perf-script is to dump the events and information
> in the file. H/W, S/W and raw events all dump callchains if they
> are present; might as well make that the default for tracepoints
> too.
> 
> v2: Only add options for sym, dso and ip if callchains are present
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>

Thanks, looks good, just a few things:

> ---
>  tools/perf/builtin-script.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index b392770766dd..9f3ba4404a14 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -280,6 +280,30 @@ static int perf_session__check_output_opt(struct perf_session *session)
>  		set_print_ip_opts(&evsel->attr);
>  	}
>  
> +	/*
> +	 * set default for tracepoints to print symbols only
> +	 * if callchains are present
> +	 */
> +	if (symbol_conf.use_callchain &&
> +	    !output[PERF_TYPE_TRACEPOINT].user_set) {
> +		struct perf_event_attr *attr;
> +
> +		j = PERF_TYPE_TRACEPOINT;
> +		evsel = perf_session__find_first_evtype(session, j);
> +		if (evsel == NULL)
> +			goto out;
> +
> +		attr = &evsel->attr;
> +
> +		if (attr->sample_type & PERF_SAMPLE_CALLCHAIN) {
> +			output[j].fields |= PERF_OUTPUT_IP;
> +			output[j].fields |= PERF_OUTPUT_SYM;
> +			output[j].fields |= PERF_OUTPUT_DSO;
> +			set_print_ip_opts(attr);

We probably want to avoid DSO and IP for most tracepoints. For kernel
tracepoints dso is going to be the kernel anyway and IP is often not that relevant
either.

tracepoint already carries a lot of informations and it's better to avoid any noise.

Thanks.

> +		}
> +	}
> +
> +out:
>  	return 0;
>  }
>  
> -- 
> 1.8.3.4 (Apple Git-47)
>