linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andi Kleen <ak@linux.intel.com>
To: Changbin Du <changbin.du@huawei.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	 Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	 Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	 Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	 <linux-kernel@vger.kernel.org>,
	<linux-perf-users@vger.kernel.org>
Subject: Re: [PATCH 0/3] perf: script: Intro capstone disasm engine to show instruction trace
Date: Tue, 16 Jan 2024 05:53:40 -0800	[thread overview]
Message-ID: <87il3tl6zf.fsf@linux.intel.com> (raw)
In-Reply-To: <20240116113437.1507537-1-changbin.du@huawei.com> (Changbin Du's message of "Tue, 16 Jan 2024 19:34:34 +0800")

Changbin Du <changbin.du@huawei.com> writes:

> This series introduces capstone disassembler engine to print instructions of
> Intel PT trace, which was printed via the XED tool.

FWIW at least on x86 in my experience capstone isn't that great an
disassembler. I used it in another project and ran into many decoding bugs.
They're mostly in obscure corners, but can be fairly annoying.

My other concern with your patchkit is that you change the default
output formats. Since perf script is often used with scripts
(as the name implies) there is a certain expectation that the output
remains stable and parse-able. There are actually use cases where
the raw bytes "insn" output is needed.

I would rather define new perf script output types for the new decoded output,
but keep the old alone.

-Andi

  parent reply	other threads:[~2024-01-16 13:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-16 11:34 [PATCH 0/3] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
2024-01-16 11:34 ` [PATCH 1/3] perf: build: introduce the libcapstone Changbin Du
2024-01-16 11:34 ` [PATCH 2/3] perf: script: use capstone disasm engine to show assembly instructions Changbin Du
2024-01-16 14:26   ` Thomas Richter
2024-01-17  2:17     ` Changbin Du
2024-01-16 11:34 ` [PATCH 3/3] perf: script: deprecate the '--xed' option Changbin Du
2024-01-16 13:53 ` Andi Kleen [this message]
2024-01-17  2:45   ` [PATCH 0/3] perf: script: Intro capstone disasm engine to show instruction trace Changbin Du
2024-01-17  3:48     ` Namhyung Kim

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=87il3tl6zf.fsf@linux.intel.com \
    --to=ak@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=changbin.du@huawei.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).