Re: [PATCH v5 6/9] perf, tools, script: Make itrace script default to all calls

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Tue, Sep 18, 2018 at 04:24:55PM +0300, Adrian Hunter escreveu:
> On 18/09/18 15:32, Andi Kleen wrote:
> > +++ b/tools/perf/util/auxtrace.c
> > @@ -964,16 +964,23 @@ s64 perf_event__process_auxtrace(struct perf_tool *tool,
> >  #define PERF_ITRACE_DEFAULT_LAST_BRANCH_SZ	64
> >  #define PERF_ITRACE_MAX_LAST_BRANCH_SZ		1024
> >  
> > -void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts)
> > +void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts,
> > +				    bool no_sample)
> 
> Rather than pass no_sample, what about passing the whole of the tool's
> options i.e.

This can be done on top, right? Or will you need the other options on
work you're doing right now and that would be an advantage to do this
right now?

- Arnaldo
 
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index db1511359c5e..d4db61626a5f 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -964,8 +964,14 @@ s64 perf_event__process_auxtrace(struct perf_tool *tool,
>  #define PERF_ITRACE_DEFAULT_LAST_BRANCH_SZ	64
>  #define PERF_ITRACE_MAX_LAST_BRANCH_SZ		1024
>  
> -void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts)
> +bool itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts,
> +				    const struct itrace_synth_opts *tool_synth_opts)
>  {
> +	if (tool_synth_opts && tool_synth_opts->set) {
> +		*synth_opts = *tool_synth_opts;
> +		return false;
> +	}
> +
>  	synth_opts->instructions = true;
>  	synth_opts->branches = true;
>  	synth_opts->transactions = true;
> @@ -977,6 +983,8 @@ void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts)
>  	synth_opts->callchain_sz = PERF_ITRACE_DEFAULT_CALLCHAIN_SZ;
>  	synth_opts->last_branch_sz = PERF_ITRACE_DEFAULT_LAST_BRANCH_SZ;
>  	synth_opts->initial_skip = 0;
> +
> +	return true;
>  }
>  
>  /*
> @@ -1001,7 +1009,7 @@ int itrace_parse_synth_opts(const struct option *opt, const char *str,
>  	}
>  
>  	if (!str) {
> -		itrace_synth_opts__set_default(synth_opts);
> +		itrace_synth_opts__set_default(synth_opts, NULL);
>  		return 0;
>  	}
>  
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index a516a891d7ea..172c92a8c596 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -528,7 +528,8 @@ int perf_event__process_auxtrace_error(struct perf_tool *tool,
>  				       struct perf_session *session);
>  int itrace_parse_synth_opts(const struct option *opt, const char *str,
>  			    int unset);
> -void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts);
> +bool itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts,
> +				    const struct itrace_synth_opts *tool_synth_opts);
>  
>  size_t perf_event__fprintf_auxtrace_error(union perf_event *event, FILE *fp);
>  void perf_session__auxtrace_error_inc(struct perf_session *session,
> diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> index 2ae640257fdb..45b9a2b9f473 100644
> --- a/tools/perf/util/cs-etm.c
> +++ b/tools/perf/util/cs-etm.c
> @@ -1429,12 +1429,9 @@ int cs_etm__process_auxtrace_info(union perf_event *event,
>  		return 0;
>  	}
>  
> -	if (session->itrace_synth_opts && session->itrace_synth_opts->set) {
> -		etm->synth_opts = *session->itrace_synth_opts;
> -	} else {
> -		itrace_synth_opts__set_default(&etm->synth_opts);
> +	if (itrace_synth_opts__set_default(&etm->synth_opts,
> +					   session->itrace_synth_opts))
>  		etm->synth_opts.callchain = false;
> -	}
>  
>  	err = cs_etm__synth_events(etm, session);
>  	if (err)
> diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c
> index 7f0c83b6332b..2d96f4743789 100644
> --- a/tools/perf/util/intel-bts.c
> +++ b/tools/perf/util/intel-bts.c
> @@ -907,12 +907,9 @@ int intel_bts_process_auxtrace_info(union perf_event *event,
>  	if (dump_trace)
>  		return 0;
>  
> -	if (session->itrace_synth_opts && session->itrace_synth_opts->set) {
> -		bts->synth_opts = *session->itrace_synth_opts;
> -	} else {
> -		itrace_synth_opts__set_default(&bts->synth_opts);
> -		if (session->itrace_synth_opts)
> -			bts->synth_opts.thread_stack =
> +	if (itrace_synth_opts__set_default(&bts->synth_opts,
> +					   session->itrace_synth_opts)) {
> +		bts->synth_opts.thread_stack =
>  				session->itrace_synth_opts->thread_stack;
>  	}
>  
> diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
> index aec68908d604..b6ac8ac2f4c3 100644
> --- a/tools/perf/util/intel-pt.c
> +++ b/tools/perf/util/intel-pt.c
> @@ -2551,10 +2551,8 @@ int intel_pt_process_auxtrace_info(union perf_event *event,
>  		goto err_delete_thread;
>  	}
>  
> -	if (session->itrace_synth_opts && session->itrace_synth_opts->set) {
> -		pt->synth_opts = *session->itrace_synth_opts;
> -	} else {
> -		itrace_synth_opts__set_default(&pt->synth_opts);
> +	if (itrace_synth_opts__set_default(&pt->synth_opts,
> +					   session->itrace_synth_opts)) {
>  		if (use_browser != -1) {
>  			pt->synth_opts.branches = false;
>  			pt->synth_opts.callchain = true;