Re: [PATCH v3 1/2] selftests/amd-pstate: Fix broken paths to run workloads in amd-pstate-ut

On 10/3/2023 00:10, Swapnil Sapkal wrote:

In selftests/amd-pstate, tbench and gitsource microbenchmarks are used to compare the performance with different governors. In Current

s/Current/current/

implementation relative path to run `amd_pstate_tracer.py` are broken.

The plurality of this sentence needs some work. I suggest:

s,relative,the relative, s,are broken,is broken,

Fixed this by using absolute paths.

The tense is wrong.

s,Fixed,Fix/,

Signed-off-by: Swapnil Sapkal swapnil.sapkal@amd.com

.../x86/amd_pstate_tracer/amd_pstate_trace.py | 2 +- tools/testing/selftests/amd-pstate/gitsource.sh | 14 +++++++++----- tools/testing/selftests/amd-pstate/run.sh | 9 ++++++--- tools/testing/selftests/amd-pstate/tbench.sh | 2 +- 4 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py index 904df0ea0a1e..2448bb07973f 100755 --- a/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py +++ b/tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py @@ -30,7 +30,7 @@ import getopt import Gnuplot from numpy import * from decimal import * -sys.path.append('../intel_pstate_tracer') +sys.path.append(os.path.join(os.path.dirname(__file__), '../intel_pstate_tracer'))

If you're using os.path.join, shouldn't you not be hardcoding a "/" in there?

IE it should be:

sys.path.append(os.path.join(os.path.dirname(__file__), "..", "intel_pstate_tracer"))

#import intel_pstate_tracer

I think another patch should remove this commented line, it conveys zero information.

import intel_pstate_tracer as ipt diff --git a/tools/testing/selftests/amd-pstate/gitsource.sh b/tools/testing/selftests/amd-pstate/gitsource.sh index 5f2171f0116d..d0ad2ed5ba9d 100755 --- a/tools/testing/selftests/amd-pstate/gitsource.sh +++ b/tools/testing/selftests/amd-pstate/gitsource.sh @@ -66,12 +66,15 @@ post_clear_gitsource() install_gitsource() {

• if [ ! -d $git_name ]; then

• if [ ! -d $SCRIPTDIR/$git_name ]; then

• BACKUP_DIR=$(pwd)
  

• cd $SCRIPTDIR
  

  printf "Download gitsource, please wait a moment ...\n\n" wget -O $git_tar $gitsource_url > /dev/null 2>&1

printf "Tar gitsource ...\n\n" tar -xzf $git_tar

• cd $BACKUP_DIR
  

If you change to /bin/bash instead of /bin/sh you could use pushd/popd instead. If your goal is to keep compatibility with things like /bin/dash then this makes ense.

fi } @@ -79,12 +82,13 @@ install_gitsource() run_gitsource() { echo "Launching amd pstate tracer for $1 #$2 tracer_interval: $TRACER_INTERVAL"

• ./amd_pstate_trace.py -n tracer-gitsource-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 &

• $TRACER -n tracer-gitsource-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 &

printf "Make and test gitsource for $1 #$2 make_cpus: $MAKE_CPUS\n"

• cd $git_name
• perf stat -a --per-socket -I 1000 -e power/energy-pkg/ /usr/bin/time -o ../$OUTFILE_GIT.time-gitsource-$1-$2.log make test -j$MAKE_CPUS > ../$OUTFILE_GIT-perf-$1-$2.log 2>&1
• cd ..

• 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
• cd $BACKUP_DIR

Similar pushd/popd comment could apply here.

for job in `jobs -p` do diff --git a/tools/testing/selftests/amd-pstate/run.sh b/tools/testing/selftests/amd-pstate/run.sh index de4d8e9c9565..279d073c5728 100755 --- a/tools/testing/selftests/amd-pstate/run.sh +++ b/tools/testing/selftests/amd-pstate/run.sh @@ -8,9 +8,12 @@ else FILE_MAIN=DONE fi -source basic.sh -source tbench.sh -source gitsource.sh +SCRIPTDIR=`dirname "$0"` +TRACER=$SCRIPTDIR/../../../power/x86/amd_pstate_tracer/amd_pstate_trace.py

+source $SCRIPTDIR/basic.sh +source $SCRIPTDIR/tbench.sh +source $SCRIPTDIR/gitsource.sh # amd-pstate-ut only run on x86/x86_64 AMD systems. ARCH=$(uname -m 2>/dev/null | sed -e 's/i.86/x86/' -e 's/x86_64/x86/') diff --git a/tools/testing/selftests/amd-pstate/tbench.sh b/tools/testing/selftests/amd-pstate/tbench.sh index 49c9850341f6..4d2e8ce2da3b 100755 --- a/tools/testing/selftests/amd-pstate/tbench.sh +++ b/tools/testing/selftests/amd-pstate/tbench.sh @@ -64,7 +64,7 @@ post_clear_tbench() run_tbench() { echo "Launching amd pstate tracer for $1 #$2 tracer_interval: $TRACER_INTERVAL"

• ./amd_pstate_trace.py -n tracer-tbench-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 &

• $TRACER -n tracer-tbench-$1-$2 -i $TRACER_INTERVAL > /dev/null 2>&1 &

printf "Test tbench for $1 #$2 time_limit: $TIME_LIMIT procs_num: $PROCESS_NUM\n" tbench_srv > /dev/null 2>&1 &