[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fUBSOHcK5rjJgyJed3PkgLb_dMys_qDNu=Cy_18faLX_Q@mail.gmail.com>
Date: Tue, 16 Dec 2025 12:39:15 -0800
From: Ian Rogers <irogers@...gle.com>
To: James Clark <james.clark@...aro.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Suzuki K Poulose <suzuki.poulose@....com>,
Mike Leach <mike.leach@...aro.org>, John Garry <john.g.garry@...cle.com>,
Will Deacon <will@...nel.org>, Leo Yan <leo.yan@...ux.dev>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 03/12] perf parse-events: Track all user changed config bits
On Fri, Dec 12, 2025 at 7:32 AM James Clark <james.clark@...aro.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@...aro.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
>
Powered by blists - more mailing lists