| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z4F3qxFaYnMTtPw7@google.com>
Date: Fri, 10 Jan 2025 11:40:27 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...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>,
Kan Liang <kan.liang@...ux.intel.com>,
James Clark <james.clark@...aro.org>, Ze Gao <zegao2021@...il.com>,
Weilin Wang <weilin.wang@...el.com>,
Dominique Martinet <asmadeus@...ewreck.org>,
Jean-Philippe Romain <jean-philippe.romain@...s.st.com>,
Junhao He <hejunhao3@...wei.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
Aditya Bodkhe <Aditya.Bodkhe1@....com>,
Atish Patra <atishp@...osinc.com>, Leo Yan <leo.yan@....com>,
Beeman Strong <beeman@...osinc.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH v5 4/4] perf parse-events: Reapply "Prefer sysfs/JSON
hardware events over legacy"
On Thu, Jan 09, 2025 at 02:21:09PM -0800, Ian Rogers wrote:
> Originally posted and merged from:
> https://lore.kernel.org/r/20240416061533.921723-10-irogers@google.com
> This reverts commit 4f1b067359ac8364cdb7f9fda41085fa85789d0f although
> the patch is now smaller due to related fixes being applied in commit
> 22a4db3c3603 ("perf evsel: Add alternate_hw_config and use in
> evsel__match").
> The original commit message was:
>
> It was requested that RISC-V be able to add events to the perf tool so
> the PMU driver didn't need to map legacy events to config encodings:
> https://lore.kernel.org/lkml/20240217005738.3744121-1-atishp@rivosinc.com/
>
> This change makes the priority of events specified without a PMU the
> same as those specified with a PMU, namely sysfs and JSON events are
> checked first before using the legacy encoding.
I'm still not convinced why we need this change despite of these
troubles. If it's because RISC-V cannot define the lagacy hardware
events in the kernel driver, why not using a different name in JSON and
ask users to use the name specifically? Something like:
$ perf record -e riscv-cycles ...
Thanks,
Namhyung
>
> The hw_term is made more generic as a hardware_event that encodes a
> pair of string and int value, allowing parse_events_multi_pmu_add to
> fall back on a known encoding when the sysfs/JSON adding fails for
> core events. As this covers PE_VALUE_SYM_HW, that token is removed and
> related code simplified.
>
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> Reviewed-by: Kan Liang <kan.liang@...ux.intel.com>
> Tested-by: Atish Patra <atishp@...osinc.com>
> Tested-by: James Clark <james.clark@...aro.org>
> Tested-by: Leo Yan <leo.yan@....com>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> Cc: Beeman Strong <beeman@...osinc.com>
> Cc: Ingo Molnar <mingo@...hat.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> ---
> tools/perf/util/parse-events.c | 26 +++++++++---
> tools/perf/util/parse-events.l | 76 +++++++++++++++++-----------------
> tools/perf/util/parse-events.y | 60 ++++++++++++++++++---------
> 3 files changed, 98 insertions(+), 64 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 1e23faa364b1..3a60fca53cfa 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1545,8 +1545,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> struct list_head *list = NULL;
> struct perf_pmu *pmu = NULL;
> YYLTYPE *loc = loc_;
> - int ok = 0;
> - const char *config;
> + int ok = 0, core_ok = 0;
> + const char *tmp;
> struct parse_events_terms parsed_terms;
>
> *listp = NULL;
> @@ -1559,15 +1559,15 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> return ret;
> }
>
> - config = strdup(event_name);
> - if (!config)
> + tmp = strdup(event_name);
> + if (!tmp)
> goto out_err;
>
> if (parse_events_term__num(&term,
> PARSE_EVENTS__TERM_TYPE_USER,
> - config, /*num=*/1, /*novalue=*/true,
> + tmp, /*num=*/1, /*novalue=*/true,
> loc, /*loc_val=*/NULL) < 0) {
> - zfree(&config);
> + zfree(&tmp);
> goto out_err;
> }
> list_add_tail(&term->list, &parsed_terms.terms);
> @@ -1598,6 +1598,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> pr_debug("%s -> %s/%s/\n", event_name, pmu->name, sb.buf);
> strbuf_release(&sb);
> ok++;
> + if (pmu->is_core)
> + core_ok++;
> }
> }
>
> @@ -1614,6 +1616,18 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> }
> }
>
> + if (hw_config != PERF_COUNT_HW_MAX && !core_ok) {
> + /*
> + * The event wasn't found on core PMUs but it has a hardware
> + * config version to try.
> + */
> + if (!parse_events_add_numeric(parse_state, list,
> + PERF_TYPE_HARDWARE, hw_config,
> + const_parsed_terms,
> + /*wildcard=*/true))
> + ok++;
> + }
> +
> out_err:
> parse_events_terms__exit(&parsed_terms);
> if (ok)
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index bf7f73548605..29082a22ccc9 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -113,12 +113,12 @@ do { \
> yyless(0); \
> } while (0)
>
> -static int sym(yyscan_t scanner, int type, int config)
> +static int sym(yyscan_t scanner, int config)
> {
> YYSTYPE *yylval = parse_events_get_lval(scanner);
>
> - yylval->num = (type << 16) + config;
> - return type == PERF_TYPE_HARDWARE ? PE_VALUE_SYM_HW : PE_VALUE_SYM_SW;
> + yylval->num = config;
> + return PE_VALUE_SYM_SW;
> }
>
> static int term(yyscan_t scanner, enum parse_events__term_type type)
> @@ -129,13 +129,13 @@ static int term(yyscan_t scanner, enum parse_events__term_type type)
> return PE_TERM;
> }
>
> -static int hw_term(yyscan_t scanner, int config)
> +static int hw(yyscan_t scanner, int config)
> {
> YYSTYPE *yylval = parse_events_get_lval(scanner);
> char *text = parse_events_get_text(scanner);
>
> - yylval->hardware_term.str = strdup(text);
> - yylval->hardware_term.num = PERF_TYPE_HARDWARE + config;
> + yylval->hardware_event.str = strdup(text);
> + yylval->hardware_event.num = config;
> return PE_TERM_HW;
> }
>
> @@ -324,16 +324,16 @@ aux-output { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT); }
> aux-action { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_ACTION); }
> aux-sample-size { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE); }
> metric-id { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_METRIC_ID); }
> -cpu-cycles|cycles { return hw_term(yyscanner, PERF_COUNT_HW_CPU_CYCLES); }
> -stalled-cycles-frontend|idle-cycles-frontend { return hw_term(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
> -stalled-cycles-backend|idle-cycles-backend { return hw_term(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
> -instructions { return hw_term(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); }
> -cache-references { return hw_term(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); }
> -cache-misses { return hw_term(yyscanner, PERF_COUNT_HW_CACHE_MISSES); }
> -branch-instructions|branches { return hw_term(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); }
> -branch-misses { return hw_term(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); }
> -bus-cycles { return hw_term(yyscanner, PERF_COUNT_HW_BUS_CYCLES); }
> -ref-cycles { return hw_term(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); }
> +cpu-cycles|cycles { return hw(yyscanner, PERF_COUNT_HW_CPU_CYCLES); }
> +stalled-cycles-frontend|idle-cycles-frontend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
> +stalled-cycles-backend|idle-cycles-backend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
> +instructions { return hw(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); }
> +cache-references { return hw(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); }
> +cache-misses { return hw(yyscanner, PERF_COUNT_HW_CACHE_MISSES); }
> +branch-instructions|branches { return hw(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); }
> +branch-misses { return hw(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); }
> +bus-cycles { return hw(yyscanner, PERF_COUNT_HW_BUS_CYCLES); }
> +ref-cycles { return hw(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); }
> r{num_raw_hex} { return str(yyscanner, PE_RAW); }
> r0x{num_raw_hex} { return str(yyscanner, PE_RAW); }
> , { return ','; }
> @@ -377,28 +377,28 @@ r0x{num_raw_hex} { return str(yyscanner, PE_RAW); }
> <<EOF>> { BEGIN(INITIAL); }
> }
>
> -cpu-cycles|cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CPU_CYCLES); }
> -stalled-cycles-frontend|idle-cycles-frontend { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
> -stalled-cycles-backend|idle-cycles-backend { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
> -instructions { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_INSTRUCTIONS); }
> -cache-references { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_REFERENCES); }
> -cache-misses { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_CACHE_MISSES); }
> -branch-instructions|branches { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); }
> -branch-misses { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BRANCH_MISSES); }
> -bus-cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_BUS_CYCLES); }
> -ref-cycles { return sym(yyscanner, PERF_TYPE_HARDWARE, PERF_COUNT_HW_REF_CPU_CYCLES); }
> -cpu-clock { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CPU_CLOCK); }
> -task-clock { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_TASK_CLOCK); }
> -page-faults|faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS); }
> -minor-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MIN); }
> -major-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_PAGE_FAULTS_MAJ); }
> -context-switches|cs { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CONTEXT_SWITCHES); }
> -cpu-migrations|migrations { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CPU_MIGRATIONS); }
> -alignment-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_ALIGNMENT_FAULTS); }
> -emulation-faults { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_EMULATION_FAULTS); }
> -dummy { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_DUMMY); }
> -bpf-output { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUTPUT); }
> -cgroup-switches { return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_CGROUP_SWITCHES); }
> +cpu-cycles|cycles { return hw(yyscanner, PERF_COUNT_HW_CPU_CYCLES); }
> +stalled-cycles-frontend|idle-cycles-frontend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
> +stalled-cycles-backend|idle-cycles-backend { return hw(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
> +instructions { return hw(yyscanner, PERF_COUNT_HW_INSTRUCTIONS); }
> +cache-references { return hw(yyscanner, PERF_COUNT_HW_CACHE_REFERENCES); }
> +cache-misses { return hw(yyscanner, PERF_COUNT_HW_CACHE_MISSES); }
> +branch-instructions|branches { return hw(yyscanner, PERF_COUNT_HW_BRANCH_INSTRUCTIONS); }
> +branch-misses { return hw(yyscanner, PERF_COUNT_HW_BRANCH_MISSES); }
> +bus-cycles { return hw(yyscanner, PERF_COUNT_HW_BUS_CYCLES); }
> +ref-cycles { return hw(yyscanner, PERF_COUNT_HW_REF_CPU_CYCLES); }
> +cpu-clock { return sym(yyscanner, PERF_COUNT_SW_CPU_CLOCK); }
> +task-clock { return sym(yyscanner, PERF_COUNT_SW_TASK_CLOCK); }
> +page-faults|faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS); }
> +minor-faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS_MIN); }
> +major-faults { return sym(yyscanner, PERF_COUNT_SW_PAGE_FAULTS_MAJ); }
> +context-switches|cs { return sym(yyscanner, PERF_COUNT_SW_CONTEXT_SWITCHES); }
> +cpu-migrations|migrations { return sym(yyscanner, PERF_COUNT_SW_CPU_MIGRATIONS); }
> +alignment-faults { return sym(yyscanner, PERF_COUNT_SW_ALIGNMENT_FAULTS); }
> +emulation-faults { return sym(yyscanner, PERF_COUNT_SW_EMULATION_FAULTS); }
> +dummy { return sym(yyscanner, PERF_COUNT_SW_DUMMY); }
> +bpf-output { return sym(yyscanner, PERF_COUNT_SW_BPF_OUTPUT); }
> +cgroup-switches { return sym(yyscanner, PERF_COUNT_SW_CGROUP_SWITCHES); }
>
> {lc_type} { return str(yyscanner, PE_LEGACY_CACHE); }
> {lc_type}-{lc_op_result} { return str(yyscanner, PE_LEGACY_CACHE); }
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index f888cbb076d6..d2ef1890007e 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -55,7 +55,7 @@ static void free_list_evsel(struct list_head* list_evsel)
> %}
>
> %token PE_START_EVENTS PE_START_TERMS
> -%token PE_VALUE PE_VALUE_SYM_HW PE_VALUE_SYM_SW PE_TERM
> +%token PE_VALUE PE_VALUE_SYM_SW PE_TERM
> %token PE_EVENT_NAME
> %token PE_RAW PE_NAME
> %token PE_MODIFIER_EVENT PE_MODIFIER_BP PE_BP_COLON PE_BP_SLASH
> @@ -65,11 +65,9 @@ static void free_list_evsel(struct list_head* list_evsel)
> %token PE_DRV_CFG_TERM
> %token PE_TERM_HW
> %type <num> PE_VALUE
> -%type <num> PE_VALUE_SYM_HW
> %type <num> PE_VALUE_SYM_SW
> %type <mod> PE_MODIFIER_EVENT
> %type <term_type> PE_TERM
> -%type <num> value_sym
> %type <str> PE_RAW
> %type <str> PE_NAME
> %type <str> PE_LEGACY_CACHE
> @@ -85,6 +83,7 @@ static void free_list_evsel(struct list_head* list_evsel)
> %type <list_terms> opt_pmu_config
> %destructor { parse_events_terms__delete ($$); } <list_terms>
> %type <list_evsel> event_pmu
> +%type <list_evsel> event_legacy_hardware
> %type <list_evsel> event_legacy_symbol
> %type <list_evsel> event_legacy_cache
> %type <list_evsel> event_legacy_mem
> @@ -102,8 +101,8 @@ static void free_list_evsel(struct list_head* list_evsel)
> %destructor { free_list_evsel ($$); } <list_evsel>
> %type <tracepoint_name> tracepoint_name
> %destructor { free ($$.sys); free ($$.event); } <tracepoint_name>
> -%type <hardware_term> PE_TERM_HW
> -%destructor { free ($$.str); } <hardware_term>
> +%type <hardware_event> PE_TERM_HW
> +%destructor { free ($$.str); } <hardware_event>
>
> %union
> {
> @@ -118,10 +117,10 @@ static void free_list_evsel(struct list_head* list_evsel)
> char *sys;
> char *event;
> } tracepoint_name;
> - struct hardware_term {
> + struct hardware_event {
> char *str;
> u64 num;
> - } hardware_term;
> + } hardware_event;
> }
> %%
>
> @@ -264,6 +263,7 @@ PE_EVENT_NAME event_def
> event_def
>
> event_def: event_pmu |
> + event_legacy_hardware |
> event_legacy_symbol |
> event_legacy_cache sep_dc |
> event_legacy_mem sep_dc |
> @@ -306,24 +306,45 @@ PE_NAME sep_dc
> $$ = list;
> }
>
> -value_sym:
> -PE_VALUE_SYM_HW
> +event_legacy_hardware:
> +PE_TERM_HW opt_pmu_config
> +{
> + /* List of created evsels. */
> + struct list_head *list = NULL;
> + int err = parse_events_multi_pmu_add(_parse_state, $1.str, $1.num, $2, &list, &@1);
> +
> + free($1.str);
> + parse_events_terms__delete($2);
> + if (err)
> + PE_ABORT(err);
> +
> + $$ = list;
> +}
> |
> -PE_VALUE_SYM_SW
> +PE_TERM_HW sep_dc
> +{
> + struct list_head *list;
> + int err;
> +
> + err = parse_events_multi_pmu_add(_parse_state, $1.str, $1.num, NULL, &list, &@1);
> + free($1.str);
> + if (err)
> + PE_ABORT(err);
> + $$ = list;
> +}
>
> event_legacy_symbol:
> -value_sym '/' event_config '/'
> +PE_VALUE_SYM_SW '/' event_config '/'
> {
> struct list_head *list;
> - int type = $1 >> 16;
> - int config = $1 & 255;
> int err;
> - bool wildcard = (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE);
>
> list = alloc_list();
> if (!list)
> YYNOMEM;
> - err = parse_events_add_numeric(_parse_state, list, type, config, $3, wildcard);
> + err = parse_events_add_numeric(_parse_state, list,
> + /*type=*/PERF_TYPE_SOFTWARE, /*config=*/$1,
> + $3, /*wildcard=*/false);
> parse_events_terms__delete($3);
> if (err) {
> free_list_evsel(list);
> @@ -332,18 +353,17 @@ value_sym '/' event_config '/'
> $$ = list;
> }
> |
> -value_sym sep_slash_slash_dc
> +PE_VALUE_SYM_SW sep_slash_slash_dc
> {
> struct list_head *list;
> - int type = $1 >> 16;
> - int config = $1 & 255;
> - bool wildcard = (type == PERF_TYPE_HARDWARE || type == PERF_TYPE_HW_CACHE);
> int err;
>
> list = alloc_list();
> if (!list)
> YYNOMEM;
> - err = parse_events_add_numeric(_parse_state, list, type, config, /*head_config=*/NULL, wildcard);
> + err = parse_events_add_numeric(_parse_state, list,
> + /*type=*/PERF_TYPE_SOFTWARE, /*config=*/$1,
> + /*head_config=*/NULL, /*wildcard=*/false);
> if (err)
> PE_ABORT(err);
> $$ = list;
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
Powered by blists - more mailing lists