On 03.12.24 18:24, Thorsten Leemhuis wrote: > 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/ TWIMC, Florian Fainelli and Naresh Kamboju (both now CCed) meanwhile reported the same problem for the latest 6.11.y and 6.12.y rc releases: https://lore.kernel.org/all/5eeb1f9c-2b9f-49f1-9861-051478a8630d@xxxxxxxxx/ https://lore.kernel.org/all/713b5871-c7a3-44fa-a5ac-5cf558be81c9@xxxxxxxxx/ https://lore.kernel.org/all/CA+G9fYu21yqTvL428TFueMJ1uU1H_u8Vc470dER2CTrNK=Js0g@xxxxxxxxxxxxxx/ https://lore.kernel.org/all/CA+G9fYtNvEDcUEuv=QFC84y+pXY1UszoRYOitJztCApLV7-psg@xxxxxxxxxxxxxx/ Ciao, Thorsten > 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); >> >
