From: Ian Rogers <irogers@google.com>
To: James Clark <james.clark@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Suzuki K Poulose <suzuki.poulose@arm.com>,
Mike Leach <mike.leach@linaro.org>,
John Garry <john.g.garry@oracle.com>,
Will Deacon <will@kernel.org>, Leo Yan <leo.yan@linux.dev>,
linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
coresight@lists.linaro.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 03/12] perf parse-events: Track all user changed config bits
Date: Tue, 16 Dec 2025 12:39:15 -0800 [thread overview]
Message-ID: <CAP-5=fUBSOHcK5rjJgyJed3PkgLb_dMys_qDNu=Cy_18faLX_Q@mail.gmail.com> (raw)
In-Reply-To: <20251212-james-perf-config-bits-v3-3-aa36a4846776@linaro.org>
On Fri, Dec 12, 2025 at 7:32 AM James Clark <james.clark@linaro.org> wrote:
>
> Currently we only track which bits were set by the user in attr->config.
> But all configN fields should be treated equally as they can all have
> default and user overridden values.
>
> Track them all by making get_config_chgs() generic and calling it once
> for each config value.
>
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
> tools/perf/util/evsel.c | 6 ++-
> tools/perf/util/evsel_config.h | 6 ++-
> tools/perf/util/parse-events.c | 98 +++++++++++++++++++++++-------------------
> tools/perf/util/pmu.c | 2 +-
> 4 files changed, 65 insertions(+), 47 deletions(-)
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 9cd706f6279313c2cc53eba1adbd45f352090f75..dcadf022405d398bf394b3da7f0db5c81a3ec268 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1242,7 +1242,11 @@ static void evsel__apply_config_terms(struct evsel *evsel,
> case EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE:
> /* Already applied by auxtrace */
> break;
> - case EVSEL__CONFIG_TERM_CFG_CHG:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG1:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG2:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG3:
> + case EVSEL__CONFIG_TERM_USR_CHG_CONFIG4:
> break;
> case EVSEL__CONFIG_TERM_RATIO_TO_PREV:
> rtp_buf = term->val.str;
> diff --git a/tools/perf/util/evsel_config.h b/tools/perf/util/evsel_config.h
> index 685fd8d5c4a88fb8ae601ded60c1fc36dde73403..7b565d76c0bc4277effbbf950110986b423f571c 100644
> --- a/tools/perf/util/evsel_config.h
> +++ b/tools/perf/util/evsel_config.h
> @@ -27,7 +27,11 @@ enum evsel_term_type {
> EVSEL__CONFIG_TERM_AUX_OUTPUT,
> EVSEL__CONFIG_TERM_AUX_ACTION,
> EVSEL__CONFIG_TERM_AUX_SAMPLE_SIZE,
> - EVSEL__CONFIG_TERM_CFG_CHG,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG1,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG2,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG3,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG4,
> EVSEL__CONFIG_TERM_RATIO_TO_PREV,
> };
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 46422286380f33a48087e7079a45bb21beb2d573..7ef63b808d0542999fd1303b9f94cae68e38030f 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1253,66 +1253,32 @@ static int get_config_terms(const struct parse_events_terms *head_config,
> return 0;
> }
>
> -/*
> - * Add EVSEL__CONFIG_TERM_CFG_CHG where cfg_chg will have a bit set for
> - * each bit of attr->config that the user has changed.
> - */
> -static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head_config,
> - struct list_head *head_terms)
> +static int add_cfg_chg(struct perf_pmu *pmu,
> + struct parse_events_terms *head_config,
nit: the head_config could be made const here.
> + struct list_head *head_terms,
> + int format_type,
> + enum parse_events__term_type term_type,
> + enum evsel_term_type new_term_type)
> {
> struct parse_events_term *term;
> u64 bits = 0;
> int type;
>
> list_for_each_entry(term, &head_config->terms, list) {
> - switch (term->type_term) {
> - case PARSE_EVENTS__TERM_TYPE_USER:
> + if (term->type_term == PARSE_EVENTS__TERM_TYPE_USER) {
> type = perf_pmu__format_type(pmu, term->config);
> - if (type != PERF_PMU_FORMAT_VALUE_CONFIG)
> + if (type != format_type)
> continue;
> bits |= perf_pmu__format_bits(pmu, term->config);
> - break;
> - case PARSE_EVENTS__TERM_TYPE_CONFIG:
> + } else if (term->type_term == term_type) {
> bits = ~(u64)0;
> - break;
> - case PARSE_EVENTS__TERM_TYPE_CONFIG1:
> - case PARSE_EVENTS__TERM_TYPE_CONFIG2:
> - case PARSE_EVENTS__TERM_TYPE_CONFIG3:
> - case PARSE_EVENTS__TERM_TYPE_CONFIG4:
> - case PARSE_EVENTS__TERM_TYPE_LEGACY_HARDWARE_CONFIG:
> - case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE_CONFIG:
> - case PARSE_EVENTS__TERM_TYPE_NAME:
> - case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
> - case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
> - case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
> - case PARSE_EVENTS__TERM_TYPE_TIME:
> - case PARSE_EVENTS__TERM_TYPE_CALLGRAPH:
> - case PARSE_EVENTS__TERM_TYPE_STACKSIZE:
> - case PARSE_EVENTS__TERM_TYPE_NOINHERIT:
> - case PARSE_EVENTS__TERM_TYPE_INHERIT:
> - case PARSE_EVENTS__TERM_TYPE_MAX_STACK:
> - case PARSE_EVENTS__TERM_TYPE_MAX_EVENTS:
> - case PARSE_EVENTS__TERM_TYPE_NOOVERWRITE:
> - case PARSE_EVENTS__TERM_TYPE_OVERWRITE:
> - case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
> - case PARSE_EVENTS__TERM_TYPE_PERCORE:
> - case PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT:
> - case PARSE_EVENTS__TERM_TYPE_AUX_ACTION:
> - case PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE:
> - case PARSE_EVENTS__TERM_TYPE_METRIC_ID:
> - case PARSE_EVENTS__TERM_TYPE_RAW:
> - case PARSE_EVENTS__TERM_TYPE_CPU:
> - case PARSE_EVENTS__TERM_TYPE_RATIO_TO_PREV:
> - default:
> - break;
> }
> }
>
> if (bits) {
> struct evsel_config_term *new_term;
>
> - new_term = add_config_term(EVSEL__CONFIG_TERM_CFG_CHG,
> - head_terms, false);
> + new_term = add_config_term(new_term_type, head_terms, false);
> if (!new_term)
> return -ENOMEM;
> new_term->val.cfg_chg = bits;
> @@ -1321,6 +1287,50 @@ static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head
> return 0;
> }
>
> +/*
> + * Add EVSEL__CONFIG_TERM_USR_CFG_CONFIGn where cfg_chg will have a bit set for
> + * each bit of attr->configN that the user has changed.
> + */
> +static int get_config_chgs(struct perf_pmu *pmu,
> + struct parse_events_terms *head_config,
> + struct list_head *head_terms)
> +{
> + int ret;
> +
> + ret = add_cfg_chg(pmu, head_config, head_terms,
> + PERF_PMU_FORMAT_VALUE_CONFIG,
> + PARSE_EVENTS__TERM_TYPE_CONFIG,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG);
> + if (ret)
> + return ret;
> +
> + ret = add_cfg_chg(pmu, head_config, head_terms,
> + PERF_PMU_FORMAT_VALUE_CONFIG1,
> + PARSE_EVENTS__TERM_TYPE_CONFIG1,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG1);
> + if (ret)
> + return ret;
> +
> + ret = add_cfg_chg(pmu, head_config, head_terms,
> + PERF_PMU_FORMAT_VALUE_CONFIG2,
> + PARSE_EVENTS__TERM_TYPE_CONFIG2,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG2);
> + if (ret)
> + return ret;
> +
> + ret = add_cfg_chg(pmu, head_config, head_terms,
> + PERF_PMU_FORMAT_VALUE_CONFIG3,
> + PARSE_EVENTS__TERM_TYPE_CONFIG3,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG3);
> + if (ret)
> + return ret;
> +
> + return add_cfg_chg(pmu, head_config, head_terms,
> + PERF_PMU_FORMAT_VALUE_CONFIG4,
> + PARSE_EVENTS__TERM_TYPE_CONFIG4,
> + EVSEL__CONFIG_TERM_USR_CHG_CONFIG4);
> +}
> +
> int parse_events_add_tracepoint(struct parse_events_state *parse_state,
> struct list_head *list,
> const char *sys, const char *event,
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 2c26c8d76bed920c7b5b43beb867e4d1bcaa2970..e02f74d4020c151082ecc434c29c0ec4c2c9b2c8 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1387,7 +1387,7 @@ void evsel__set_config_if_unset(struct perf_pmu *pmu, struct evsel *evsel,
> const char *config_name, u64 val)
> {
> u64 user_bits = 0, bits;
> - struct evsel_config_term *term = evsel__get_config_term(evsel, CFG_CHG);
> + struct evsel_config_term *term = evsel__get_config_term(evsel, USR_CHG_CONFIG);
>
> if (term)
> user_bits = term->val.cfg_chg;
>
> --
> 2.34.1
>
next prev parent reply other threads:[~2025-12-16 20:39 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-12 15:32 [PATCH v3 00/12] perf cs-etm/arm-spe: Remove hard coded config fields James Clark
2025-12-12 15:32 ` [PATCH v3 01/12] perf parse-events: Refactor get_config_terms() to remove macros James Clark
2025-12-12 15:32 ` [PATCH v3 02/12] perf evsel: Support sparse fields in evsel__set_config_if_unset() James Clark
2025-12-12 15:32 ` [PATCH v3 03/12] perf parse-events: Track all user changed config bits James Clark
2025-12-16 20:39 ` Ian Rogers [this message]
2025-12-12 15:32 ` [PATCH v3 04/12] perf evsel: apply evsel__set_config_if_unset() to all config fields James Clark
2025-12-12 15:32 ` [PATCH v3 05/12] perf evsel: Add a helper to get the value of a config field James Clark
2025-12-16 20:44 ` Ian Rogers
2025-12-17 9:30 ` James Clark
2025-12-12 15:32 ` [PATCH v3 06/12] perf parse-events: Always track user config changes James Clark
2025-12-12 15:32 ` [PATCH v3 07/12] perf tests: Test evsel__set_config_if_unset() and config change tracking James Clark
2025-12-12 15:32 ` [PATCH v3 08/12] perf cs-etm: Make a helper to find the Coresight evsel James Clark
2025-12-12 15:32 ` [PATCH v3 09/12] perf cs-etm: Don't use hard coded config bits when setting up ETMCR James Clark
2025-12-12 15:32 ` [PATCH v3 10/12] perf cs-etm: Don't use hard coded config bits when setting up TRCCONFIGR James Clark
2025-12-12 15:32 ` [PATCH v3 11/12] perf cs-etm: Don't hard code config attribute when configuring the event James Clark
2025-12-12 15:32 ` [PATCH v3 12/12] perf arm-spe: Don't hard code config attribute James Clark
2025-12-16 21:00 ` [PATCH v3 00/12] perf cs-etm/arm-spe: Remove hard coded config fields Ian Rogers
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='CAP-5=fUBSOHcK5rjJgyJed3PkgLb_dMys_qDNu=Cy_18faLX_Q@mail.gmail.com' \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=coresight@lists.linaro.org \
--cc=james.clark@linaro.org \
--cc=john.g.garry@oracle.com \
--cc=jolsa@kernel.org \
--cc=leo.yan@linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mike.leach@linaro.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=suzuki.poulose@arm.com \
--cc=will@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).