From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Jin Yao <yao.jin@linux.intel.com>
Cc: jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com,
alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org,
ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v3 1/4] perf: Add a 'percore' event qualifier
Date: Wed, 10 Apr 2019 09:36:41 -0300 [thread overview]
Message-ID: <20190410123641.GA13888@kernel.org> (raw)
In-Reply-To: <1552985816-20915-2-git-send-email-yao.jin@linux.intel.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
next prev parent reply other threads:[~2019-04-10 12:36 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-19 8:56 [PATCH v3 0/4] perf: Support a new 'percore' event qualifier Jin Yao
2019-03-19 8:56 ` [PATCH v3 1/4] perf: Add a " Jin Yao
2019-04-10 12:36 ` Arnaldo Carvalho de Melo [this message]
2019-04-10 12:54 ` Arnaldo Carvalho de Melo
2019-04-10 14:15 ` Jin, Yao
2019-03-19 8:56 ` [PATCH v3 2/4] perf stat: Factor out aggregate counts printing Jin Yao
2019-03-19 8:56 ` [PATCH v3 3/4] perf stat: Support 'percore' event qualifier Jin Yao
2019-03-19 8:56 ` [PATCH v3 4/4] perf test: Add a simple test for term 'percore' Jin Yao
2019-03-19 10:20 ` [PATCH v3 0/4] perf: Support a new 'percore' event qualifier Jiri Olsa
2019-04-10 2:32 ` Jin, Yao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190410123641.GA13888@kernel.org \
--to=arnaldo.melo@gmail.com \
--cc=Linux-kernel@vger.kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@intel.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yao.jin@intel.com \
--cc=yao.jin@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.