From: Ian Rogers <irogers@google.com>
To: Thomas Richter <tmricht@linux.ibm.com>
Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
acme@kernel.org, Jiri Olsa <jolsa@kernel.org>,
Namhyung Kim <namhyung@kernel.org>,
svens@linux.ibm.com, gor@linux.ibm.com, sumanthk@linux.ibm.com,
hca@linux.ibm.com
Subject: Re: [PATCH] perf test: Add skip to --per-thread test
Date: Mon, 2 May 2022 21:33:05 -0700 [thread overview]
Message-ID: <CAP-5=fU8+QOeZjL=DMgwwQeJiKqnKw28eF=0m1kmp8C7Mwfz+Q@mail.gmail.com> (raw)
In-Reply-To: <d36e7dc7-e063-365f-2bf4-8cac590b5802@linux.ibm.com>
On Mon, May 2, 2022 at 2:39 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>
> On 4/29/22 08:13, Ian Rogers wrote:
> > As reported in:
> > https://lore.kernel.org/linux-perf-users/20220428122821.3652015-1-tmricht@linux.ibm.com/
> > the 'instructions:u' event may not be supported. Add a skip using 'perf
> > stat'.
> >
> > Switch some code away from pipe to make the failures clearer.
> >
> > Reported-by: Thomas Richter <tmricht@linux.ibm.com>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/tests/shell/record.sh | 44 +++++++++++++++++++++++++++++---
> > 1 file changed, 41 insertions(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> > index cd1cf14259b8..efc0cb07a929 100755
> > --- a/tools/perf/tests/shell/record.sh
> > +++ b/tools/perf/tests/shell/record.sh
> > @@ -5,11 +5,43 @@
> > set -e
> >
> > err=0
> > +perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> > +
> > +cleanup() {
> > + rm -f ${perfdata}
> > + rm -f ${perfdata}.old
> > + trap - exit term int
> > +}
> > +
> > +trap_cleanup() {
> > + cleanup
> > + exit 1
> > +}
> > +trap trap_cleanup exit term int
> > +
> > test_per_thread() {
> > echo "Basic --per-thread mode test"
> > - perf record -e instructions:u --per-thread -o- true 2> /dev/null \
> > - | perf report -i- -q \
> > - | egrep -q true
> > + if ! perf stat -e instructions:u --quiet true
> > + then
> > + echo "Per-thread record [Skipped instructions:u not supported]"
> > + if [ $err -ne 1 ]
> > + then
> > + err=2
> > + fi
> > + return
> > + fi
> > + if ! perf record -e instructions:u --per-thread -o ${perfdata} true 2> /dev/null
> > + then
> > + echo "Per-thread record of instructions:u [Failed]"
> > + err=1
> > + return
> > + fi
> > + if ! perf report -i ${perfdata} -q | egrep -q true
> > + then
> > + echo "Per-thread record [Failed missing output]"
> > + err=1
> > + return
> > + fi
> > echo "Basic --per-thread mode test [Success]"
> > }
> >
> > @@ -18,6 +50,10 @@ test_register_capture() {
> > if ! perf list | egrep -q 'br_inst_retired.near_call'
> > then
> > echo "Register capture test [Skipped missing instruction]"
> > + if [ $err -ne 1 ]
> > + then
> > + err=2
> > + fi
> > return
> > fi
> > if ! perf record --intr-regs=\? 2>&1 | egrep -q 'available registers: AX BX CX DX SI DI BP SP IP FLAGS CS SS R8 R9 R10 R11 R12 R13 R14 R15'
> > @@ -39,4 +75,6 @@ test_register_capture() {
> >
> > test_per_thread
> > test_register_capture
> > +
> > +cleanup
> > exit $err
>
> Ian,
>
> thanks very much for looking into this.
> I have tested your patch on several configurations and to be perfect, please
> change this line:
> # git diff
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index efc0cb07a929..9cb8494f314d 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -21,7 +21,7 @@ trap trap_cleanup exit term int
>
> test_per_thread() {
> echo "Basic --per-thread mode test"
> - if ! perf stat -e instructions:u --quiet true
> + if ! perf record -e instructions:u --quiet true 2> /dev/null
> then
> echo "Per-thread record [Skipped instructions:u not supported]"
> if [ $err -ne 1 ]
> #
>
> This runs fine, just use record instead of stat, you use perf record later
> on anyway.
>
> With this change you have my
> Tested-by: Thomas Richter <tmricht@linux.ibm.com>
Thanks! Done, but I also add a '-o' in v2 so the perf.data file is
cleaned up by the cleanup function.
Ian
> --
> Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
> --
> Vorsitzender des Aufsichtsrats: Gregor Pillen
> Geschäftsführung: David Faller
> Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
prev parent reply other threads:[~2022-05-03 4:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-29 6:13 [PATCH] perf test: Add skip to --per-thread test Ian Rogers
2022-05-02 9:39 ` Thomas Richter
2022-05-03 4:33 ` Ian Rogers [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAP-5=fU8+QOeZjL=DMgwwQeJiKqnKw28eF=0m1kmp8C7Mwfz+Q@mail.gmail.com' \
--to=irogers@google.com \
--cc=acme@kernel.org \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=namhyung@kernel.org \
--cc=sumanthk@linux.ibm.com \
--cc=svens@linux.ibm.com \
--cc=tmricht@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).