[PATCHSET 0/7] perf annotate: Add --code-with-type option

Namhyung Kim posted 7 patches 11 months, 1 week ago
There is a newer version of this series
tools/perf/Documentation/perf-annotate.txt |   4 +
tools/perf/builtin-annotate.c              |  19 +-
tools/perf/builtin-top.c                   |   2 +-
tools/perf/util/annotate-data.c            |  34 +++
tools/perf/util/annotate-data.h            |   3 +
tools/perf/util/annotate.c                 | 267 +++++++++++++--------
tools/perf/util/annotate.h                 |   8 +-
tools/perf/util/sort.c                     |  39 +--
8 files changed, 236 insertions(+), 140 deletions(-)
[PATCHSET 0/7] perf annotate: Add --code-with-type option
Posted by Namhyung Kim 11 months, 1 week ago
Hello,

There are roughly two modes in the perf annotate.  One is normal code
anootation and the other is data type annotation.  I'm proposing a new
way to combine the code and data annotation together.

With this option, "# data-type: <name> [offset (field)]" part will be
adeded when it found a data type for the given instruction.  This is
for every instruction in the function regardless whether it has a
sample or not.

This will be useful to understand function level data access patterns.
Currently it only works with --stdio, but I can add it to TUI later.

Here's an example output.

  $ perf annotate --code-with-type --stdio
  ...
 Percent |      Source code & Disassembly of elf for cpu/mem-loads,ldlat=30/P (4 samples, percent: local period)
  ----------------------------------------------------------------------------------------------------------------
           : 0                0xffffffff81bb7060 <__schedule>:
      0.00 :   ffffffff81bb7060:        pushq   %rbp              # data-type: (stack operation)
      0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
      0.00 :   ffffffff81bb7064:        pushq   %r15              # data-type: (stack operation)
      0.00 :   ffffffff81bb7066:        pushq   %r14              # data-type: (stack operation)
      0.00 :   ffffffff81bb7068:        pushq   %r13              # data-type: (stack operation)
      0.00 :   ffffffff81bb706a:        pushq   %r12              # data-type: (stack operation)
      0.00 :   ffffffff81bb706c:        pushq   %rbx              # data-type: (stack operation)
      0.00 :   ffffffff81bb706d:        movq    $0x33180, %rbx
      0.00 :   ffffffff81bb7074:        movq    %rbx, %r12
      0.00 :   ffffffff81bb7077:        subq    $0x38, %rsp
      0.00 :   ffffffff81bb707b:        movl    %edi, -0x5c(%rbp)         # data-type: unsigned int +0
      0.00 :   ffffffff81bb707e:        movq    %gs:0x28, %rax            # data-type: (stack canary)
      0.00 :   ffffffff81bb7087:        movq    %rax, -0x30(%rbp)         # data-type: (stack canary)
      0.00 :   ffffffff81bb708b:        xorl    %eax, %eax
      0.00 :   ffffffff81bb708d:        movq    $0x0, -0x58(%rbp)         # data-type: struct rq_flags +0 (flags)
      0.00 :   ffffffff81bb7095:        movq    $0x0, -0x50(%rbp)         # data-type: struct rq_flags +0x8 (clock_update_flags)
      0.00 :   ffffffff81bb709d:        callq   0xffffffff81baa100 <debug_smp_processor_id>               # data-type: (stack operation)
      0.00 :   ffffffff81bb70a2:        cltq
      0.00 :   ffffffff81bb70a4:        addq    -0x7dcf0500(,%rax,8), %r12                # data-type: long unsigned int[] +0
      0.00 :   ffffffff81bb70ac:        movq    0x960(%r12), %r13         # data-type: struct rq +0x960 (curr)
      0.00 :   ffffffff81bb70b4:        movq    0x20(%r13), %rax          # data-type: struct task_struct +0x20 (stack)
      0.00 :   ffffffff81bb70b8:        cmpq    $0x57ac6e9d, (%rax)
      0.00 :   ffffffff81bb70bf:        jne     0xffffffff81bb7bca <__schedule+0xb6a>
      0.00 :   ffffffff81bb70c5:        movl    %gs:0x7e47b87c(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)
      0.00 :   ffffffff81bb70cc:        andl    $0x7fffffff, %eax
      0.00 :   ffffffff81bb70d1:        cmpl    $0x1, %eax
      0.00 :   ffffffff81bb70d4:        jne     0xffffffff81bb79de <__schedule+0x97e>
      0.00 :   ffffffff81bb70da:        nopl    (%rax,%rax)
      0.00 :   ffffffff81bb70df:        cmpl    $0x2, 0xe86b3a(%rip)  # 0xffffffff82a3dc20 <prof_on>              # data-type: int +0
      0.00 :   ffffffff81bb70e6:        movq    0x8(%rbp), %rsi
      0.00 :   ffffffff81bb70ea:        je      0xffffffff81bb7a0b <__schedule+0x9ab>
      0.00 :   ffffffff81bb70f0:        nopl    (%rax,%rax)
      0.00 :   ffffffff81bb70f5:        nop
      ...

The code is also available at 'perf/annotate-code-data-v1' branch in
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung


Namhyung Kim (7):
  perf annotate-data: Add annotated_data_type__get_member_name()
  perf annotate: Remove unused len parameter from
    annotation_line__print()
  perf annotate: Pass annotation_options to annotation_line__print()
  perf annotate: Pass hist_entry to annotate functions
  perf annotate: Factor out __hist_entry__get_data_type()
  perf annotate: Implement code + data type annotation
  perf annotate: Add --code-with-type option.

 tools/perf/Documentation/perf-annotate.txt |   4 +
 tools/perf/builtin-annotate.c              |  19 +-
 tools/perf/builtin-top.c                   |   2 +-
 tools/perf/util/annotate-data.c            |  34 +++
 tools/perf/util/annotate-data.h            |   3 +
 tools/perf/util/annotate.c                 | 267 +++++++++++++--------
 tools/perf/util/annotate.h                 |   8 +-
 tools/perf/util/sort.c                     |  39 +--
 8 files changed, 236 insertions(+), 140 deletions(-)

-- 
2.48.1.711.g2feabab25a-goog
Re: [PATCHSET 0/7] perf annotate: Add --code-with-type option
Posted by Ian Rogers 11 months, 1 week ago
On Mon, Mar 3, 2025 at 9:38 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> There are roughly two modes in the perf annotate.  One is normal code
> anootation and the other is data type annotation.  I'm proposing a new

nit: s/anootation/annotation/

> way to combine the code and data annotation together.
>
> With this option, "# data-type: <name> [offset (field)]" part will be
> adeded when it found a data type for the given instruction.  This is

nit: s/adeded/added/

> for every instruction in the function regardless whether it has a
> sample or not.
>
> This will be useful to understand function level data access patterns.
> Currently it only works with --stdio, but I can add it to TUI later.
>
> Here's an example output.

At some point I think we might as well have our own disassembler. The
annotations remind me of the relocation and other annotation output of
objdump.

>   $ perf annotate --code-with-type --stdio
>   ...
>  Percent |      Source code & Disassembly of elf for cpu/mem-loads,ldlat=30/P (4 samples, percent: local period)
>   ----------------------------------------------------------------------------------------------------------------
>            : 0                0xffffffff81bb7060 <__schedule>:
>       0.00 :   ffffffff81bb7060:        pushq   %rbp              # data-type: (stack operation)
>       0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
>       0.00 :   ffffffff81bb7064:        pushq   %r15              # data-type: (stack operation)
>       0.00 :   ffffffff81bb7066:        pushq   %r14              # data-type: (stack operation)
>       0.00 :   ffffffff81bb7068:        pushq   %r13              # data-type: (stack operation)
>       0.00 :   ffffffff81bb706a:        pushq   %r12              # data-type: (stack operation)
>       0.00 :   ffffffff81bb706c:        pushq   %rbx              # data-type: (stack operation)

data-type: looks a bit weird here. I wonder if the
.cfi_offset/.cfi_restore could  make this information richer, maybe:
       0.00 :   ffffffff81bb7060:        pushq   %rbp              #
*(frame-16) = caller rbp
       0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
       0.00 :   ffffffff81bb7064:        pushq   %r15              #
*(frame-24) = caller r15

>       0.00 :   ffffffff81bb706d:        movq    $0x33180, %rbx
>       0.00 :   ffffffff81bb7074:        movq    %rbx, %r12
>       0.00 :   ffffffff81bb7077:        subq    $0x38, %rsp
>       0.00 :   ffffffff81bb707b:        movl    %edi, -0x5c(%rbp)         # data-type: unsigned int +0
>       0.00 :   ffffffff81bb707e:        movq    %gs:0x28, %rax            # data-type: (stack canary)
>       0.00 :   ffffffff81bb7087:        movq    %rax, -0x30(%rbp)         # data-type: (stack canary)
>       0.00 :   ffffffff81bb708b:        xorl    %eax, %eax
>       0.00 :   ffffffff81bb708d:        movq    $0x0, -0x58(%rbp)         # data-type: struct rq_flags +0 (flags)
>       0.00 :   ffffffff81bb7095:        movq    $0x0, -0x50(%rbp)         # data-type: struct rq_flags +0x8 (clock_update_flags)
>       0.00 :   ffffffff81bb709d:        callq   0xffffffff81baa100 <debug_smp_processor_id>               # data-type: (stack operation)
>       0.00 :   ffffffff81bb70a2:        cltq
>       0.00 :   ffffffff81bb70a4:        addq    -0x7dcf0500(,%rax,8), %r12                # data-type: long unsigned int[] +0
>       0.00 :   ffffffff81bb70ac:        movq    0x960(%r12), %r13         # data-type: struct rq +0x960 (curr)
>       0.00 :   ffffffff81bb70b4:        movq    0x20(%r13), %rax          # data-type: struct task_struct +0x20 (stack)
>       0.00 :   ffffffff81bb70b8:        cmpq    $0x57ac6e9d, (%rax)
>       0.00 :   ffffffff81bb70bf:        jne     0xffffffff81bb7bca <__schedule+0xb6a>
>       0.00 :   ffffffff81bb70c5:        movl    %gs:0x7e47b87c(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)

The variable name in the struct is really nice, it is a shame the same
information gets repeated a few times on the line.

Thanks,
Ian

>       0.00 :   ffffffff81bb70cc:        andl    $0x7fffffff, %eax
>       0.00 :   ffffffff81bb70d1:        cmpl    $0x1, %eax
>       0.00 :   ffffffff81bb70d4:        jne     0xffffffff81bb79de <__schedule+0x97e>
>       0.00 :   ffffffff81bb70da:        nopl    (%rax,%rax)
>       0.00 :   ffffffff81bb70df:        cmpl    $0x2, 0xe86b3a(%rip)  # 0xffffffff82a3dc20 <prof_on>              # data-type: int +0
>       0.00 :   ffffffff81bb70e6:        movq    0x8(%rbp), %rsi
>       0.00 :   ffffffff81bb70ea:        je      0xffffffff81bb7a0b <__schedule+0x9ab>
>       0.00 :   ffffffff81bb70f0:        nopl    (%rax,%rax)
>       0.00 :   ffffffff81bb70f5:        nop
>       ...
>
> The code is also available at 'perf/annotate-code-data-v1' branch in
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Thanks,
> Namhyung
>
>
> Namhyung Kim (7):
>   perf annotate-data: Add annotated_data_type__get_member_name()
>   perf annotate: Remove unused len parameter from
>     annotation_line__print()
>   perf annotate: Pass annotation_options to annotation_line__print()
>   perf annotate: Pass hist_entry to annotate functions
>   perf annotate: Factor out __hist_entry__get_data_type()
>   perf annotate: Implement code + data type annotation
>   perf annotate: Add --code-with-type option.
>
>  tools/perf/Documentation/perf-annotate.txt |   4 +
>  tools/perf/builtin-annotate.c              |  19 +-
>  tools/perf/builtin-top.c                   |   2 +-
>  tools/perf/util/annotate-data.c            |  34 +++
>  tools/perf/util/annotate-data.h            |   3 +
>  tools/perf/util/annotate.c                 | 267 +++++++++++++--------
>  tools/perf/util/annotate.h                 |   8 +-
>  tools/perf/util/sort.c                     |  39 +--
>  8 files changed, 236 insertions(+), 140 deletions(-)
>
> --
> 2.48.1.711.g2feabab25a-goog
>
Re: [PATCHSET 0/7] perf annotate: Add --code-with-type option
Posted by Ian Rogers 11 months, 1 week ago
On Wed, Mar 5, 2025 at 3:27 PM Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Mar 3, 2025 at 9:38 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hello,
> >
> > There are roughly two modes in the perf annotate.  One is normal code
> > anootation and the other is data type annotation.  I'm proposing a new
>
> nit: s/anootation/annotation/
>
> > way to combine the code and data annotation together.
> >
> > With this option, "# data-type: <name> [offset (field)]" part will be
> > adeded when it found a data type for the given instruction.  This is
>
> nit: s/adeded/added/
>
> > for every instruction in the function regardless whether it has a
> > sample or not.
> >
> > This will be useful to understand function level data access patterns.
> > Currently it only works with --stdio, but I can add it to TUI later.
> >
> > Here's an example output.
>
> At some point I think we might as well have our own disassembler. The
> annotations remind me of the relocation and other annotation output of
> objdump.
>
> >   $ perf annotate --code-with-type --stdio
> >   ...
> >  Percent |      Source code & Disassembly of elf for cpu/mem-loads,ldlat=30/P (4 samples, percent: local period)
> >   ----------------------------------------------------------------------------------------------------------------
> >            : 0                0xffffffff81bb7060 <__schedule>:
> >       0.00 :   ffffffff81bb7060:        pushq   %rbp              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
> >       0.00 :   ffffffff81bb7064:        pushq   %r15              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb7066:        pushq   %r14              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb7068:        pushq   %r13              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb706a:        pushq   %r12              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb706c:        pushq   %rbx              # data-type: (stack operation)
>
> data-type: looks a bit weird here. I wonder if the
> .cfi_offset/.cfi_restore could  make this information richer, maybe:
>        0.00 :   ffffffff81bb7060:        pushq   %rbp              #
> *(frame-16) = caller rbp
>        0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
>        0.00 :   ffffffff81bb7064:        pushq   %r15              #
> *(frame-24) = caller r15
>
> >       0.00 :   ffffffff81bb706d:        movq    $0x33180, %rbx
> >       0.00 :   ffffffff81bb7074:        movq    %rbx, %r12
> >       0.00 :   ffffffff81bb7077:        subq    $0x38, %rsp
> >       0.00 :   ffffffff81bb707b:        movl    %edi, -0x5c(%rbp)         # data-type: unsigned int +0
> >       0.00 :   ffffffff81bb707e:        movq    %gs:0x28, %rax            # data-type: (stack canary)
> >       0.00 :   ffffffff81bb7087:        movq    %rax, -0x30(%rbp)         # data-type: (stack canary)
> >       0.00 :   ffffffff81bb708b:        xorl    %eax, %eax
> >       0.00 :   ffffffff81bb708d:        movq    $0x0, -0x58(%rbp)         # data-type: struct rq_flags +0 (flags)
> >       0.00 :   ffffffff81bb7095:        movq    $0x0, -0x50(%rbp)         # data-type: struct rq_flags +0x8 (clock_update_flags)
> >       0.00 :   ffffffff81bb709d:        callq   0xffffffff81baa100 <debug_smp_processor_id>               # data-type: (stack operation)
> >       0.00 :   ffffffff81bb70a2:        cltq
> >       0.00 :   ffffffff81bb70a4:        addq    -0x7dcf0500(,%rax,8), %r12                # data-type: long unsigned int[] +0
> >       0.00 :   ffffffff81bb70ac:        movq    0x960(%r12), %r13         # data-type: struct rq +0x960 (curr)
> >       0.00 :   ffffffff81bb70b4:        movq    0x20(%r13), %rax          # data-type: struct task_struct +0x20 (stack)
> >       0.00 :   ffffffff81bb70b8:        cmpq    $0x57ac6e9d, (%rax)
> >       0.00 :   ffffffff81bb70bf:        jne     0xffffffff81bb7bca <__schedule+0xb6a>
> >       0.00 :   ffffffff81bb70c5:        movl    %gs:0x7e47b87c(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)
>
> The variable name in the struct is really nice, it is a shame the same
> information gets repeated a few times on the line.

Outside of the typos in the cover letter, the rest of the patches lgtm.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> >       0.00 :   ffffffff81bb70cc:        andl    $0x7fffffff, %eax
> >       0.00 :   ffffffff81bb70d1:        cmpl    $0x1, %eax
> >       0.00 :   ffffffff81bb70d4:        jne     0xffffffff81bb79de <__schedule+0x97e>
> >       0.00 :   ffffffff81bb70da:        nopl    (%rax,%rax)
> >       0.00 :   ffffffff81bb70df:        cmpl    $0x2, 0xe86b3a(%rip)  # 0xffffffff82a3dc20 <prof_on>              # data-type: int +0
> >       0.00 :   ffffffff81bb70e6:        movq    0x8(%rbp), %rsi
> >       0.00 :   ffffffff81bb70ea:        je      0xffffffff81bb7a0b <__schedule+0x9ab>
> >       0.00 :   ffffffff81bb70f0:        nopl    (%rax,%rax)
> >       0.00 :   ffffffff81bb70f5:        nop
> >       ...
> >
> > The code is also available at 'perf/annotate-code-data-v1' branch in
> > git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >
> > Thanks,
> > Namhyung
> >
> >
> > Namhyung Kim (7):
> >   perf annotate-data: Add annotated_data_type__get_member_name()
> >   perf annotate: Remove unused len parameter from
> >     annotation_line__print()
> >   perf annotate: Pass annotation_options to annotation_line__print()
> >   perf annotate: Pass hist_entry to annotate functions
> >   perf annotate: Factor out __hist_entry__get_data_type()
> >   perf annotate: Implement code + data type annotation
> >   perf annotate: Add --code-with-type option.
> >
> >  tools/perf/Documentation/perf-annotate.txt |   4 +
> >  tools/perf/builtin-annotate.c              |  19 +-
> >  tools/perf/builtin-top.c                   |   2 +-
> >  tools/perf/util/annotate-data.c            |  34 +++
> >  tools/perf/util/annotate-data.h            |   3 +
> >  tools/perf/util/annotate.c                 | 267 +++++++++++++--------
> >  tools/perf/util/annotate.h                 |   8 +-
> >  tools/perf/util/sort.c                     |  39 +--
> >  8 files changed, 236 insertions(+), 140 deletions(-)
> >
> > --
> > 2.48.1.711.g2feabab25a-goog
> >
Re: [PATCHSET 0/7] perf annotate: Add --code-with-type option
Posted by Namhyung Kim 11 months, 1 week ago
On Wed, Mar 05, 2025 at 03:31:45PM -0800, Ian Rogers wrote:
> On Wed, Mar 5, 2025 at 3:27 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Mon, Mar 3, 2025 at 9:38 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > Hello,
> > >
> > > There are roughly two modes in the perf annotate.  One is normal code
> > > anootation and the other is data type annotation.  I'm proposing a new
> >
> > nit: s/anootation/annotation/

Oops.

> >
> > > way to combine the code and data annotation together.
> > >
> > > With this option, "# data-type: <name> [offset (field)]" part will be
> > > adeded when it found a data type for the given instruction.  This is
> >
> > nit: s/adeded/added/

Thanks for pointing this out.

> >
> > > for every instruction in the function regardless whether it has a
> > > sample or not.
> > >
> > > This will be useful to understand function level data access patterns.
> > > Currently it only works with --stdio, but I can add it to TUI later.
> > >
> > > Here's an example output.
> >
> > At some point I think we might as well have our own disassembler. The
> > annotations remind me of the relocation and other annotation output of
> > objdump.
> >
> > >   $ perf annotate --code-with-type --stdio
> > >   ...
> > >  Percent |      Source code & Disassembly of elf for cpu/mem-loads,ldlat=30/P (4 samples, percent: local period)
> > >   ----------------------------------------------------------------------------------------------------------------
> > >            : 0                0xffffffff81bb7060 <__schedule>:
> > >       0.00 :   ffffffff81bb7060:        pushq   %rbp              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
> > >       0.00 :   ffffffff81bb7064:        pushq   %r15              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb7066:        pushq   %r14              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb7068:        pushq   %r13              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb706a:        pushq   %r12              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb706c:        pushq   %rbx              # data-type: (stack operation)
> >
> > data-type: looks a bit weird here. I wonder if the
> > .cfi_offset/.cfi_restore could  make this information richer, maybe:
> >        0.00 :   ffffffff81bb7060:        pushq   %rbp              # *(frame-16) = caller rbp
> >        0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
> >        0.00 :   ffffffff81bb7064:        pushq   %r15              # *(frame-24) = caller r15

Probably possible, but it's also obvious from the instruction.
I didn't try to be smart here as we're interested in types.

> >
> > >       0.00 :   ffffffff81bb706d:        movq    $0x33180, %rbx
> > >       0.00 :   ffffffff81bb7074:        movq    %rbx, %r12
> > >       0.00 :   ffffffff81bb7077:        subq    $0x38, %rsp
> > >       0.00 :   ffffffff81bb707b:        movl    %edi, -0x5c(%rbp)         # data-type: unsigned int +0
> > >       0.00 :   ffffffff81bb707e:        movq    %gs:0x28, %rax            # data-type: (stack canary)
> > >       0.00 :   ffffffff81bb7087:        movq    %rax, -0x30(%rbp)         # data-type: (stack canary)
> > >       0.00 :   ffffffff81bb708b:        xorl    %eax, %eax
> > >       0.00 :   ffffffff81bb708d:        movq    $0x0, -0x58(%rbp)         # data-type: struct rq_flags +0 (flags)
> > >       0.00 :   ffffffff81bb7095:        movq    $0x0, -0x50(%rbp)         # data-type: struct rq_flags +0x8 (clock_update_flags)
> > >       0.00 :   ffffffff81bb709d:        callq   0xffffffff81baa100 <debug_smp_processor_id>               # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb70a2:        cltq
> > >       0.00 :   ffffffff81bb70a4:        addq    -0x7dcf0500(,%rax,8), %r12                # data-type: long unsigned int[] +0
> > >       0.00 :   ffffffff81bb70ac:        movq    0x960(%r12), %r13         # data-type: struct rq +0x960 (curr)
> > >       0.00 :   ffffffff81bb70b4:        movq    0x20(%r13), %rax          # data-type: struct task_struct +0x20 (stack)
> > >       0.00 :   ffffffff81bb70b8:        cmpq    $0x57ac6e9d, (%rax)
> > >       0.00 :   ffffffff81bb70bf:        jne     0xffffffff81bb7bca <__schedule+0xb6a>
> > >       0.00 :   ffffffff81bb70c5:        movl    %gs:0x7e47b87c(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)
> >
> > The variable name in the struct is really nice, it is a shame the same
> > information gets repeated a few times on the line.

I guess you meant 'pcpu_hot'.  But the assembly shows the name of the
global variable and the data type shows the name of struct.  It happened
to be the same in this case.

> 
> Outside of the typos in the cover letter, the rest of the patches lgtm.
> 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks for your review!
Namhyung

> 
> > >       0.00 :   ffffffff81bb70cc:        andl    $0x7fffffff, %eax
> > >       0.00 :   ffffffff81bb70d1:        cmpl    $0x1, %eax
> > >       0.00 :   ffffffff81bb70d4:        jne     0xffffffff81bb79de <__schedule+0x97e>
> > >       0.00 :   ffffffff81bb70da:        nopl    (%rax,%rax)
> > >       0.00 :   ffffffff81bb70df:        cmpl    $0x2, 0xe86b3a(%rip)  # 0xffffffff82a3dc20 <prof_on>              # data-type: int +0
> > >       0.00 :   ffffffff81bb70e6:        movq    0x8(%rbp), %rsi
> > >       0.00 :   ffffffff81bb70ea:        je      0xffffffff81bb7a0b <__schedule+0x9ab>
> > >       0.00 :   ffffffff81bb70f0:        nopl    (%rax,%rax)
> > >       0.00 :   ffffffff81bb70f5:        nop
> > >       ...
> > >
> > > The code is also available at 'perf/annotate-code-data-v1' branch in
> > > git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > >
> > > Thanks,
> > > Namhyung
> > >
> > >
> > > Namhyung Kim (7):
> > >   perf annotate-data: Add annotated_data_type__get_member_name()
> > >   perf annotate: Remove unused len parameter from
> > >     annotation_line__print()
> > >   perf annotate: Pass annotation_options to annotation_line__print()
> > >   perf annotate: Pass hist_entry to annotate functions
> > >   perf annotate: Factor out __hist_entry__get_data_type()
> > >   perf annotate: Implement code + data type annotation
> > >   perf annotate: Add --code-with-type option.
> > >
> > >  tools/perf/Documentation/perf-annotate.txt |   4 +
> > >  tools/perf/builtin-annotate.c              |  19 +-
> > >  tools/perf/builtin-top.c                   |   2 +-
> > >  tools/perf/util/annotate-data.c            |  34 +++
> > >  tools/perf/util/annotate-data.h            |   3 +
> > >  tools/perf/util/annotate.c                 | 267 +++++++++++++--------
> > >  tools/perf/util/annotate.h                 |   8 +-
> > >  tools/perf/util/sort.c                     |  39 +--
> > >  8 files changed, 236 insertions(+), 140 deletions(-)
> > >
> > > --
> > > 2.48.1.711.g2feabab25a-goog
> > >