Re: [PATCH] perf/tests: fix record+probe_libc_inet_pton test on aarch64

[Leo Yan <leo.yan@arm.com>]

Hi Namhyung, Jing,

On Thu, Oct 17, 2024 at 03:12:36PM -0700, Namhyung Kim wrote:
> 
> Hello,
> 
> On Tue, Oct 15, 2024 at 03:53:27PM +0800, Jing Zhang wrote:
> > Since commit 1f85d016768f ("perf test record+probe_libc_inet_pton: Fix
> > call chain match on x86_64") remove function getaddrinfo() on expected
> > file, the test failed on aarch64. On aarch64, function getaddrinfo()
> > show up in the call chain.
> >
> > $perf script -i /tmp/perf.data.1PV
> > ping 2588319 [125] 500119.122843: probe_libc:inet_pton: (ffff9a4f7410)
> >             ffff9a4f7410 __GI___inet_pton+0x0 (/usr/lib64/libc-2.32.so)
> >             ffff9a4c5f7c getaddrinfo+0xec (/usr/lib64/libc-2.32.so)
> >             aaaad6d32b38 [unknown] (/usr/bin/ping)
> 
> I'm curious how other ARM folks don't see this.  Does it depend on
> something other?

This test has several dependencies. E.g. libtraceevent and IPv6 (one
of my machines skips the test due to lack IPv6 interface).

This test is definitely a regression on Arm64 machine. I will go back to
check why this test failure is missed in our CI. If have any update,
will let you know.

> Then can we make the line optional like we did on s390 recently?

I verified two distros (one is libc-2.31 and another is libc-2.37), both of
them have the symbol "getaddrinfo":

Machine 1:

  # nm -D /usr/lib/aarch64-linux-gnu/libc-2.31.so | grep getaddrinfo
    00000000000bdf58 T getaddrinfo

Machine 2:

  # /usr/lib/aarch64-linux-gnu/libc.so.6
  GNU C Library (Debian GLIBC 2.37-16) stable release version 2.37.

  # nm -D /usr/lib/aarch64-linux-gnu/libc.so.6 | grep getaddrinfo
  00000000000d4da4 T getaddrinfo@@GLIBC_2.17
  0000000000114260 T getaddrinfo_a@@GLIBC_2.34
  0000000000114260 T getaddrinfo_a@GLIBC_2.17

I don't see the need for making the line optional. And I don't see any
requirement for adding conditional check for symbol "gaih_inet".

> > So just remove getaddrinfo() on x86_64.
> 
> I'm not sure how it works on other archs.

I have no idea neither. Maybe this patch is fine - it just rolls back to
old format for rest archs, before if the test can work well on them, then
this patch just makes it to work again.

However, this patch has a syntax error. Please see below.

> > Fixes: 1f85d016768f ("perf test record+probe_libc_inet_pton: Fix call chain match on x86_64")
> > Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com>
> > ---
> >  tools/perf/tests/shell/record+probe_libc_inet_pton.sh | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
> > index 47a26f2..09d7b0b 100755
> > --- a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
> > +++ b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
> > @@ -52,8 +52,12 @@ trace_libc_inet_pton_backtrace() {
> >               echo "getaddrinfo\+0x[[:xdigit:]]+[[:space:]]\($libc\)$" >> $expected
> >               echo ".*(\+0x[[:xdigit:]]+|\[unknown\])[[:space:]]\(.*/bin/ping.*\)$" >> $expected
> >               ;;
> > +     x86_64)
> > +             eventattr='max-stack=3'
> > +             echo ".*(\+0x[[:xdigit:]]+|\[unknown\])[[:space:]]\(.*/bin/ping.*\)$" >> $expected

Please add ";;" at here to break switch, otherwise, it reports syntax
error.

With this change:

Tested-by: Leo Yan <leo.yan@arm.com>

Thanks for the fixing!

> >       *)
> >               eventattr='max-stack=3'
> > +             echo "getaddrinfo\+0x[[:xdigit:]]+[[:space:]]\($libc\)$" >> $expected
> >               echo ".*(\+0x[[:xdigit:]]+|\[unknown\])[[:space:]]\(.*/bin/ping.*\)$" >> $expected
> >               ;;
> >       esac
> > --
> > 1.8.3.1
> >
>