From: sashiko-bot@kernel.org
To: "Arnaldo Carvalho de Melo" <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 2/2] perf trace: Guard __probe_ip suppression with evsel__is_probe()
Date: Tue, 16 Jun 2026 01:54:22 +0000 [thread overview]
Message-ID: <20260616015423.0B6DC1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260616013157.38960-3-acme@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `evsel__is_probe()` check evaluates to false for Ftrace dynamic probes, breaking the intended suppression of the `__probe_ip` field.
--
commit 0e15cda4a40a59d474ee0a3a5cdae64267b1f3b2
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
perf trace: Guard __probe_ip suppression with evsel__is_probe()
This commit adds an evsel__is_probe() check to trace__fprintf_tp_fields()
to skip the string comparison of __probe_ip for non-probe events.
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index a8492da23a9cc..57f3f14c5d435 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -3267,7 +3267,7 @@ static size_t trace__fprintf_tp_fields(struct trace *trace, struct perf_sample *
> * If verbose mode is enabled, ensure it is formatted as a
> * hexadecimal memory address rather than a signed integer.
> */
> - if (!strcmp(field->name, "__probe_ip")) {
> + if (evsel__is_probe(evsel) && !strcmp(field->name, "__probe_ip")) {
[Severity: High]
Does this check accidentally disable the suppression entirely for Ftrace
dynamic probes?
Since trace__fprintf_tp_fields() processes fields specifically for tracepoint
events (where evsel__tp_format() succeeds), the event has type
PERF_TYPE_TRACEPOINT. For these events, the PMU name is "tracepoint".
However, evsel__is_probe() resolves the probe type by checking if the PMU
name is "kprobe" or "uprobe". If it evaluates to false here because the PMU
is "tracepoint", the condition short-circuits and the __probe_ip field is
no longer suppressed.
Will this cause __probe_ip to be erroneously printed for all dynamic probes
instead of being suppressed by default as intended?
> if (!verbose)
> continue;
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616013157.38960-1-acme@kernel.org?part=2
prev parent reply other threads:[~2026-06-16 1:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-16 1:31 [PATCHES 0/2] perf tools: Add cached probe type detection for evsel Arnaldo Carvalho de Melo
2026-06-16 1:31 ` [PATCH 1/2] perf evsel: Add lazy-initialized probe type detection helpers Arnaldo Carvalho de Melo
2026-06-16 1:31 ` [PATCH 2/2] perf trace: Guard __probe_ip suppression with evsel__is_probe() Arnaldo Carvalho de Melo
2026-06-16 1:54 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260616015423.0B6DC1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=acme@kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.