Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 20 Jun 2018 19:45:46 -0500
From:   Kim Phillips <kim.phillips@arm.com>
To:     Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     linux-kernel@vger.kernel.org,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Thomas Richter <tmricht@linux.vnet.ibm.com>,
        Michael Petlan <mpetlan@redhat.com>,
        Hendrik =?ISO-8859-1?Q?Br=FCckner?= 
        <brueckner@linux.vnet.ibm.com>,
        Sandipan Das <sandipan@linux.vnet.ibm.com>
Subject: Re: [PATCH 2/2] perf test shell: make perf inet_pton test more
 portable
Message-Id: <20180620194546.364cc109430ac51c5a288739@arm.com>
In-Reply-To: <20180620134622.GM20477@kernel.org>
References: <20180619184952.dac6a3a1e39d3b1d255e7119@arm.com>
        <20180620134622.GM20477@kernel.org>
Organization: Arm
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 20 Jun 2018 10:46:22 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Tue, Jun 19, 2018 at 06:49:52PM -0500, Kim Phillips escreveu:
> > Debian based systems such as Ubuntu have dash as their default shell.
> > Even if the normal or root user's shell is bash, certain scripts still
> > call /bin/sh, which points to dash, so we fix this perf test by
> > rewriting it in a more portable way.
> 
> Isn't it better to just make /bin/bash a requirement for these tests?

Perf is more bug-prone in distributions other than its main developers'
distributions, and when its own built-in tests start depending on those
same (primary) distributions' preferences, tests start to get skipped
on the secondary ones, which start to get subsequently ignored and
become acceptable to testers, which is a whole pattern I'd like to
avoid if at all possible.  I'd eventually like to see the real perf run
on Android, for example, and adding bash to Android is nontrivial,
AFAICT.

> I think the alternative is cryptic :-\

I think that's because of the fake array stuff, which technically isn't
needed by design.  How about something like the following?

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 263057039693..d5cceaeba42d 100755
--- a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
@@ -14,20 +14,21 @@ libc=$(grep -w libc /proc/self/maps | head -1 | sed -r 's/.*[[:space:]](\/.*)/\1
 nm -Dg $libc 2>/dev/null | fgrep -q inet_pton || exit 254
 
 trace_libc_inet_pton_backtrace() {
+       newline='\n'
        idx=0
-       expected[0]="ping[][0-9 \.:]+probe_libc:inet_pton: \([[:xdigit:]]+\)"
-       expected[1]=".*inet_pton\+0x[[:xdigit:]]+[[:space:]]\($libc|inlined\)$"
+       expected="ping[][0-9 \.:]+probe_libc:inet_pton: \([[:xdigit:]]+\)"
+       expected="${expected}${newline}.*inet_pton\+0x[[:xdigit:]]+[[:space:]]\($libc|inlined\)$"
        case "$(uname -m)" in
        s390x)
                eventattr='call-graph=dwarf,max-stack=4'
-               expected[2]="gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\($libc|inlined\)$"
-               expected[3]="(__GI_)?getaddrinfo\+0x[[:xdigit:]]+[[:space:]]\($libc|inlined\)$"
-               expected[4]="main\+0x[[:xdigit:]]+[[:space:]]\(.*/bin/ping.*\)$"
+               expected="${expected}${newline}gaih_inet.*\+0x[[:xdigit:]]+[[:space:]]\($libc|inlined\)$"
+               expected="${expected}${newline}(__GI_)?getaddrinfo\+0x[[:xdigit:]]+[[:space:]]\($libc|inlined\)$"
+               expected="${expected}${newline}main\+0x[[:xdigit:]]+[[:space:]]\(.*/bin/ping.*\)$"
                ;;
        *)
                eventattr='max-stack=3'
-               expected[2]="getaddrinfo\+0x[[:xdigit:]]+[[:space:]]\($libc\)$"
-               expected[3]=".*\+0x[[:xdigit:]]+[[:space:]]\(.*/bin/ping.*\)$"
+               expected="${expected}${newline}getaddrinfo\+0x[[:xdigit:]]+[[:space:]]\($libc\)$"
+               expected="${expected}${newline}.*\+0x[[:xdigit:]]+[[:space:]]\(.*/bin/ping.*\)$"
                ;;
        esac
 
@@ -35,14 +36,16 @@ trace_libc_inet_pton_backtrace() {
 
        perf record -e probe_libc:inet_pton/$eventattr/ -o $file ping -6 -c 1 ::1 > /dev/null 2>&1
        perf script -i $file | while read line ; do
+               [ -z "${line}" ] && break
                echo $line
-               echo "$line" | egrep -q "${expected[$idx]}"
+               idx=$((idx + 1))
+               first="$(echo ${expected} | head -$idx | tail -1)"
+               [ -z "${first}" ] && break
+               echo "$line" | egrep -q "$first"
                if [ $? -ne 0 ] ; then
-                       printf "FAIL: expected backtrace entry %d \"%s\" got \"%s\"\n" $idx "${expected[$idx]}" "$line"
+                       printf "FAIL: expected backtrace entry %d \"%s\" got \"%s\"\n" $idx "${first}" "$line"
                        exit 1
                fi
-               let idx+=1
-               [ -z "${expected[$idx]}" ] && break
        done
 
        # If any statements are executed from this point onwards,

Thanks,

Kim