| 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: <CAP-5=fUDex9Co5-_fkQD4a_fS_+Y5k_rewcPYR+gzxyg3YdAPQ@mail.gmail.com>
Date: Fri, 6 Feb 2026 11:19:13 -0800
From: Ian Rogers <irogers@...gle.com>
To: Leo Yan <leo.yan@....com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, James Clark <james.clark@...aro.org>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf record: Simplify open event error logs
On Fri, Feb 6, 2026 at 8:29 AM Leo Yan <leo.yan@....com> wrote:
>
> Instead of printing error messages for every event failure, this commit
> prints a log only when the first error occurs.
>
> Duplicate event names are a special case. Print an explicit log message
> only when verbose logging is enabled.
>
> Signed-off-by: Leo Yan <leo.yan@....com>
Nack. I find the logic harder to read and "event name duplicated" ? We
silence the warning specifically on cycles for ARM due to Linus not
wanting the spam. It doesn't make sense to me that we may fail to open
>1 event for sampling, but still open 1 event.. but we only complain
about 1 of the events failing to open which is misleading.
I think this is motivated by not wanting to see >1 warning when there
is a permission issue, so let's special case EPERM and if we just show
1 warning for that then we must terminate perf record and not hide
other issues. Maybe is_event_name_duplicated can be called
is_special_case_keep_quiet.
Thanks,
Ian
> ---
> tools/perf/builtin-record.c | 69 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 43 insertions(+), 26 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index aa8dc3e18190dd7db1ed5e3c7673fde8d5785a26..9382ff57a822ad4ee5391116510941ce180a12d8 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1343,6 +1343,31 @@ static int record__mmap(struct record *rec)
> return record__mmap_evlist(rec, rec->evlist);
> }
>
> +static bool record__is_event_name_duplicated(struct evlist *evlist,
> + struct evsel *pos)
> +{
> +#if defined(__aarch64__) || defined(__arm__)
> + if (strstr(evsel__name(pos), "cycles")) {
> + struct evsel *pos2;
> +
> + /*
> + * Unfortunately ARM has many events named
> + * "cycles" on PMUs like the system-level (L3)
> + * cache which don't support sampling. Only
> + * display such failures to open when there is
> + * only 1 cycles event or verbose is enabled.
> + */
> + evlist__for_each_entry(evlist, pos2) {
> + if (pos2 == pos)
> + continue;
> + if (strstr(evsel__name(pos2), "cycles"))
> + return true;
> + }
> + }
> +#endif
> + return false;
> +}
> +
> static int record__open(struct record *rec)
> {
> char msg[BUFSIZ];
> @@ -1353,6 +1378,7 @@ static int record__open(struct record *rec)
> int rc = 0;
> bool skipped = false;
> bool removed_tracking = false;
> + bool report_once = true;
>
> evlist__for_each_entry(evlist, pos) {
> if (removed_tracking) {
> @@ -1370,8 +1396,6 @@ static int record__open(struct record *rec)
> }
> try_again:
> if (evsel__open(pos, pos->core.cpus, pos->core.threads) < 0) {
> - bool report_error = true;
> -
> if (evsel__fallback(pos, &opts->target, errno, msg, sizeof(msg))) {
> if (verbose > 0)
> ui__warning("%s\n", msg);
> @@ -1383,36 +1407,29 @@ static int record__open(struct record *rec)
> pos = evlist__reset_weak_group(evlist, pos, true);
> goto try_again;
> }
> -#if defined(__aarch64__) || defined(__arm__)
> - if (strstr(evsel__name(pos), "cycles")) {
> - struct evsel *pos2;
> - /*
> - * Unfortunately ARM has many events named
> - * "cycles" on PMUs like the system-level (L3)
> - * cache which don't support sampling. Only
> - * display such failures to open when there is
> - * only 1 cycles event or verbose is enabled.
> - */
> - evlist__for_each_entry(evlist, pos2) {
> - if (pos2 == pos)
> - continue;
> - if (strstr(evsel__name(pos2), "cycles")) {
> - report_error = false;
> - break;
> - }
> - }
> +
> + if (pos->tracking)
> + removed_tracking = true;
> + pos->skippable = true;
> + skipped = true;
> +
> + if (record__is_event_name_duplicated(evlist, pos)) {
> + if (verbose > 0)
> + ui__error("Skip to open event '%s' on PMU '%s' due to "
> + "the duplicated naming.\n",
> + evsel__name(pos), evsel__pmu_name(pos));
> + continue;
> }
> -#endif
> - if (report_error || verbose > 0) {
> +
> + if (verbose > 0 || report_once) {
> evsel__open_strerror(pos, &opts->target, errno, msg, sizeof(msg));
> ui__error("Failure to open event '%s' on PMU '%s' which will be "
> "removed.\n%s\n",
> evsel__name(pos), evsel__pmu_name(pos), msg);
> +
> + if (report_once)
> + report_once = false;
> }
> - if (pos->tracking)
> - removed_tracking = true;
> - pos->skippable = true;
> - skipped = true;
> }
> }
>
>
> ---
> base-commit: 3d8b2baefecdd5e84a1c738bcfdb079aa536d997
> change-id: 20260204-perf_improve_log_for_open_event_failures-deda2c61b93d
>
> Best regards,
> --
> Leo Yan <leo.yan@....com>
>
Powered by blists - more mailing lists