On 03.12.24 15:43, Greg Kroah-Hartman wrote: > 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: My 6.11.y-rc and 6.12.y-rc builds for Fedora failed when building perf. I did not bisect, but from a brief look at the error message (see below) I suspect it might be caused by this patch, which is the second of the patch-set "Event parsing fixes": https://lore.kernel.org/all/20240926144851.245903-1-james.clark@xxxxxxxxxx/ To my untrained eyes and from a quick look I guess the first patch in the series needs to be backported as well: perf evsel: Add alternate_hw_config and use in evsel__match https://lore.kernel.org/all/20240926144851.245903-2-james.clark@xxxxxxxxxx/ This is 22a4db3c36034e ("perf evsel: Add alternate_hw_config and use in evsel__match") in mainline. I tried to cherry-pick it on-top of 6.12.2-rc1, but there were a few small conflicts. Ciao, Thorsten P.S.: The build error: gcc -Wp,-MD,util/.topdown.o.d -Wp,-MT,util/topdown.o -O2 -fexceptions -g -grecord-gcc-switches -pipe -Wall -Wno-complain-wrong-lang -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -march=x86-64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -mtls-dialect=gnu2 -Wbad-function-cast -Wdeclaration-after-statement -Wformat-security -Wformat-y2k -Winit-self -Wmissing-declarations -Wmissing-prototypes -Wno-system-headers -Wold-style-definition -Wpacked -Wredundant-decls -Wstrict-prototypes -Wswitch-default -Wswitch-enum -Wundef -Wwrite-strings -Wformat -Wno-type-limits -Wstrict-aliasing=3 -Wshadow -DHAVE_SYSCALL_TABLE_SUPPORT -Iarch/x86/include/generated -DHAVE_ARCH_X86_64_SUPPORT -DHAVE_ARCH_REGS_QUERY_REGISTER_OFFSET -DNDEBUG=1 -O3 -fno-omit-frame-pointer -Wall -Wextra -std=gnu11 -fstack-protector-all -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/perf/util/include -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/perf/arch/x86/include -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/include/ -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/arch/x86/include/uapi -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/include/uapi -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/arch/x86/include/ -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/arch/x86/ -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/perf/util -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/perf -DHAVE_PTHREAD_ATTR_SETAFFINITY_NP -DHAVE_PTHREAD_BARRIER -DHAVE_EVENTFD_SUPPORT -DHAVE_GET_CURRENT_DIR_NAME -DHAVE_GETTID -DHAVE_FILE_HANDLE -DHAVE_DWARF_GETLOCATIONS_SUPPORT -DHAVE_DWARF_CFI_SUPPORT -DHAVE_AIO_SUPPORT -DHAVE_SCANDIRAT_SUPPORT -DHAVE_SCHED_GETCPU_SUPPORT -DHAVE_SETNS_SUPPORT -DHAVE_ZLIB_SUPPORT -DHAVE_LIBELF_SUPPORT -DHAVE_ELF_GETPHDRNUM_SUPPORT -DHAVE_GELF_GETNOTE_SUPPORT -DHAVE_ELF_GETSHDRSTRNDX_SUPPORT -DHAVE_DWARF_SUPPORT -DHAVE_LIBBPF_SUPPORT -DHAVE_SDT_EVENT -DHAVE_JITDUMP -DHAVE_BPF_SKEL -DHAVE_DWARF_UNWIND_SUPPORT -DHAVE_LIBCRYPTO_SUPPORT -DHAVE_SLANG_SUPPORT -DHAVE_LIBPERL_SUPPORT -DHAVE_TIMERFD_SUPPORT -DHAVE_LIBPYTHON_SUPPORT -fPIC -DHAVE_LIBLLVM_SUPPORT -I/usr/include -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DHAVE_CXA_DEMANGLE_SUPPORT -DHAVE_LZMA_SUPPORT -DHAVE_ZSTD_SUPPORT -DHAVE_BACKTRACE_SUPPORT -DHAVE_LIBNUMA_SUPPORT -DHAVE_KVM_STAT_SUPPORT -DDISASM_FOUR_ARGS_SIGNATURE -DDISASM_INIT_STYLED -DHAVE_LIBBABELTRACE_SUPPORT -DHAVE_AUXTRACE_SUPPORT -DHAVE_JVMTI_CMLR -DHAVE_LIBTRACEEVENT -I/usr/include/traceevent -DLIBTRACEEVENT_VERSION=67067 -I/usr/include/tracefs -I/usr/include/traceevent -DLIBTRACEFS_VERSION=67065 -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/perf/libapi/include -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/perf/libsubcmd/include -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/perf/libsymbol/include -I/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/perf/libperf/include -D"BUILD_STR(s)=#s" -c -o util/topdown.o util/topdown.c util/stat-display.c: In function ‘uniquify_event_name’: util/stat-display.c:895:45: error: ‘struct evsel’ has no member named ‘alternate_hw_config’ 895 | if (counter->pmu->is_core && counter->alternate_hw_config != PERF_COUNT_HW_MAX) | ^~ make[4]: *** [/builddir/build/BUILD/kernel-6.12.2-build/kernel-6.12.2-rc1/linux-6.12.2-0.rc1.400.vanilla.fc41.x86_64/tools/build/Makefile.build:106: util/stat-display.o] Error 1 make[4]: *** Waiting for unfinished jobs.... > ``` > $ 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); >
