linux-kernel - Re: [PATCH v5] perf test: Introduce script for Arm CoreSight testing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 7 Sep 2020 15:32:48 +0800
From:   Leo Yan <leo.yan@...aro.org>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Mike Leach <mike.leach@...aro.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] perf test: Introduce script for Arm CoreSight testing

Hi Mathieu,

On Fri, Sep 04, 2020 at 03:33:32PM -0600, Mathieu Poirier wrote:

[...]

> > diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
> > new file mode 100755
> > index 000000000000..8696bb5df45a
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/test_arm_coresight.sh
> > @@ -0,0 +1,175 @@
> > +#!/bin/sh
> > +# Check Arm CoreSight trace data recording and branch samples
> > +
> > +# Uses the 'perf record' to record trace data with Arm CoreSight sinks;
> > +# then verify if there have any branch samples and instruction samples
> > +# are generated by CoreSight with 'perf script' and 'perf report'
> > +# commands.
> > +
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Leo Yan <leo.yan@...aro.org>, 2020
> > +
> > +perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> > +file=$(mktemp /tmp/temporary_file.XXXXX)
> > +
> > +skip_if_no_cs_etm_event() {
> > +	perf list | grep -q 'cs_etm//' && return 0
> > +
> > +	# cs_etm event doesn't exist
> > +	return 2
> > +}
> > +
> > +skip_if_no_cs_etm_event || exit 2
> > +
> > +record_touch_file() {
> > +	echo "Recording trace (only user mode) with path: CPU$2 => $1"
> > +	rm -f $file
> > +	perf record -o ${perfdata} -e cs_etm/@...u --per-thread \
> > +		-- taskset -c $2 touch $file
> > +}
> > +
> > +perf_script_branch_samples() {
> > +	echo "Looking at perf.data file for dumping branch samples:"
> > +
> > +	# Below is an example of the branch samples dumping:
> > +	#   touch  6512          1         branches:u:      ffffb220824c strcmp+0xc (/lib/aarch64-linux-gnu/ld-2.27.so)
> > +	#   touch  6512          1         branches:u:      ffffb22082e0 strcmp+0xa0 (/lib/aarch64-linux-gnu/ld-2.27.so)
> > +	#   touch  6512          1         branches:u:      ffffb2208320 strcmp+0xe0 (/lib/aarch64-linux-gnu/ld-2.27.so)
> > +	perf script -F,-time -i ${perfdata} | \
> > +		egrep " +$1 +[0-9]+ .* +branches:([u|k]:)? +"
> > +}
> > +
> > +perf_report_branch_samples() {
> > +	echo "Looking at perf.data file for reporting branch samples:"
> > +
> > +	# Below is an example of the branch samples reporting:
> > +	#   73.04%    73.04%  touch    libc-2.27.so      [.] _dl_addr
> > +	#    7.71%     7.71%  touch    libc-2.27.so      [.] getenv
> > +	#    2.59%     2.59%  touch    ld-2.27.so        [.] strcmp
> > +	perf report --stdio -i ${perfdata} | \
> > +		egrep " +[0-9]+\.[0-9]+% +[0-9]+\.[0-9]+% +$1 "
> > +}
> > +
> > +perf_report_instruction_samples() {
> > +	echo "Looking at perf.data file for instruction samples:"
> > +
> > +	# Below is an example of the instruction samples reporting:
> > +	#   68.12%  touch    libc-2.27.so   [.] _dl_addr
> > +	#    5.80%  touch    libc-2.27.so   [.] getenv
> > +	#    4.35%  touch    ld-2.27.so     [.] _dl_fixup
> > +	perf report --itrace=i1000i --stdio -i ${perfdata} | \
> > +		egrep " +[0-9]+\.[0-9]+% +$1"
> > +}
> > +
> > +arm_cs_iterate_devices() {
> > +	for dev in $1/connections/out\:*; do
> > +
> > +		# Skip testing if it's not a directory
> > +		! [ -d $dev ] && continue;
> > +
> > +		# Read out its symbol link file name
> > +		path=`readlink -f $dev`
> > +
> > +		# Extract device name from path, e.g.
> > +		#   path = '/sys/devices/platform/20010000.etf/tmc_etf0'
> > +		#     `> device_name = 'tmc_etf0'
> > +		device_name=`echo $path | awk -F/ '{print $(NF)}'`
> > +
> > +
> > +		# If the node of "enable_sink" is existed under the device path, this
> > +		# means the device is a sink device.  Need to exclude 'tpiu' since it
> > +		# cannot support perf PMU.
> > +		echo $device_name | egrep -q -v "tpiu"
> > +		if [ $? -eq 0 -a -e "$path/enable_sink" ]; then
> > +
> > +			pmu_dev="/sys/bus/event_source/devices/cs_etm/sinks/$device_name"
> > +
> > +			# Exit if the sink device is supported by PMU or not
> > +			if ! [ -f $pmu_dev ]; then
> > +				echo "PMU doesn't support $pmu_dev"
> > +				exit 1
> > +			fi
> > +
> > +			record_touch_file $device_name $2 &&
> > +				perf_script_branch_samples touch &&
> > +				perf_report_branch_samples touch &&
> > +				perf_report_instruction_samples touch
> 
> Did you want to have an extra tab after the first line?  I think it makes the
> code harder to read, but that's a personal opinion. 

Thanks for the reviewing!  Have removed the extra tab in patch v6.

> > +
> > +			err=$?
> > +
> > +			# Exit when find failure
> > +			[ $err != 0 ] && exit $err
> 
> If a problem occurs ${perfdata} and ${file} are not removed.  The same is true
> in several instances below.

Have fixed to remove ${perfdata} and ${file} for failure cases in
patch v6.

Thanks,
Leo