Re: [V4 04/16] tools/perf: Use sort keys to determine whether to pick objdump to disassemble

Namhyung Kim
2024-06-25T05:32:24Z

On Fri, Jun 14, 2024 at 10:56:19PM +0530, Athira Rajeev wrote:
> perf annotate can be done in different ways. One way is to directly use
> "perf annotate" command, other way to annotate specific symbol is to do
> "perf report" and press "a" on the sample in UI mode. The approach
> preferred in powerpc to parse sample for data type profiling is:
> - Read directly from DSO using dso__data_read_offset
> - If that fails for any case, fallback to using libcapstone
> - If libcapstone is not supported, approach will use objdump
>
> The above works well when perf report is invoked with only sort keys for
> data type ie type and typeoff. Because there is no instruction level
> annotation needed if only data type information is requested for. For
> annotating sample, along with type and typeoff sort key, "sym" sort key
> is also needed. And by default invoking just "perf report" uses sort key
> "sym" that displays the symbol information.
>
> With approach changes in powerpc which first reads DSO for raw
> instruction, "perf annotate" and "perf report" + a key breaks since
> it doesn't do the instruction level disassembly.

So as I said, it'd be nice you can read the raw insn from the objdump
output directly.

Thanks,
Namhyung

>
> Snippet of result from perf report:
>
> Samples: 1K of event 'mem-loads', 4000 Hz, Event count (approx.): 937238
> do_work /usr/bin/pmlogger [Percent: local period]
> Percent│ ea230010
> │ 3a550010
> │ 3a600000
>
> │ 38f60001
> │ 39490008
> │ 42400438
> 51.44 │ 81290008
> │ 7d485378
>
> Here, raw instruction is displayed in the output instead of human
> readable annotated form.
>
> One way to get the appropriate data is to specify "--objdump path", by
> which code annotation will be done. But the default behaviour will be
> changed. To fix this breakage, check if "sym" sort key is set. If so
> fallback and use the libcapstone/objdump way of disassmbling the sample.
>
> With the changes and "perf report"
>
> Samples: 1K of event 'mem-loads', 4000 Hz, Event count (approx.): 937238
> do_work /usr/bin/pmlogger [Percent: local period]
> Percent│ ld r17,16(r3)
> │ addi r18,r21,16
> │ li r19,0
>
> │ 8b0: rldicl r10,r10,63,33
> │ addi r10,r10,1
> │ mtctr r10
> │ ↓ b 8e4
> │ 8c0: addi r7,r22,1
> │ addi r10,r9,8
> │ ↓ bdz d00
> 51.44 │ lwz r9,8(r9)
> │ mr r8,r10
> │ cmpw r20,r9
>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> ---
> tools/perf/util/disasm.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index f19496133bf0..b81cdcf4d6b4 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -25,6 +25,7 @@
> #include "srcline.h"
> #include "symbol.h"
> #include "util.h"
> +#include "sort.h"
>
> static regex_t file_lineno;
>
> @@ -1803,9 +1804,11 @@ int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
> * not required in case of powerpc.
> */
> if (arch__is(args->arch, "powerpc")) {
> - err = symbol__disassemble_dso(symfs_filename, sym, args);
> - if (err == 0)
> - goto out_remove_tmp;
> + if (sort_order && !strstr(sort_order, "sym")) {
> + err = symbol__disassemble_dso(symfs_filename, sym, args);
> + if (err == 0)
> + goto out_remove_tmp;
> + }
> }
>
> #ifdef HAVE_LIBCAPSTONE_SUPPORT
> --
> 2.43.0
>