diff options
| author | Ian Rogers <irogers@google.com> | 2023-11-22 20:29:22 -0800 |
|---|---|---|
| committer | Arnaldo Carvalho de Melo <acme@redhat.com> | 2023-11-27 10:21:27 -0300 |
| commit | a24d9d9dc096fc0d0bd85302c9a4fe4fe3b1107b (patch) | |
| tree | b6bdb04955c904a1318965b1cff6e44f4b7743ba /tools/perf/util/pmu.c | |
| parent | a4271827e609d30b7255aa7e4c453a8f3fe36a7b (diff) | |
perf parse-events: Make legacy events lower priority than sysfs/JSON
The perf tool has previously made legacy events the priority so with
or without a PMU the legacy event would be opened:
$ perf stat -e cpu-cycles,cpu/cpu-cycles/ true
Using CPUID GenuineIntel-6-8D-1
intel_pt default config: tsc,mtc,mtc_period=3,psb_period=3,pt,branch
Attempting to add event pmu 'cpu' with 'cpu-cycles,' that may result in non-fatal errors
After aliases, add event pmu 'cpu' with 'cpu-cycles,' that may result in non-fatal errors
Control descriptor is not initialized
------------------------------------------------------------
perf_event_attr:
type 0 (PERF_TYPE_HARDWARE)
size 136
config 0 (PERF_COUNT_HW_CPU_CYCLES)
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
enable_on_exec 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid 833967 cpu -1 group_fd -1 flags 0x8 = 3
------------------------------------------------------------
perf_event_attr:
type 0 (PERF_TYPE_HARDWARE)
size 136
config 0 (PERF_COUNT_HW_CPU_CYCLES)
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
enable_on_exec 1
exclude_guest 1
------------------------------------------------------------
...
Fixes to make hybrid/BIG.little PMUs behave correctly, ie as core PMUs
capable of opening legacy events on each, removing hard coded "cpu_core"
and "cpu_atom" Intel PMU names, etc. caused a behavioral difference on
Apple/ARM due to latent issues in the PMU driver reported in:
https://lore.kernel.org/lkml/08f1f185-e259-4014-9ca4-6411d5c1bc65@marcan.st/
As part of that report Mark Rutland <mark.rutland@arm.com> requested
that legacy events not be higher in priority when a PMU is specified
reversing what has until this change been perf's default behavior. With
this change the above becomes:
$ perf stat -e cpu-cycles,cpu/cpu-cycles/ true
Using CPUID GenuineIntel-6-8D-1
Attempt to add: cpu/cpu-cycles=0/
..after resolving event: cpu/event=0x3c/
Control descriptor is not initialized
------------------------------------------------------------
perf_event_attr:
type 0 (PERF_TYPE_HARDWARE)
size 136
config 0 (PERF_COUNT_HW_CPU_CYCLES)
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
enable_on_exec 1
exclude_guest 1
------------------------------------------------------------
sys_perf_event_open: pid 827628 cpu -1 group_fd -1 flags 0x8 = 3
------------------------------------------------------------
perf_event_attr:
type 4 (PERF_TYPE_RAW)
size 136
config 0x3c
sample_type IDENTIFIER
read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING
disabled 1
inherit 1
enable_on_exec 1
exclude_guest 1
------------------------------------------------------------
...
So the second event has become a raw event as
/sys/devices/cpu/events/cpu-cycles exists.
A fix was necessary to config_term_pmu in parse-events.c as check_alias
expansion needs to happen after config_term_pmu, and config_term_pmu may
need calling a second time because of this.
config_term_pmu is updated to not use the legacy event when the PMU has
such a named event (either from JSON or sysfs).
The bulk of this change is updating all of the parse-events test
expectations so that if a sysfs/JSON event exists for a PMU the test
doesn't fail - a further sign, if it were needed, that the legacy event
priority was a known and tested behavior of the perf tool.
Reported-by: Hector Martin <marcan@marcan.st>
Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Hector Martin <marcan@marcan.st>
Tested-by: Marc Zyngier <maz@kernel.org>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: https://lore.kernel.org/r/20231123042922.834425-1-irogers@google.com
[ Initialize the 'alias_rewrote_terms' variable to false to address a clang warning ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Diffstat (limited to 'tools/perf/util/pmu.c')
| -rw-r--r-- | tools/perf/util/pmu.c | 8 |
1 files changed, 6 insertions, 2 deletions
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index d3c9aa4326be..3c9609944a2f 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -1494,12 +1494,14 @@ static int check_info_data(struct perf_pmu *pmu, * defined for the alias */ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_terms, - struct perf_pmu_info *info, struct parse_events_error *err) + struct perf_pmu_info *info, bool *rewrote_terms, + struct parse_events_error *err) { struct parse_events_term *term, *h; struct perf_pmu_alias *alias; int ret; + *rewrote_terms = false; info->per_pkg = false; /* @@ -1521,7 +1523,7 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct parse_events_terms *head_ NULL); return ret; } - + *rewrote_terms = true; ret = check_info_data(pmu, alias, info, err, term->err_term); if (ret) return ret; @@ -1615,6 +1617,8 @@ bool perf_pmu__auto_merge_stats(const struct perf_pmu *pmu) bool perf_pmu__have_event(struct perf_pmu *pmu, const char *name) { + if (!name) + return false; if (perf_pmu__find_alias(pmu, name, /*load=*/ true) != NULL) return true; if (pmu->cpu_aliases_added || !pmu->events_table) |