Received-SPF: pass (google.com: domain of linux-kernel+bounces-157802-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <20240424001231.849972-1-namhyung@kernel.org> <CAP-5=fWNWiuiLOKyNHYcuT9xmJkciUBodjCX1E_TRrTMMsM4uw@mail.gmail.com>
 <ZilzYDlzj7c1mQq4@x1>
In-Reply-To: <ZilzYDlzj7c1mQq4@x1>
From: Namhyung Kim <namhyung@kernel.org>
Date: Wed, 24 Apr 2024 15:48:37 -0700
Message-ID: <CAM9d7cg0bKF7+BtmkFxnHtiUZcgCWNG7eMqDccrZnM92G78vdQ@mail.gmail.com>
Subject: Re: [PATCH] perf test: Add a new test for perf annotate
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Ian Rogers <irogers@google.com>, Kan Liang <kan.liang@linux.intel.com>, 
	Jiri Olsa <jolsa@kernel.org>, Adrian Hunter <adrian.hunter@intel.com>, 
	Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@kernel.org>, 
	LKML <linux-kernel@vger.kernel.org>, linux-perf-users@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Apr 24, 2024 at 2:02=E2=80=AFPM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Wed, Apr 24, 2024 at 11:09:48AM -0700, Ian Rogers wrote:
> > On Tue, Apr 23, 2024 at 5:12=E2=80=AFPM Namhyung Kim <namhyung@kernel.o=
rg> wrote:
> > >
> > > Add a basic perf annotate test
> > >
> > >   $ ./perf test annotate -vv
> > >    76: perf annotate basic tests:
> > >   --- start ---
> > >   test child forked, pid 846989
> > >    fbcd0-fbd55 l noploop
> > >   perf does have symbol 'noploop'
> > >   Basic perf annotate test
> > >            : 0     0xfbcd0 <noploop>:
> > >       0.00 :   fbcd0:       pushq   %rbp
> > >       0.00 :   fbcd1:       movq    %rsp, %rbp
> > >       0.00 :   fbcd4:       pushq   %r12
> > >       0.00 :   fbcd6:       pushq   %rbx
> > >       0.00 :   fbcd7:       movl    $1, %ebx
> > >       0.00 :   fbcdc:       subq    $0x10, %rsp
> > >       0.00 :   fbce0:       movq    %fs:0x28, %rax
> > >       0.00 :   fbce9:       movq    %rax, -0x18(%rbp)
> > >       0.00 :   fbced:       xorl    %eax, %eax
> > >       0.00 :   fbcef:       testl   %edi, %edi
> > >       0.00 :   fbcf1:       jle     0xfbd04
> > >       0.00 :   fbcf3:       movq    (%rsi), %rdi
> > >       0.00 :   fbcf6:       movl    $0xa, %edx
> > >       0.00 :   fbcfb:       xorl    %esi, %esi
> > >       0.00 :   fbcfd:       callq   0x41920
> > >       0.00 :   fbd02:       movl    %eax, %ebx
> > >       0.00 :   fbd04:       leaq    -0x7b(%rip), %r12   # fbc90 <sigh=
andler>
> > >       0.00 :   fbd0b:       movl    $2, %edi
> > >       0.00 :   fbd10:       movq    %r12, %rsi
> > >       0.00 :   fbd13:       callq   0x40a00
> > >       0.00 :   fbd18:       movl    $0xe, %edi
> > >       0.00 :   fbd1d:       movq    %r12, %rsi
> > >       0.00 :   fbd20:       callq   0x40a00
> > >       0.00 :   fbd25:       movl    %ebx, %edi
> > >       0.00 :   fbd27:       callq   0x407c0
> > >       0.10 :   fbd2c:       movl    0x89785e(%rip), %eax        # 993=
590 <done>
> > >       0.00 :   fbd32:       testl   %eax, %eax
> > >      99.90 :   fbd34:       je      0xfbd2c
> > >       0.00 :   fbd36:       movq    -0x18(%rbp), %rax
> > >       0.00 :   fbd3a:       subq    %fs:0x28, %rax
> > >       0.00 :   fbd43:       jne     0xfbd50
> > >       0.00 :   fbd45:       addq    $0x10, %rsp
> > >       0.00 :   fbd49:       xorl    %eax, %eax
> > >       0.00 :   fbd4b:       popq    %rbx
> > >       0.00 :   fbd4c:       popq    %r12
> > >       0.00 :   fbd4e:       popq    %rbp
> > >       0.00 :   fbd4f:       retq
> > >       0.00 :   fbd50:       callq   0x407e0
> > >       0.00 :   fbcd0:       pushq   %rbp
> > >       0.00 :   fbcd1:       movq    %rsp, %rbp
> > >       0.00 :   fbcd4:       pushq   %r12
> > >       0.00 :   fbcd0:  push   %rbp
> > >       0.00 :   fbcd1:  mov    %rsp,%rbp
> > >       0.00 :   fbcd4:  push   %r12
> > >   Basic annotate test [Success]
> > >   ---- end(0) ----
> > >    76: perf annotate basic tests                                     =
  : Ok
> > >
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >
> > Looks good, thanks for this!
> >
> > Reviewed-by: Ian Rogers <irogers@google.com>
>
> Thanks! Applied, with the following changes to improve the error
> reporting, please holler if you disagree:

LGTM!

Thanks,
Namhyung

>
> diff --git a/tools/perf/tests/shell/annotate.sh b/tools/perf/tests/shell/=
annotate.sh
> index 7820d13eebaef535..1db1e8113d9943a6 100755
> --- a/tools/perf/tests/shell/annotate.sh
> +++ b/tools/perf/tests/shell/annotate.sh
> @@ -36,7 +36,7 @@ test_basic() {
>    echo "Basic perf annotate test"
>    if ! perf record -o "${perfdata}" ${testprog} 2> /dev/null
>    then
> -    echo "Basic annotate [Failed record]"
> +    echo "Basic annotate [Failed: perf record]"
>      err=3D1
>      return
>    fi
> @@ -44,7 +44,7 @@ test_basic() {
>    # check if it has the target symbol
>    if ! perf annotate -i "${perfdata}" 2> /dev/null | grep "${testsym}"
>    then
> -    echo "Basic annotate [Failed missing symbol]"
> +    echo "Basic annotate [Failed: missing target symbol]"
>      err=3D1
>      return
>    fi
> @@ -52,7 +52,7 @@ test_basic() {
>    # check if it has the disassembly lines
>    if ! perf annotate -i "${perfdata}" 2> /dev/null | grep "${disasm_rege=
x}"
>    then
> -    echo "Basic annotate [Failed missing disasm output]"
> +    echo "Basic annotate [Failed: missing disasm output from default dis=
assembler]"
>      err=3D1
>      return
>    fi
> @@ -61,7 +61,7 @@ test_basic() {
>    if ! perf annotate -i "${perfdata}" "${testsym}" 2> /dev/null | \
>           grep -m 3 "${disasm_regex}"
>    then
> -    echo "Basic annotate [Failed missing disasm output]"
> +    echo "Basic annotate [Failed: missing disasm output when specifying =
the target symbol]"
>      err=3D1
>      return
>    fi
> @@ -70,7 +70,7 @@ test_basic() {
>    if ! perf annotate -i "${perfdata}" --objdump=3Dobjdump 2> /dev/null |=
 \
>           grep -m 3 "${disasm_regex}"
>    then
> -    echo "Basic annotate [Failed missing disasm output from objdump]"
> +    echo "Basic annotate [Failed: missing disasm output from non default=
 disassembler (using --objdump)]"
>      err=3D1
>      return
>    fi