Re: [PATCH v3 1/4] perf: Add a 'percore' event qualifier

[Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>]

Em Tue, Mar 19, 2019 at 04:56:53PM +0800, Jin Yao escreveu:
> Add a 'percore' event qualifier, like cpu/event=0,umask=0x3,percore=1/,
> that sums up the event counts for both hardware threads in a core.
> 
> We can already do this with --per-core, but it's often useful to do
> this together with other metrics that are collected per hardware thread.
> So we need to support this per-core counting on a event level.
> 
> This can be implemented in only the user tool, no kernel support needed.
> 
>  v3:
>  ---
>  Simplify the code according to Jiri's comments.
>  Before:
>    "return term->val.percore ? true : false;"
>  Now:
>    "return term->val.percore;"
> 
>  v2:
>  ---
>  Change the qualifier name from 'coresum' to 'percore' according to
>  comments from Jiri and Andi.

I'm applying this, but please, don't forget to, when adding a new
qualifier, to update the documentation... I'm doing this for you this
time.

- Arnaldo
 
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>  tools/perf/util/evsel.c        |  2 ++
>  tools/perf/util/evsel.h        |  3 +++
>  tools/perf/util/parse-events.c | 27 +++++++++++++++++++++++++++
>  tools/perf/util/parse-events.h |  1 +
>  tools/perf/util/parse-events.l |  1 +
>  5 files changed, 34 insertions(+)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 3bbf73e..b900157 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -803,6 +803,8 @@ static void apply_config_terms(struct perf_evsel *evsel,
>  			break;
>  		case PERF_EVSEL__CONFIG_TERM_DRV_CFG:
>  			break;
> +		case PERF_EVSEL__CONFIG_TERM_PERCORE:
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index cc578e0..fd86689 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -50,6 +50,7 @@ enum term_type {
>  	PERF_EVSEL__CONFIG_TERM_OVERWRITE,
>  	PERF_EVSEL__CONFIG_TERM_DRV_CFG,
>  	PERF_EVSEL__CONFIG_TERM_BRANCH,
> +	PERF_EVSEL__CONFIG_TERM_PERCORE,
>  };
>  
>  struct perf_evsel_config_term {
> @@ -67,6 +68,7 @@ struct perf_evsel_config_term {
>  		bool	overwrite;
>  		char	*branch;
>  		unsigned long max_events;
> +		bool	percore;
>  	} val;
>  	bool weak;
>  };
> @@ -150,6 +152,7 @@ struct perf_evsel {
>  	struct perf_evsel	**metric_events;
>  	bool			collect_stat;
>  	bool			weak_group;
> +	bool			percore;
>  	const char		*pmu_name;
>  };
>  
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 4dcc01b..f77be35 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -930,6 +930,7 @@ static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = {
>  	[PARSE_EVENTS__TERM_TYPE_OVERWRITE]		= "overwrite",
>  	[PARSE_EVENTS__TERM_TYPE_NOOVERWRITE]		= "no-overwrite",
>  	[PARSE_EVENTS__TERM_TYPE_DRV_CFG]		= "driver-config",
> +	[PARSE_EVENTS__TERM_TYPE_PERCORE]		= "percore",
>  };
>  
>  static bool config_term_shrinked;
> @@ -950,6 +951,7 @@ config_term_avail(int term_type, struct parse_events_error *err)
>  	case PARSE_EVENTS__TERM_TYPE_CONFIG2:
>  	case PARSE_EVENTS__TERM_TYPE_NAME:
>  	case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
> +	case PARSE_EVENTS__TERM_TYPE_PERCORE:
>  		return true;
>  	default:
>  		if (!err)
> @@ -1041,6 +1043,14 @@ do {									   \
>  	case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS:
>  		CHECK_TYPE_VAL(NUM);
>  		break;
> +	case PARSE_EVENTS__TERM_TYPE_PERCORE:
> +		CHECK_TYPE_VAL(NUM);
> +		if ((unsigned int)term->val.num > 1) {
> +			err->str = strdup("expected 0 or 1");
> +			err->idx = term->err_val;
> +			return -EINVAL;
> +		}
> +		break;
>  	default:
>  		err->str = strdup("unknown term");
>  		err->idx = term->err_term;
> @@ -1179,6 +1189,10 @@ do {								\
>  		case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
>  			ADD_CONFIG_TERM(DRV_CFG, drv_cfg, term->val.str);
>  			break;
> +		case PARSE_EVENTS__TERM_TYPE_PERCORE:
> +			ADD_CONFIG_TERM(PERCORE, percore,
> +					term->val.num ? true : false);
> +			break;
>  		default:
>  			break;
>  		}
> @@ -1233,6 +1247,18 @@ int parse_events_add_numeric(struct parse_events_state *parse_state,
>  			 get_config_name(head_config), &config_terms);
>  }
>  
> +static bool config_term_percore(struct list_head *config_terms)
> +{
> +	struct perf_evsel_config_term *term;
> +
> +	list_for_each_entry(term, config_terms, list) {
> +		if (term->type == PERF_EVSEL__CONFIG_TERM_PERCORE)
> +			return term->val.percore;
> +	}
> +
> +	return false;
> +}
> +
>  int parse_events_add_pmu(struct parse_events_state *parse_state,
>  			 struct list_head *list, char *name,
>  			 struct list_head *head_config,
> @@ -1305,6 +1331,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
>  		evsel->metric_name = info.metric_name;
>  		evsel->pmu_name = name;
>  		evsel->use_uncore_alias = use_uncore_alias;
> +		evsel->percore = config_term_percore(&evsel->config_terms);
>  	}
>  
>  	return evsel ? 0 : -ENOMEM;
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index 5ed035c..ad4fbf3 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -75,6 +75,7 @@ enum {
>  	PARSE_EVENTS__TERM_TYPE_NOOVERWRITE,
>  	PARSE_EVENTS__TERM_TYPE_OVERWRITE,
>  	PARSE_EVENTS__TERM_TYPE_DRV_CFG,
> +	PARSE_EVENTS__TERM_TYPE_PERCORE,
>  	__PARSE_EVENTS__TERM_TYPE_NR,
>  };
>  
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 7805c71..7e9f8dc 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -274,6 +274,7 @@ inherit			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_INHERIT); }
>  no-inherit		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NOINHERIT); }
>  overwrite		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_OVERWRITE); }
>  no-overwrite		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_NOOVERWRITE); }
> +percore			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_PERCORE); }
>  ,			{ return ','; }
>  "/"			{ BEGIN(INITIAL); return '/'; }
>  {name_minus}		{ return str(yyscanner, PE_NAME); }
> -- 
> 2.7.4

-- 

- Arnaldo