All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Aaron Tomlin" <atomlin@atomlin.com>
Cc: linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v3] perf trace: Introduce --show-cpu option to display cpu id
Date: Thu, 23 Apr 2026 22:43:28 +0000	[thread overview]
Message-ID: <20260423224329.14907C2BCAF@smtp.kernel.org> (raw)
In-Reply-To: <20260423194032.134416-1-atomlin@atomlin.com>

Sashiko AI review found 1 potential issue(s):
- [Medium] The `--show-cpu` option fails to print the CPU ID for interrupted system call entries, resulting in lost information and broken columnar alignment.
--

commit f837c4ea902d9b8f1e88b211cf0a2320cd3c7a26
Author: Aaron Tomlin <atomlin@atomlin.com>
perf trace: Introduce --show-cpu option to display cpu id

This commit introduces the --show-cpu option for perf trace. It extracts 
the CPU from the perf sample and prints it alongside the timestamp to 
better trace system-wide workloads.

> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 311d9da9896a41..820b0a304ff90f 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c

[ ... ]

> @@ -2704,7 +2729,9 @@ static int trace__printf_interrupted_entry(struct trace *trace)
>  	if (!ttrace->entry_pending)
>  		return 0;
>  
> -	printed  = trace__fprintf_entry_head(trace, trace->current, 0, false, ttrace->entry_time, trace->output);
> +	printed = trace__fprintf_entry_head(trace, trace->current, 0, false,
> +					    ttrace->entry_time, (u32)-1,
> +					    trace->output);

When a system call entry is processed in trace__sys_enter(), it is often
buffered (ttrace->entry_pending = true) so it can be combined with its
exit event on a single line. Does hardcoding (u32)-1 here cause the CPU ID
to be lost when a pending entry is interrupted?

Since trace__fprintf_entry_head() skips printing the CPU ID entirely when
the cpu is (u32)-1, it appears this will omit the CPU column for interrupted
entries (which print with " ..."). Could this break the columnar alignment of
the output?

Would it be better to extend struct thread_trace to store the cpu ID from the
sample during trace__sys_enter(), so it can be accurately printed when the
entry is flushed here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423194032.134416-1-atomlin@atomlin.com?part=1

      reply	other threads:[~2026-04-23 22:43 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-23 19:40 [PATCH v3] perf trace: Introduce --show-cpu option to display cpu id Aaron Tomlin
2026-04-23 22:43 ` 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=20260423224329.14907C2BCAF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=atomlin@atomlin.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko@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.