Re: [PATCH v5 07/12] perf annotate: Add --code-with-type support for TUI

[Namhyung Kim <namhyung@kernel.org>]

Hi Arnaldo,

On Wed, Aug 20, 2025 at 03:54:49PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 15, 2025 at 08:16:30PM -0700, Namhyung Kim wrote:
> > Until now, the --code-with-type option is available only on stdio.
> > But it was an artifical limitation because of an implemention issue.
> > 
> > Implement the same logic in annotation_line__write() for stdio2/TUI
> > and remove the limitation and update the man page.
> 
> Samples: 54K of event 'cycles:P', 4000 Hz, Event count (approx.): 25749717559
> poll_idle  /usr/lib/debug/lib/modules/6.15.9-201.fc42.x86_64/vmlinux [Percent: local period]
> Percent │      mov     %rdi,%rbx                                                                                                                                        ▒
>         │    → call    local_clock_noinstr              # data-type: (stack operation)                                                                                  ▒
>         │      andb    $0xfb,(%rbx)             # data-type: struct cpuidle_device +0 (registered:1)                                                                    ▒
>         │      mov     %rax,%rbp                                                                                                                                        ▒
>         │    → call    *0x1342a22(%rip)        # ffffffff8384f778 <pv_ops+0xf8>         # data-type: (stack operation)                                                  ▒
>         │      mov     current_task,%r14                # data-type: struct task_struct* +0                                                                             ▒
>         │      lock    orb     $0x20,0x2(%r14)          # data-type: struct task_struct +0x2 (thread_info.flags)                                                        ▒
>         │      mov     (%r14),%rax              # data-type: struct task_struct +0 (thread_info.flags)                                                                  ▒
>         │      test    $0x8,%al                                                                                                                                         ▒
>         │    ↓ jne     6d                                                                                                                                               ▒
>         │      mov     %r12,%rdi                                                                                                                                        ▒
>         │      mov     %rbx,%rsi                                                                                                                                        ▒
>         │    → call    cpuidle_poll_time                # data-type: (stack operation)                                                                                  ▒
>         │      mov     %rax,%r12                                                                                                                                        ▒
>         │49:   mov     $0xc9,%eax                                                                                                                                       ▒
>    0.75 │4e:   mov     (%r14),%rdx              # data-type: struct task_struct +0 (thread_info.flags)                                                                  ▒
>    0.42 │      and     $0x8,%edx                                                                                                                                        ▒
>         │    ↓ jne     6d                                                                                                                                               ▒
>   97.81 │      pause                                                                                                                                                    ▒
>    0.70 │      sub     $0x1,%eax                                                                                                                                        ▒
>    0.04 │    ↑ jne     4e                                                                                                                                               ▒
>    0.14 │    → call    local_clock_noinstr              # data-type: (stack operation)                                                                                  ▒
>    0.01 │      sub     %rbp,%rax                                                                                                                                        ▒
>    0.05 │      cmp     %rax,%r12                                                                                                                                        ▒
>         │    ↑ jae     49                                                                                                                                               ▒
>         │      orb     $0x4,(%rbx)              # data-type: struct cpuidle_device +0 (registered:1)                                                                    ▒
>         │6d: → call    *0x13429cd(%rip)        # ffffffff8384f770 <pv_ops+0xf0>         # data-type: (stack operation)                                                  ▒
>         │      lock    andb    $0xdf,0x2(%r14)          # data-type: struct task_struct +0x2 (thread_info.flags)                                                        ▒
>         │      mov     (%r14),%rax              # data-type: struct task_struct +0 (thread_info.flags) 
> 
> 
> Some suggestions:
> 
> Align the # data-type: annotations.

I was thinking about this but then probably it needs to align to the
longest line like call instruction.  It can be annoying if you are
using a small terminal.

> 
> Remove the "data-type: " part, just uses space and it should be obvious
> what that type refers to.

I think it's better to have some kind of signature to find the info
deterministically as I need to extract them in a script.

> 
> At some point, if we can do it super cheaply, maybe BTF, do this by
> default.

BTF doesn't have variables and locations so it's impossible to match
instructions to types currently.

> 
> The 'T' hotkey looks great, but perhaps we should have more visibility
> for it? Like what I suggested in:
> 
> https://lore.kernel.org/all/aBvx-J-ISifmw0NS@google.com/T/#u

Yep, in the next patch. :)

Thanks,
Namhyung