diff mbox series

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

Message ID 20231003051006.6343-3-swapnil.sapkal@amd.com (mailing list archive)
State New
Headers show
Series Fix issues observed in selftests/amd-pstate | expand

Commit Message

Swapnil Sapkal Oct. 3, 2023, 5:10 a.m. UTC 
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@amd.com>
---
 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(-)

Comments

Mario Limonciello Oct. 6, 2023, 6:59 p.m. UTC | #1 
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@amd.com>
> ---

One small nit, otherwise:

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

>   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 <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
Swapnil Sapkal Oct. 12, 2023, 5:04 a.m. UTC | #2 
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@amd.com>
>> ---
> 
> One small nit, otherwise:
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> 
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
diff mbox series

Patch

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>]
 	[-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