From: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
Jiri Olsa <jolsa@kernel.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Ian Rogers <irogers@google.com>,
Segher Boessenkool <segher@kernel.crashing.org>,
Christophe Leroy <christophe.leroy@csgroup.eu>,
linux-perf-users <linux-perf-users@vger.kernel.org>,
linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
Madhavan Srinivasan <maddy@linux.ibm.com>,
Kajol Jain <kjain@linux.ibm.com>,
Disha Goel <disgoel@linux.vnet.ibm.com>,
LKML <linux-kernel@vger.kernel.org>,
akanksha@linux.ibm.com
Subject: Re: [PATCH V2 0/9] Add data type profiling support for powerpc
Date: Mon, 13 May 2024 13:02:10 +0530 [thread overview]
Message-ID: <C5802D81-BBBE-4409-8887-5DBB2269BCB5@linux.vnet.ibm.com> (raw)
In-Reply-To: <CAM9d7ci=AZFpcYy4=qwTYTTVhWd6g6HHHUuV2B53hS-t8SOCag@mail.gmail.com>
> On 7 May 2024, at 10:09 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> On Mon, May 6, 2024 at 5:19 AM Athira Rajeev
> <atrajeev@linux.vnet.ibm.com> wrote:
>>
>> The patchset from Namhyung added support for data type profiling
>> in perf tool. This enabled support to associate PMU samples to data
>> types they refer using DWARF debug information. With the upstream
>> perf, currently it possible to run perf report or perf annotate to
>> view the data type information on x86.
>>
>> Initial patchset posted here had changes need to enable data type
>> profiling support for powerpc.
>>
>> https://lore.kernel.org/all/6e09dc28-4a2e-49d8-a2b5-ffb3396a9952@csgroup.eu/T/
>>
>> Main change were:
>> 1. powerpc instruction nmemonic table to associate load/store
>> instructions with move_ops which is use to identify if instruction
>> is a memory access one.
>> 2. To get register number and access offset from the given
>> instruction, code uses fields from "struct arch" -> objump.
>> Added entry for powerpc here.
>> 3. A get_arch_regnum to return register number from the
>> register name string.
>>
>> But the apporach used in the initial patchset used parsing of
>> disassembled code which the current perf tool implementation does.
>>
>> Example: lwz r10,0(r9)
>>
>> This line "lwz r10,0(r9)" is parsed to extract instruction name,
>> registers names and offset. Also to find whether there is a memory
>> reference in the operands, "memory_ref_char" field of objdump is used.
>> For x86, "(" is used as memory_ref_char to tackle instructions of the
>> form "mov (%rax), %rcx".
>>
>> In case of powerpc, not all instructions using "(" are the only memory
>> instructions. Example, above instruction can also be of extended form (X
>> form) "lwzx r10,0,r19". Inorder to easy identify the instruction category
>> and extract the source/target registers, this patchset adds support to use
>> raw instruction. With raw instruction, macros are added to extract opcode
>> and register fields.
>>
>> Example representation using --show-raw-insn in objdump gives result:
>>
>> 38 01 81 e8 ld r4,312(r1)
>>
>> Here "38 01 81 e8" is the raw instruction representation. In powerpc,
>> this translates to instruction form: "ld RT,DS(RA)" and binary code
>> as:
>> _____________________________________
>> | 58 | RT | RA | DS | |
>> -------------------------------------
>> 0 6 11 16 30 31
>>
>> Patchset adds support to pick the opcode and reg fields from this
>> raw/binary instruction code. This approach came in from review comment
>> by Segher Boessenkool for the initial patchset.
>>
>> Apart from that, instruction tracking is enabled for powerpc and
>> support function is added to find variables defined as registers
>> Example, in powerpc, two registers are
>> defined to represent variable:
>> 1. r13: represents local_paca
>> register struct paca_struct *local_paca asm("r13");
>>
>> 2. r1: represents stack_pointer
>> register void *__stack_pointer asm("r1");
>>
>> These are handled in this patchset.
>>
>> - Patch 1 is to rearrange register state type structures to header file
>> so that it can referred from other arch specific files
>> - Patch 2 is to make instruction tracking as a callback to"struct arch"
>> so that it can be implemented by other archs easily and defined in arch
>> specific files
>> - Patch 3 is to fix a small comment
>> - Patch 4 adds support to capture and parse raw instruction in objdump
>> by keeping existing approach intact.
>> - Patch 5 update parameters for reg extract functions to use raw
>> instruction on powerpc
>> - Patch 6 and patch 7 handles instruction tracking for powerpc.
>> - Patch 8 and Patch 8 handles support to find global register variables
>>
>> With the current patchset:
>>
>> ./perf record -a -e mem-loads sleep 1
>> ./perf report -s type,typeoff --hierarchy --group --stdio
>> ./perf annotate --data-type --insn-stat
>>
>> perf annotate logs:
>>
>> Annotate Instruction stats
>> total 562, ok 441 (78.5%), bad 121 (21.5%)
>>
>> Name : Good Bad
>> -----------------------------------------------------------
>> ld : 313 54
>> lwz : 51 32
>> lbz : 31 5
>> ldx : 6 21
>> lhz : 23 0
>> lwa : 4 3
>> lwarx : 5 0
>> lwzx : 2 2
>> ldarx : 3 0
>> lwzu : 2 0
>> stdcx. : 0 1
>> nop : 0 1
>> ldu : 1 0
>> lbzx : 0 1
>> lwax : 0 1
>>
>> perf report logs:
>>
>> # Samples: 1K of event 'mem-loads'
>> # Event count (approx.): 937238
>> #
>> # Overhead Data Type Data Type Offset
>> # ........ ......... ................
>> #
>> 48.81% (unknown) (unknown) +0 (no field)
>> 12.85% long unsigned int long unsigned int +0 (current_stack_pointer)
>> 4.68% struct paca_struct struct paca_struct +2312 (__current)
>> 4.57% struct paca_struct struct paca_struct +2354 (irq_soft_mask)
>> 2.68% struct paca_struct struct paca_struct +8 (paca_index)
>> 2.64% struct paca_struct struct paca_struct +2808 (canary)
>> 2.24% struct paca_struct struct paca_struct +48 (data_offset)
>> 1.41% struct vm_fault struct vm_fault +0 (vma)
>> 1.29% struct task_struct struct task_struct +276 (flags)
>> 1.03% struct pt_regs struct pt_regs +264 (user_regs.msr)
>> 1.00% struct menu_device struct menu_device +4 (tick_wakeup)
>> 0.90% struct security_hook_list struct security_hook_list +0 (list.next)
>> 0.76% struct irq_desc struct irq_desc +304 (irq_data.chip)
>> 0.76% struct rq struct rq +2856 (cpu)
>
> Looks great! I'm glad it worked on powerpc too.
> We still need to verify the returned type is properly annotated.
> But overall it looks really good, I will leave comments in reply.
Hi Namhyung
Thanks a lot for looking at the patchset and sharing the review comments.
I will address review comments and respond to them sooner.
Thanks
Athira
>
> Thanks,
> Namhyung
>
>>
>> Thanks
>> Athira Rajeev
>>
>> Changelog:
>> From v1->v2:
>> - Addressed suggestion from Christophe Leroy and Segher Boessenkool
>> to use the binary code (raw insn) to fetch opcode, register and
>> offset fields.
>> - Added support for instruction tracking in powerpc
>> - Find the register defined variables (r13 and r1 which points to
>> local_paca and current_stack_pointer in powerpc)
>>
>> Athira Rajeev (9):
>> tools/perf: Move the data structures related to register type to
>> header file
>> tools/perf: Add "update_insn_state" callback function to handle arch
>> specific instruction tracking
>> tools/perf: Fix a comment about multi_regs in extract_reg_offset
>> function
>> tools/perf: Add support to capture and parse raw instruction in
>> objdump
>> tools/perf: Update parameters for reg extract functions to use raw
>> instruction on powerpc
>> tools/perf: Update instruction tracking for powerpc
>> tools/perf: Update instruction tracking with add instruction
>> tools/perf: Add support to find global register variables using
>> find_data_type_global_reg
>> tools/perf: Add support for global_die to capture name of variable in
>> case of register defined variable
>>
>> tools/include/linux/string.h | 2 +
>> tools/lib/string.c | 13 +
>> .../perf/arch/powerpc/annotate/instructions.c | 84 +++
>> tools/perf/arch/powerpc/util/dwarf-regs.c | 52 ++
>> tools/perf/arch/x86/annotate/instructions.c | 383 +++++++++++++
>> tools/perf/util/annotate-data.c | 519 +++---------------
>> tools/perf/util/annotate-data.h | 78 +++
>> tools/perf/util/annotate.c | 32 +-
>> tools/perf/util/annotate.h | 1 +
>> tools/perf/util/disasm.c | 109 +++-
>> tools/perf/util/disasm.h | 17 +-
>> tools/perf/util/dwarf-aux.c | 1 +
>> tools/perf/util/dwarf-aux.h | 1 +
>> tools/perf/util/include/dwarf-regs.h | 12 +
>> tools/perf/util/sort.c | 7 +-
>> 15 files changed, 854 insertions(+), 457 deletions(-)
>>
>> --
>> 2.43.0
prev parent reply other threads:[~2024-05-13 7:32 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-06 12:18 [PATCH V2 0/9] Add data type profiling support for powerpc Athira Rajeev
2024-05-06 12:18 ` [PATCH V2 1/9] tools/perf: Move the data structures related to register type to header file Athira Rajeev
2024-05-06 12:18 ` [PATCH V2 2/9] tools/perf: Add "update_insn_state" callback function to handle arch specific instruction tracking Athira Rajeev
2024-05-06 12:19 ` [PATCH V2 3/9] tools/perf: Fix a comment about multi_regs in extract_reg_offset function Athira Rajeev
2024-05-07 4:40 ` Namhyung Kim
2024-05-07 14:52 ` Arnaldo Carvalho de Melo
2024-05-06 12:19 ` [PATCH V2 4/9] tools/perf: Add support to capture and parse raw instruction in objdump Athira Rajeev
2024-05-07 4:57 ` Namhyung Kim
2024-05-07 9:35 ` Christophe Leroy
2024-05-09 17:26 ` Athira Rajeev
2024-05-09 20:55 ` Namhyung Kim
2024-05-10 14:26 ` Arnaldo Carvalho de Melo
2024-05-22 13:58 ` Athira Rajeev
2024-05-06 12:19 ` [PATCH V2 5/9] tools/perf: Update parameters for reg extract functions to use raw instruction on powerpc Athira Rajeev
2024-05-07 9:48 ` Christophe Leroy
2024-05-22 14:06 ` Athira Rajeev
2024-05-06 12:19 ` [PATCH V2 6/9] tools/perf: Update instruction tracking for powerpc Athira Rajeev
2024-05-07 9:52 ` Christophe Leroy
2024-05-23 13:58 ` Athira Rajeev
2024-05-06 12:19 ` [PATCH V2 7/9] tools/perf: Update instruction tracking with add instruction Athira Rajeev
2024-05-07 9:58 ` Christophe Leroy
2024-05-23 13:55 ` Athira Rajeev
2024-05-06 12:19 ` [PATCH V2 8/9] tools/perf: Add support to find global register variables using find_data_type_global_reg Athira Rajeev
2024-05-07 10:03 ` Christophe Leroy
2024-05-24 12:17 ` Athira Rajeev
2024-05-24 12:47 ` Christophe Leroy
2024-05-06 12:19 ` [PATCH V2 9/9] tools/perf: Add support for global_die to capture name of variable in case of register defined variable Athira Rajeev
2024-05-07 4:39 ` [PATCH V2 0/9] Add data type profiling support for powerpc Namhyung Kim
2024-05-13 7:32 ` Athira Rajeev [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=C5802D81-BBBE-4409-8887-5DBB2269BCB5@linux.vnet.ibm.com \
--to=atrajeev@linux.vnet.ibm.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=akanksha@linux.ibm.com \
--cc=christophe.leroy@csgroup.eu \
--cc=disgoel@linux.vnet.ibm.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=kjain@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=maddy@linux.ibm.com \
--cc=namhyung@kernel.org \
--cc=segher@kernel.crashing.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 read-only IMAP folder(s) and NNTP newsgroup(s).