6.12-stable review patch. If anyone has any objections, please let me know.
------------------
From: Ian Rogers <irogers@xxxxxxxxxx>
[ Upstream commit 057f8bfc6f7070577523d1e3081081bbf4229c1c ]
Without aggregation on Intel:
```
$ perf stat -e instructions,cycles ...
```
Will use "cycles" for the name of the legacy cycles event but as
"instructions" has a sysfs name it will and a "[cpu]" PMU suffix. This
often breaks things as the space between the event and the PMU name
look like an extra column. The existing uniquify logic was also
uniquifying in cases when all events are core and not with uncore
events, it was not correctly handling modifiers, etc.
Change the logic so that an initial pass that can disable
uniquification is run. For individual counters, disable uniquification
in more cases such as for consistency with legacy events or for
libpfm4 events. Don't use the "[pmu]" style suffix in uniquification,
always use "pmu/.../". Change how modifiers/terms are handled in the
uniquification so that they look like parse-able events.
This fixes "102: perf stat metrics (shadow stat) test:" that has been
failing due to "instructions [cpu]" breaking its column/awk logic when
values aren't aggregated. This started happening when instructions
could match a sysfs rather than a legacy event, so the fixes tag
reflects this.
Fixes: 617824a7f0f7 ("perf parse-events: Prefer sysfs/JSON hardware events over legacy")
Acked-by: Namhyung Kim <namhyung@xxxxxxxxxx>
Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
[ Fix Intel TPEBS counting mode test ]
Acked-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
Signed-off-by: James Clark <james.clark@xxxxxxxxxx>
Cc: Yang Jihong <yangjihong@xxxxxxxxxxxxx>
Cc: Dominique Martinet <asmadeus@xxxxxxxxxxxxx>
Cc: Colin Ian King <colin.i.king@xxxxxxxxx>
Cc: Howard Chu <howardchu95@xxxxxxxxx>
Cc: Ze Gao <zegao2021@xxxxxxxxx>
Cc: Yicong Yang <yangyicong@xxxxxxxxxxxxx>
Cc: Weilin Wang <weilin.wang@xxxxxxxxx>
Cc: Will Deacon <will@xxxxxxxxxx>
Cc: Mike Leach <mike.leach@xxxxxxxxxx>
Cc: Jing Zhang <renyu.zj@xxxxxxxxxxxxxxxxx>
Cc: Yang Li <yang.lee@xxxxxxxxxxxxxxxxx>
Cc: Leo Yan <leo.yan@xxxxxxxxx>
Cc: ak@xxxxxxxxxxxxxxx
Cc: Athira Rajeev <atrajeev@xxxxxxxxxxxxxxxxxx>
Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Cc: Sun Haiyong <sunhaiyong@xxxxxxxxxxx>
Cc: John Garry <john.g.garry@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20240926144851.245903-3-james.clark@xxxxxxxxxx
Signed-off-by: Namhyung Kim <namhyung@xxxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
.../perf/tests/shell/test_stat_intel_tpebs.sh | 11 +-
tools/perf/util/stat-display.c | 101 ++++++++++++++----
2 files changed, 85 insertions(+), 27 deletions(-)
diff --git a/tools/perf/tests/shell/test_stat_intel_tpebs.sh b/tools/perf/tests/shell/test_stat_intel_tpebs.sh
index c60b29add9801..9a11f42d153ca 100755
--- a/tools/perf/tests/shell/test_stat_intel_tpebs.sh
+++ b/tools/perf/tests/shell/test_stat_intel_tpebs.sh
@@ -8,12 +8,15 @@ grep -q GenuineIntel /proc/cpuinfo || { echo Skipping non-Intel; exit 2; }
# Use this event for testing because it should exist in all platforms
event=cache-misses:R
+# Hybrid platforms output like "cpu_atom/cache-misses/R", rather than as above
+alt_name=/cache-misses/R
+
# Without this cmd option, default value or zero is returned
-echo "Testing without --record-tpebs"
-result=$(perf stat -e "$event" true 2>&1)
-[[ "$result" =~ $event ]] || exit 1
+#echo "Testing without --record-tpebs"
+#result=$(perf stat -e "$event" true 2>&1)
+#[[ "$result" =~ $event || "$result" =~ $alt_name ]] || exit 1
# In platforms that do not support TPEBS, it should execute without error.
echo "Testing with --record-tpebs"
result=$(perf stat -e "$event" --record-tpebs -a sleep 0.01 2>&1)
-[[ "$result" =~ "perf record" && "$result" =~ $event ]] || exit 1
+[[ "$result" =~ "perf record" && "$result" =~ $event || "$result" =~ $alt_name ]] || exit 1
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index ea96e4ebad8c8..cbff43ff8d0fb 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -871,38 +871,66 @@ static void printout(struct perf_stat_config *config, struct outstate *os,
static void uniquify_event_name(struct evsel *counter)
{
- char *new_name;
- char *config;
- int ret = 0;
+ const char *name, *pmu_name;
+ char *new_name, *config;
+ int ret;
- if (counter->uniquified_name || counter->use_config_name ||
- !counter->pmu_name || !strncmp(evsel__name(counter), counter->pmu_name,
- strlen(counter->pmu_name)))
+ /* The evsel was already uniquified. */
+ if (counter->uniquified_name)
return;
- config = strchr(counter->name, '/');
+ /* Avoid checking to uniquify twice. */
+ counter->uniquified_name = true;
+
+ /* The evsel has a "name=" config term or is from libpfm. */
+ if (counter->use_config_name || counter->is_libpfm_event)
+ return;
+
+ /* Legacy no PMU event, don't uniquify. */
+ if (!counter->pmu ||
+ (counter->pmu->type < PERF_TYPE_MAX && counter->pmu->type != PERF_TYPE_RAW))
+ return;
+
+ /* A sysfs or json event replacing a legacy event, don't uniquify. */
+ if (counter->pmu->is_core && counter->alternate_hw_config != PERF_COUNT_HW_MAX)
+ return;
+
+ name = evsel__name(counter);
+ pmu_name = counter->pmu->name;
+ /* Already prefixed by the PMU name. */
+ if (!strncmp(name, pmu_name, strlen(pmu_name)))
+ return;
+
+ config = strchr(name, '/');
if (config) {
- if (asprintf(&new_name,
- "%s%s", counter->pmu_name, config) > 0) {
- free(counter->name);
- counter->name = new_name;
- }
- } else {
- if (evsel__is_hybrid(counter)) {
- ret = asprintf(&new_name, "%s/%s/",
- counter->pmu_name, counter->name);
+ int len = config - name;
+
+ if (config[1] == '/') {
+ /* case: event// */
+ ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 2);
} else {
- ret = asprintf(&new_name, "%s [%s]",
- counter->name, counter->pmu_name);
+ /* case: event/.../ */
+ ret = asprintf(&new_name, "%s/%.*s,%s", pmu_name, len, name, config + 1);
}
+ } else {
+ config = strchr(name, ':');
+ if (config) {
+ /* case: event:.. */
+ int len = config - name;
- if (ret) {
- free(counter->name);
- counter->name = new_name;
+ ret = asprintf(&new_name, "%s/%.*s/%s", pmu_name, len, name, config + 1);
+ } else {
+ /* case: event */
+ ret = asprintf(&new_name, "%s/%s/", pmu_name, name);
}
}
-
- counter->uniquified_name = true;
+ if (ret > 0) {
+ free(counter->name);
+ counter->name = new_name;
+ } else {
+ /* ENOMEM from asprintf. */
+ counter->uniquified_name = false;
+ }
}
static bool hybrid_uniquify(struct evsel *evsel, struct perf_stat_config *config)
@@ -1559,6 +1587,31 @@ static void print_cgroup_counter(struct perf_stat_config *config, struct evlist
print_metric_end(config, os);
}
+static void disable_uniquify(struct evlist *evlist)
+{
+ struct evsel *counter;
+ struct perf_pmu *last_pmu = NULL;
+ bool first = true;
+
+ evlist__for_each_entry(evlist, counter) {
+ /* If PMUs vary then uniquify can be useful. */
+ if (!first && counter->pmu != last_pmu)
+ return;
+ first = false;
+ if (counter->pmu) {
+ /* Allow uniquify for uncore PMUs. */
+ if (!counter->pmu->is_core)
+ return;
+ /* Keep hybrid event names uniquified for clarity. */
+ if (perf_pmus__num_core_pmus() > 1)
+ return;
+ }
+ }
+ evlist__for_each_entry_continue(evlist, counter) {
+ counter->uniquified_name = true;
+ }
+}
+
void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *config,
struct target *_target, struct timespec *ts,
int argc, const char **argv)
@@ -1572,6 +1625,8 @@ void evlist__print_counters(struct evlist *evlist, struct perf_stat_config *conf
.first = true,
};
+ disable_uniquify(evlist);
+
if (config->iostat_run)
evlist->selected = evlist__first(evlist);
--
2.43.0
