Re: [PATCH v3 2/2] selftests/amd-pstate: Added option to provide perf binary path

[Swapnil Sapkal]

Hello Mario,

On 10/7/2023 12:29 AM, Mario Limonciello wrote:
> On 10/3/2023 00:10, Swapnil Sapkal wrote:
> > In selftests/amd-pstate, distro `perf` is used to capture `perf stat`
> > while running microbenchmarks. Distro `perf` is not working with
> > upstream kernel. Fixed this by providing an option to give the perf
> > binary path.
> >
> > Signed-off-by: Swapnil Sapkal <swapnil.sapkal@xxxxxxx>
> > ---
>
> One small nit, otherwise:
>
> Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>
I will include the tag in next version.

> >   tools/testing/selftests/amd-pstate/gitsource.sh |  2 +-
> >   tools/testing/selftests/amd-pstate/run.sh       | 14 ++++++++++----
> >   tools/testing/selftests/amd-pstate/tbench.sh    |  2 +-
> >   3 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/testing/selftests/amd-pstate/gitsource.sh b/tools/testing/selftests/amd-pstate/gitsource.sh
> > index d0ad2ed5ba9d..5acc065e9e3e 100755
> > --- a/tools/testing/selftests/amd-pstate/gitsource.sh
> > +++ b/tools/testing/selftests/amd-pstate/gitsource.sh
> > @@ -87,7 +87,7 @@ run_gitsource()
> >       printf "Make and test gitsource for $1 #$2 make_cpus: $MAKE_CPUS\n"
> >       BACKUP_DIR=$(pwd)
> >       cd $SCRIPTDIR/$git_name
> > -    perf stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o $BACKUP_DIR/$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > $BACKUP_DIR/$OUTFILE_GIT-perf-$1-$2.log 2>&1
> > +    $PERF stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o $BACKUP_DIR/$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > $BACKUP_DIR/$OUTFILE_GIT-perf-$1-$2.log 2>&1
> >       cd $BACKUP_DIR
> >       for job in `jobs -p`
> > diff --git a/tools/testing/selftests/amd-pstate/run.sh b/tools/testing/selftests/amd-pstate/run.sh
> > index 279d073c5728..b87cdc5bfe4a 100755
> > --- a/tools/testing/selftests/amd-pstate/run.sh
> > +++ b/tools/testing/selftests/amd-pstate/run.sh
> > @@ -25,6 +25,7 @@ OUTFILE=selftest
> >   OUTFILE_TBENCH="$OUTFILE.tbench"
> >   OUTFILE_GIT="$OUTFILE.gitsource"
> > +PERF=/usr/bin/perf
> >   SYSFS=
> >   CPUROOT=
> >   CPUFREQROOT=
> > @@ -152,8 +153,9 @@ help()
> >            gitsource: Gitsource testing.>]
> >       [-t <tbench time limit>]
> >       [-p <tbench process number>]
> > -    [-l <loop times for tbench>]
> > +    [-l <loop times for tbench/gitsource>]
>
> This looks like unrelated change.

I will remove this change.
> >       [-i <amd tracer interval>]
> > +    [-b <perf binary>]
> >       [-m <comparative test: acpi-cpufreq>]
> >       \n"
> >       exit 2
> > @@ -161,7 +163,7 @@ help()
> >   parse_arguments()
> >   {
> > -    while getopts ho:c:t:p:l:i:m: arg
> > +    while getopts ho:c:t:p:l:i:b:m: arg
> >       do
> >           case $arg in
> >               h) # --help
> > @@ -192,6 +194,10 @@ parse_arguments()
> >                   TRACER_INTERVAL=$OPTARG
> >                   ;;
> > +            b) # --perf-binary
> > +                PERF=`realpath $OPTARG`
> > +                ;;
> > +
> >               m) # --comparative-test
> >                   COMPARATIVE_TEST=$OPTARG
> >                   ;;
> > @@ -205,8 +211,8 @@ parse_arguments()
> >   command_perf()
> >   {
> > -    if ! command -v perf > /dev/null; then
> > -        echo $msg please install perf. >&2
> > +    if ! $PERF -v; then
> > +        echo $msg please install perf or provide perf binary path as argument >&2
> >           exit $ksft_skip
> >       fi
> >   }
> > diff --git a/tools/testing/selftests/amd-pstate/tbench.sh b/tools/testing/selftests/amd-pstate/tbench.sh
> > index 4d2e8ce2da3b..2a98d9c9202e 100755
> > --- a/tools/testing/selftests/amd-pstate/tbench.sh
> > +++ b/tools/testing/selftests/amd-pstate/tbench.sh
> > @@ -68,7 +68,7 @@ run_tbench()
> >       printf "Test tbench for $1 #$2 time_limit: $TIME_LIMIT procs_num: $PROCESS_NUM\n"
> >       tbench_srv > /dev/null 2>&1 &
> > -    perf stat -a --per-socket -I 1000 -e power/energy-pkg/ tbench -t $TIME_LIMIT $PROCESS_NUM > $OUTFILE_TBENCH-perf-$1-$2.log 2>&1
> > +    $PERF stat -a --per-socket -I 1000 -e power/energy-pkg/ tbench -t $TIME_LIMIT $PROCESS_NUM > $OUTFILE_TBENCH-perf-$1-$2.log 2>&1
> >       pid=`pidof tbench_srv`
> >       kill $pid
--
Thanks and Regards,
Swapnil