Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <33271bcf-66fd-42e9-9b19-0c1547814f38@amd.com>
Date:   Fri, 6 Oct 2023 13:59:21 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 2/2] selftests/amd-pstate: Added option to provide perf
 binary path
Content-Language: en-US
To:     Swapnil Sapkal <swapnil.sapkal@amd.com>, ray.huang@amd.com,
        shuah@kernel.org
Cc:     sukrut.bellary@gmail.com, li.meng@amd.com, gautham.shenoy@amd.com,
        wyes.karny@amd.com, Perry.Yuan@amd.com, linux-pm@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
References: <20231003051006.6343-1-swapnil.sapkal@amd.com>
 <20231003051006.6343-3-swapnil.sapkal@amd.com>
From:   Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <20231003051006.6343-3-swapnil.sapkal@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cjgvTEw4VGU5WTBwUTlZcEJncEVJU0RzOE4wemp6L3hBMjNSUkxGaXExelVv?=
 =?utf-8?B?OTJ0cnNGMzJoWWs5QkptZEdoZlRUNWlBV3VrNWlTbFVTdnFBWDJRZkZJSXZr?=
 =?utf-8?B?enBab3lZTzNYVTFEZGVrNUQxMDBTeWhZUHpWTjZscmlhTXZ6NEZYbmlkQTFj?=
 =?utf-8?B?NHBZWFg1MUtGMlJSZGVqTFlEWUFxS3hpMFdBZlVnTEZEajU4YnloUGJPcmsr?=
 =?utf-8?B?ZnJJVWNPMDFDRHZ1aTh4M3VHaUtkbnpRa2pJOExHcEJDaDUzcWk2YUxWUHZw?=
 =?utf-8?B?MFNVY3ZGbFJOQytHeGxtdTY0WFhCZ1JRMHg2cWpZSnByeWFxK3hlNkJTS0lD?=
 =?utf-8?B?NGNlSDQ5dWozOG5VS2VLUzZob3VTYndBMFp2Rk9Nc3RJeDBKT0g1TURTWERo?=
 =?utf-8?B?VDdHbGErdDRYb0VrZ20zQ2M3OVZndWxCS0xPWHkyTmxGcjRpOVNZNkZBbEJn?=
 =?utf-8?B?cWx4ckZ3UjBqRW8xVUROdzBFVXQ0eXBGMVYrWThmSzM4YlY4M3kyU2w0NEVN?=
 =?utf-8?B?Mlc3dXlKZmQ1UmpDTDNYUXlXZC9teW9haHFwbGtCbDZCMzIvZnhPZWNOSWU2?=
 =?utf-8?B?WDV5N1dsSyt6UmRFTHFzZDgxd2RPcmtpWm5oRm95RWlPZEpKZW8xKzA0bGhk?=
 =?utf-8?B?MVFiaTVoOFNOSitOM2o4UVRoV3MxUU5oTHBEOCsrNHBKU3plOFc2S1JJV21X?=
 =?utf-8?B?bnNFUFoxK2VFVm14cVF0cC9XdkhraEpIUGltSUJLQlpxMzhBeGNuSG9OdkNN?=
 =?utf-8?B?RVZocDJDQmNvZmlCSFZEL1FmZml1VG1zazZnaSt4ZENmTWJkbElCUDQyVHMz?=
 =?utf-8?B?ai9VTlRUb0h4R3haNzk5WFBFWGRxSk1FcHp3YTNCZ3IwK2lXK2pqWDZFSk9r?=
 =?utf-8?B?MUdJVy9nV3RNUzg3QUozYUMxUUY1Z2pyNHNBU0dJejA1SERFMlV0cXlXcStp?=
 =?utf-8?B?ai96RzgwMEVrWlEzSGJBMXBUTTZLRjFCZXdvQzdYUW5UNDhUQ3JLY09nNzhm?=
 =?utf-8?B?VytuRE9PYysyK1o4SWxmeEVtK3ViTXZKTjZPY2hqb0NNdzJrZmpDRHVjeTBM?=
 =?utf-8?B?Wm5neVZvSDRzaXZKQlpSSk1CckxzTktUYjhHdGdWSy9HWHp5aWtMN1h2eWVh?=
 =?utf-8?B?NVNvdjczSnk0emRoZWxmWlFKUW1EQ0M3c29ySnM4U0lrdlNyQ3dPcHlRU3hV?=
 =?utf-8?B?aXV2LzNkR0tTNTA5YUpKRHYwMGxJblA2d1NvQmVOUklsaGswSGFxcGpZQzJF?=
 =?utf-8?B?b2dZZVdDeXlsakFFdm9GSHFnd1Z0RzEweUhLdHU0K3kzWGxLaVdYZzhCSWY1?=
 =?utf-8?B?SzlGeXRCdzAxSHBUd09nRU0wYTYzWTBBanlLRUJOSTNXNTlSbVY4ZkNQNGZO?=
 =?utf-8?B?dFo5WXNCOGpUMkduajVQeERKTTVmZ3VhMmRhSUNKV1NxYko4RHV4bUZSRkpp?=
 =?utf-8?B?dGx3WjlNVnpObWE4L1d5R2IwMFA5eXRMMk9hbk9GaVlIa2VLY2NsZmZQNWky?=
 =?utf-8?B?MCtqQVB2UWxIdDVMeWxMb3dCZ0hhTllUc2dkRll0UnNJYWtpU0NDZGI0UGd4?=
 =?utf-8?B?UzB3QmYyd29DdTdUWmR3L2p3VXRNSUJ5cGM5dG0zcHpXeTl0WnhkSGQrQlA1?=
 =?utf-8?B?SUp2b05zSUl5bEwxVzQxY0E1dDlOM0ZqU3VxV1RiWVlpNW9keFlEQVBCM3Jp?=
 =?utf-8?B?ODB3Z0k1bzZDWXc5UWF5QnBWU3hiYmNsWkt5eUw1NGVoY0VHeUFSVW1mTTRm?=
 =?utf-8?B?QkZ6bDhBRWMxQmJHdUp3VjVMRlJZbS85Qm45T3RzKzJSNFJLUE50dWxiM3NC?=
 =?utf-8?B?eVpmdFlWUFJFOWg1RzBRWExnRjFoNmRqL1dsaklnb3ZvUEdyTzBlb2F1UWFi?=
 =?utf-8?B?TW5jbFA5eHVYVEpheVZjVWFVK3pYZXdkT1QzcFZLWUYyMUE5NUptS3VlWjBy?=
 =?utf-8?B?M1VtcFlIblR1RWxncEVkVG5ybFFYbGxZNy9PcEFJMGpUYTRFdWtHTTZkTTYx?=
 =?utf-8?B?Q0p6OXM0eVp2elcwRWtOMUxtM2RaVXZrUXhPbGI0YXZIa0x2MFNzM013RjFW?=
 =?utf-8?B?WXhrZjZJZko2UlVYRVE1bTVjMVE2WitHRU8zWWlYMlhreDVYMnNqbmVJTzNW?=
 =?utf-8?Q?tT/SCp2R8E9PzfusTtAlFsZp7?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 733ab3e4-af2a-41bf-55ec-08dbc69e5b24
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Oct 2023 18:59:24.1925
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: r+Pbf6NK6NLC6bBJYjJNDjgInFyQnvxtCGZb5XzYehfFQZgN1AxlVW55uXqxqiXPcMigEsYKITkRwFSpCsv2UQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6275
X-Spam-Status: No, score=2.7 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS autolearn=no
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Fri, 06 Oct 2023 11:59:42 -0700 (PDT)
X-Spam-Level: **

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