Re: [PATCH RFC V8 1/4] perf,tools: per-event time support

[Jiri Olsa <jolsa@redhat.com>]

On Tue, Aug 04, 2015 at 04:30:19AM -0400, Kan Liang wrote:
> From: Kan Liang <kan.liang@intel.com>
> 
> This patchkit adds the ability to turn off time stamps per event.
> One usable case of partial time is to work with per-event callgraph to
> enable "PEBS threshold > 1" (https://lkml.org/lkml/2015/5/10/196), which
> can significantly reduce the sampling overhead.
> The event samples with time stamps off will not be ordered.
> 
> Signed-off-by: Kan Liang <kan.liang@intel.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/perf/Documentation/perf-record.txt |  4 +++-
>  tools/perf/util/evsel.c                  | 14 +++++++++++---
>  tools/perf/util/evsel.h                  |  2 ++
>  tools/perf/util/parse-events.c           | 12 ++++++++++++
>  tools/perf/util/parse-events.h           |  1 +
>  tools/perf/util/parse-events.l           |  1 +
>  tools/perf/util/pmu.c                    |  2 +-
>  7 files changed, 31 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index ac41350..0d852d1 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -49,7 +49,9 @@ OPTIONS
>  	  These params can be used to overload default config values per event.
>  	  Here is a list of the params.
>  	  - 'period': Set event sampling period
> -
> +	  - 'time': Disable/enable time stamping. Acceptable values are 1 for
> +		    enabling time stamping. 0 for disabling time stamping.
> +		    The default is 1.
>  	  Note: If user explicitly sets options which conflict with the params,
>  	  the value set by the params will be overridden.
>  
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 7d3acba..7febfe2 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -587,15 +587,23 @@ perf_evsel__config_callgraph(struct perf_evsel *evsel,
>  	}
>  }
>  
> -static void apply_config_terms(struct perf_event_attr *attr __maybe_unused,
> -			       struct list_head *config_terms)
> +static void apply_config_terms(struct perf_evsel *evsel)
>  {
>  	struct perf_evsel_config_term *term;
> +	struct list_head *config_terms = &evsel->config_terms;
> +	struct perf_event_attr *attr = &evsel->attr;
>  
>  	list_for_each_entry(term, config_terms, list) {
>  		switch (term->type) {
>  		case PERF_EVSEL__CONFIG_TERM_PERIOD:
>  			attr->sample_period = term->val.period;
> +			break;
> +		case PERF_EVSEL__CONFIG_TERM_TIME:
> +			if (term->val.time)
> +				perf_evsel__set_sample_bit(evsel, TIME);
> +			else
> +				perf_evsel__reset_sample_bit(evsel, TIME);
> +			break;
>  		default:
>  			break;
>  		}
> @@ -798,7 +806,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
>  	 * Apply event specific term settings,
>  	 * it overloads any global configuration.
>  	 */
> -	apply_config_terms(attr, &evsel->config_terms);
> +	apply_config_terms(evsel);
>  }
>  
>  static int perf_evsel__alloc_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index a7d2175..6a12908 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -39,6 +39,7 @@ struct cgroup_sel;
>  */
>  enum {
>  	PERF_EVSEL__CONFIG_TERM_PERIOD,
> +	PERF_EVSEL__CONFIG_TERM_TIME,
>  	PERF_EVSEL__CONFIG_TERM_MAX,
>  };
>  
> @@ -47,6 +48,7 @@ struct perf_evsel_config_term {
>  	int	type;
>  	union {
>  		u64	period;
> +		bool	time;
>  	} val;
>  };
>  
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 09bee93..a6cb9af 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -603,6 +603,14 @@ do {									   \
>  		 * attr->branch_sample_type = term->val.num;
>  		 */
>  		break;
> +	case PARSE_EVENTS__TERM_TYPE_TIME:
> +		CHECK_TYPE_VAL(NUM);
> +		if (term->val.num > 1) {
> +			err->str = strdup("expected 0 or 1");
> +			err->idx = term->err_val;
> +			return -EINVAL;
> +		}
> +		break;
>  	case PARSE_EVENTS__TERM_TYPE_NAME:
>  		CHECK_TYPE_VAL(STR);
>  		break;
> @@ -650,6 +658,10 @@ do {								\
>  		switch (term->type_term) {
>  		case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
>  			ADD_CONFIG_TERM(PERIOD, period, term->val.num);
> +			break;
> +		case PARSE_EVENTS__TERM_TYPE_TIME:
> +			ADD_CONFIG_TERM(TIME, time, term->val.num);
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index 2063048..e6f9aacc 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -63,6 +63,7 @@ enum {
>  	PARSE_EVENTS__TERM_TYPE_NAME,
>  	PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD,
>  	PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE,
> +	PARSE_EVENTS__TERM_TYPE_TIME,
>  };
>  
>  struct parse_events_term {
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 13cef3c..f542750 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -183,6 +183,7 @@ config2			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG2); }
>  name			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NAME); }
>  period			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD); }
>  branch_type		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE); }
> +time			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_TIME); }
>  ,			{ return ','; }
>  "/"			{ BEGIN(INITIAL); return '/'; }
>  {name_minus}		{ return str(yyscanner, PE_NAME); }
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 7bcb8c3..b615cdf 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -607,7 +607,7 @@ static char *formats_error_string(struct list_head *formats)
>  {
>  	struct perf_pmu_format *format;
>  	char *err, *str;
> -	static const char *static_terms = "config,config1,config2,name,period,branch_type\n";
> +	static const char *static_terms = "config,config1,config2,name,period,branch_type,time\n";
>  	unsigned i = 0;
>  
>  	if (!asprintf(&str, "valid terms:"))
> -- 
> 1.8.3.1
>