[PATCH 00/17] tool/perf/test: Fix shellcheck coding/formatting issues of test shell scripts

[PATCH 00/17] tool/perf/test: Fix shellcheck coding/formatting issues of test shell scripts

[Athira Rajeev]

Patchset covers a set of fixes for coding/formatting issues observed while
running shellcheck tool on the perf test shell scripts. Shellcheck is a static
analysis tool that can find semantic/syntax bugs in the shell scripts.

Patches 1-14 fixes the issues found with shellcheck. Patch 15, 16
and patch 17 address a fix in task_analyzer test.

This cleanup is a pre-requisite to include a build option for shellcheck
discussed here: https://www.spinics.net/lists/linux-perf-users/msg25553.html
Also this is first set of patches. There will be one more set which will
include build option for shellcheck as discussed in the mail thread.

Abhirup Deb (2):
  tools/perf/tests: fix test_arm_spe.sh signal case issues
  perf/tests/shell: fix shellscript errors for lock_contention.sh

Aboorva Devarajan (1):
  tools/perf/tests: Fix shellcheck issues in test_task_analyzer.sh file

Aditya Gupta (3):
  perf tests task_analyzer: fix bad substitution ${$1}
  perf tests task_analyzer: print command on failure
  perf tests task_analyzer: skip tests if no libtraceevent support

Akanksha J N (1):
  tools/perf/tests: Fix shellcheck warnings for
    trace+probe_vfs_getname.sh

Anushree Mathur (1):
  perf/tests/shell : Shellcheck fixes for perf test
    "test_arm_coresight.sh"

Barnali Guha Thakurata (1):
  tools/perf/tests/shell/stat_all_metrics: Fix shellcheck warning SC2076
    in stat_all_metrics.sh

Disha Goel (1):
  tools/perf/tests: fix shellcheck warning for stat+json_output

Geetika (1):
  tools/perf/tests: Fix all POSIX sh warnings in perf shell test
    test_brstack.sh

Korrapati Likhitha (1):
  tools/perf/tests: Fix shellcheck warnings for stat+csv_output

Samir Mulani (1):
  tools/perf/tests: fixed shellcheck warnings for perf shell scripts

Shirisha G (1):
  tools/perf/tests: fix shellcheck warnings for daemon.sh

Sourabh Jain (1):
  perf: get rid of unused import

Spoorthy S (2):
  shellcheck : fixing signal names and adding double quotes for
    expression in test_arm_callgraph_fp
  tools/perf/tests: Fix all POSIX sh warnings in stat+shadow_stat.sh

 .../scripts/python/arm-cs-trace-disasm.py     |   1 -
 tools/perf/tests/shell/buildid.sh             |  12 +-
 tools/perf/tests/shell/daemon.sh              | 113 ++++++++++++------
 tools/perf/tests/shell/lock_contention.sh     |  70 +++++------
 .../shell/record+probe_libc_inet_pton.sh      |   6 +-
 .../shell/record+script_probe_vfs_getname.sh  |   4 +-
 tools/perf/tests/shell/stat+csv_output.sh     |   4 +-
 tools/perf/tests/shell/stat+json_output.sh    |   2 +-
 tools/perf/tests/shell/stat+shadow_stat.sh    |   4 +-
 tools/perf/tests/shell/stat_all_metrics.sh    |   6 +-
 .../perf/tests/shell/test_arm_callgraph_fp.sh |   6 +-
 tools/perf/tests/shell/test_arm_coresight.sh  |   6 +-
 tools/perf/tests/shell/test_arm_spe.sh        |   2 +-
 tools/perf/tests/shell/test_brstack.sh        |  12 +-
 tools/perf/tests/shell/test_task_analyzer.sh  |  98 ++++++++-------
 .../tests/shell/trace+probe_vfs_getname.sh    |   6 +-
 16 files changed, 203 insertions(+), 149 deletions(-)

-- 
2.39.1

[PATCH 01/17] perf: get rid of unused import

[Athira Rajeev]

From: Sourabh Jain <sourabhjain@linux.ibm.com>

Script doesn't use sys library, so remove it.

Report by pylint:
W0611: Unused import sys (unused-import)

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 tools/perf/scripts/python/arm-cs-trace-disasm.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
index 4339692a8d0b..d59ff53f1d94 100755
--- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
+++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
@@ -9,7 +9,6 @@
 from __future__ import print_function
 import os
 from os import path
-import sys
 import re
 from subprocess import *
 from optparse import OptionParser, make_option
-- 
2.39.1

Re: [PATCH 01/17] perf: get rid of unused import

[Arnaldo Carvalho de Melo]

Em Tue, Jun 13, 2023 at 10:11:29PM +0530, Athira Rajeev escreveu:
> From: Sourabh Jain <sourabhjain@linux.ibm.com>
> 
> Script doesn't use sys library, so remove it.

Please Cc the persons working on that file, I added Leo to the CC list
of this message.

Thanks, applied.

- Arnaldo
 
> Report by pylint:
> W0611: Unused import sys (unused-import)
> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>  tools/perf/scripts/python/arm-cs-trace-disasm.py | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> index 4339692a8d0b..d59ff53f1d94 100755
> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> @@ -9,7 +9,6 @@
>  from __future__ import print_function
>  import os
>  from os import path
> -import sys
>  import re
>  from subprocess import *
>  from optparse import OptionParser, make_option
> -- 
> 2.39.1
> 

-- 

- Arnaldo

Re: [PATCH 01/17] perf: get rid of unused import

[Leo Yan]

On Tue, Jun 13, 2023 at 04:54:08PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jun 13, 2023 at 10:11:29PM +0530, Athira Rajeev escreveu:
> > From: Sourabh Jain <sourabhjain@linux.ibm.com>
> > 
> > Script doesn't use sys library, so remove it.
> 
> Please Cc the persons working on that file, I added Leo to the CC list
> of this message.

Thanks, Arnaldo & Athira.  The change looks good to me.

> Thanks, applied.

Since have applied this patch, it's no need to give my review tag :)

Thanks,
Leo

> - Arnaldo
>  
> > Report by pylint:
> > W0611: Unused import sys (unused-import)
> > 
> > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> > Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> > ---
> >  tools/perf/scripts/python/arm-cs-trace-disasm.py | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > index 4339692a8d0b..d59ff53f1d94 100755
> > --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > @@ -9,7 +9,6 @@
> >  from __future__ import print_function
> >  import os
> >  from os import path
> > -import sys
> >  import re
> >  from subprocess import *
> >  from optparse import OptionParser, make_option
> > -- 
> > 2.39.1
> > 
> 
> -- 
> 
> - Arnaldo

Re: [PATCH 01/17] perf: get rid of unused import

[Arnaldo Carvalho de Melo]

Em Wed, Jun 14, 2023 at 09:59:14AM +0800, Leo Yan escreveu:
> On Tue, Jun 13, 2023 at 04:54:08PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jun 13, 2023 at 10:11:29PM +0530, Athira Rajeev escreveu:
> > > From: Sourabh Jain <sourabhjain@linux.ibm.com>
> > > 
> > > Script doesn't use sys library, so remove it.
> > 
> > Please Cc the persons working on that file, I added Leo to the CC list
> > of this message.
> 
> Thanks, Arnaldo & Athira.  The change looks good to me.
> 
> > Thanks, applied.
> 
> Since have applied this patch, it's no need to give my review tag :)

No, I usually can  add a Reviewed-by tag even after having applied it to
my local tree, as I still need to run tests before making it available
via perf-tools-next, when I should not make any further changes.

- Arnaldo
 
> Thanks,
> Leo
> 
> > - Arnaldo
> >  
> > > Report by pylint:
> > > W0611: Unused import sys (unused-import)
> > > 
> > > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > > Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> > > Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> > > ---
> > >  tools/perf/scripts/python/arm-cs-trace-disasm.py | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/tools/perf/scripts/python/arm-cs-trace-disasm.py b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > > index 4339692a8d0b..d59ff53f1d94 100755
> > > --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > > +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
> > > @@ -9,7 +9,6 @@
> > >  from __future__ import print_function
> > >  import os
> > >  from os import path
> > > -import sys
> > >  import re
> > >  from subprocess import *
> > >  from optparse import OptionParser, make_option
> > > -- 
> > > 2.39.1
> > > 
> > 
> > -- 
> > 
> > - Arnaldo

-- 

- Arnaldo

Re: [PATCH 01/17] perf: get rid of unused import

[Leo Yan]

On Tue, Jun 13, 2023 at 11:34:11PM -0300, Arnaldo Carvalho de Melo wrote:

[...]

> > Since have applied this patch, it's no need to give my review tag :)
> 
> No, I usually can  add a Reviewed-by tag even after having applied it to
> my local tree, as I still need to run tests before making it available
> via perf-tools-next, when I should not make any further changes.

Understand now.  Here is my review tag:

Reviewed-by: Leo Yan <leo.yan@linaro.org>

[PATCH 02/17] tools/perf/tests: fix shellcheck warning for stat+json_output

[Athira Rajeev]

From: Disha Goel <disgoel@linux.ibm.com>

Running shellcheck on stat+json_output testcase, generates below warning:

	 [ $(id -u) != 0 ] && [ $(cat /proc/sys/kernel/perf_event_paranoid) -gt $1 ]
           ^------^ SC2046 (warning): Quote this to prevent word splitting.
                                ^-- SC2046 (warning): Quote this to prevent word splitting.

Fixed the warning by adding quotes to avoid word splitting.

ShellCheck result with patch:
	 # shellcheck -S warning stat+json_output.sh
	 #

perf test result after the change:
	 94: perf stat JSON output linter : Ok

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Disha Goel <disgoel@linux.ibm.com>
---
 tools/perf/tests/shell/stat+json_output.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/stat+json_output.sh b/tools/perf/tests/shell/stat+json_output.sh
index f3e4967cc72e..c19f926a2951 100755
--- a/tools/perf/tests/shell/stat+json_output.sh
+++ b/tools/perf/tests/shell/stat+json_output.sh
@@ -40,7 +40,7 @@ trap trap_cleanup EXIT TERM INT
 # Return true if perf_event_paranoid is > $1 and not running as root.
 function ParanoidAndNotRoot()
 {
-	 [ $(id -u) != 0 ] && [ $(cat /proc/sys/kernel/perf_event_paranoid) -gt $1 ]
+	 [ "$(id -u)" != 0 ] && [ "$(cat /proc/sys/kernel/perf_event_paranoid)" -gt $1 ]
 }
 
 check_no_args()
-- 
2.39.1

[PATCH 03/17] shellcheck : fixing signal names and adding double quotes for expression in test_arm_callgraph_fp

[Athira Rajeev]

From: Spoorthy S <spoorts2@in.ibm.com>

Running shellcheck -S on test_arm_calligraph_fp throws warnings SC2086 and SC3049,

      $shellcheck -S warning tests/shell/test_arm_callgraph_fp.sh
         rm -f $PERF_DATA
            : Double quote to prevent globbing and word splitting.
         trap cleanup_files exit term int
                     : In POSIX sh, using lower/mixed case for signal names is undefined.

After fixing the warnings,

      $shellcheck tests/shell/test_arm_callgraph_fp.sh
      $ echo $?
      0

To address the POSIX shell warnings added changes to convert Lowercase
signal names to uppercase in the script and double quoted the
command substitutions($fix to "$fix") to solve Globbing warnings.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Spoorthy S<spoorts2@in.ibm.com>
---
 tools/perf/tests/shell/test_arm_callgraph_fp.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/test_arm_callgraph_fp.sh b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
index e61d8deaa0c4..1380e0d12dce 100755
--- a/tools/perf/tests/shell/test_arm_callgraph_fp.sh
+++ b/tools/perf/tests/shell/test_arm_callgraph_fp.sh
@@ -9,13 +9,13 @@ TEST_PROGRAM="perf test -w leafloop"
 
 cleanup_files()
 {
-	rm -f $PERF_DATA
+	rm -f "$PERF_DATA"
 }
 
-trap cleanup_files exit term int
+trap cleanup_files EXIT TERM INT
 
 # Add a 1 second delay to skip samples that are not in the leaf() function
-perf record -o $PERF_DATA --call-graph fp -e cycles//u -D 1000 --user-callchains -- $TEST_PROGRAM 2> /dev/null &
+perf record -o "$PERF_DATA" --call-graph fp -e cycles//u -D 1000 --user-callchains -- "$TEST_PROGRAM" 2> /dev/null &
 PID=$!
 
 echo " + Recording (PID=$PID)..."
-- 
2.39.1

[PATCH 04/17] tools/perf/tests: fix shellcheck warnings for daemon.sh

[Athira Rajeev]

From: Shirisha G <shirisha@linux.ibm.com>

Running shellcheck -S on daemon.sh throws below warnings:

Result from shellcheck:
     # shellcheck -S warning daemon.sh
     local line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
           ^-------^ SC2155: Declare and assign separately to avoid masking return values.

     trap "echo 'FAILED: Signal caught'; daemon_exit ${config}; exit 1" SIGINT SIGTERM
                                                     ^-------^ SC2064: Use single quotes, otherwise this expands now rather than when signalled.

     count=`ls ${base}/session-test/ | grep perf.data | wc -l`
            ^-- SC2010: Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.

     if [ ${size} != "OK" -o ${type} != "OK" ]; then
                          ^-- SC2166: Prefer [ p ] || [ q ] as [ p -o q ] is not well defined.

Fixed above warnings by:
   - declaring and assigning local variables separately
   - To fix SC2010, instead of using "ls | grep", used glob to allow non-alphanumeric filenames
   - Used single quotes to prevent expanding.

Result from shellcheck after patch changes:
     $ shellcheck -S warning daemon.sh
     $ echo $?
       0

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Shirisha G <shirisha@linux.ibm.com>
---
 tools/perf/tests/shell/daemon.sh | 113 ++++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 38 deletions(-)

diff --git a/tools/perf/tests/shell/daemon.sh b/tools/perf/tests/shell/daemon.sh
index 45fc24af5b07..aaf3849353e8 100755
--- a/tools/perf/tests/shell/daemon.sh
+++ b/tools/perf/tests/shell/daemon.sh
@@ -11,11 +11,16 @@ check_line_first()
 	local lock=$5
 	local up=$6
 
-	local line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
-	local line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
-	local line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
-	local line_lock=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
-	local line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
+	local line_name
+	line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
+	local line_base
+	line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
+	local line_output
+	line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
+	local line_lock
+	line_lock=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
+	local line_up
+	line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
 
 	if [ "${name}" != "${line_name}" ]; then
 		echo "FAILED: wrong name"
@@ -54,13 +59,20 @@ check_line_other()
 	local ack=$7
 	local up=$8
 
-	local line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
-	local line_run=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
-	local line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
-	local line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
-	local line_control=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
-	local line_ack=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $7 }'`
-	local line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $8 }'`
+	local line_name
+	line_name=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $2 }'`
+	local line_run
+	line_run=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $3 }'`
+	local line_base
+	line_base=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $4 }'`
+	local line_output
+	line_output=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $5 }'`
+	local line_control
+	line_control=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $6 }'`
+	local line_ack
+	line_ack=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $7 }'`
+	local line_up
+	line_up=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $8 }'`
 
 	if [ "${name}" != "${line_name}" ]; then
 		echo "FAILED: wrong name"
@@ -102,8 +114,10 @@ daemon_exit()
 {
 	local config=$1
 
-	local line=`perf daemon --config ${config} -x: | head -1`
-	local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+	local line
+	line=`perf daemon --config ${config} -x: | head -1`
+	local pid
+	pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
 
 	# Reset trap handler.
 	trap - SIGINT SIGTERM
@@ -123,7 +137,7 @@ daemon_start()
 	perf daemon start --config ${config}
 
 	# Clean up daemon if interrupted.
-	trap "echo 'FAILED: Signal caught'; daemon_exit ${config}; exit 1" SIGINT SIGTERM
+	trap 'echo "FAILED: Signal caught"; daemon_exit "${config}"; exit 1' SIGINT SIGTERM
 
 	# wait for the session to ping
 	local state="FAIL"
@@ -144,8 +158,10 @@ test_list()
 {
 	echo "test daemon list"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	cat <<EOF > ${config}
 [daemon]
@@ -165,19 +181,22 @@ EOF
 
 	# check first line
 	# pid:daemon:base:base/output:base/lock
-	local line=`perf daemon --config ${config} -x: | head -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -1`
 	check_line_first ${line} daemon ${base} ${base}/output ${base}/lock "0"
 
 	# check 1st session
 	# pid:size:-e cpu-clock:base/size:base/size/output:base/size/control:base/size/ack:0
-	local line=`perf daemon --config ${config} -x: | head -2 | tail -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -2 | tail -1`
 	check_line_other "${line}" size "-e cpu-clock -m 1 sleep 10" ${base}/session-size \
 			 ${base}/session-size/output ${base}/session-size/control \
 			 ${base}/session-size/ack "0"
 
 	# check 2nd session
 	# pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
-	local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -3 | tail -1`
 	check_line_other "${line}" time "-e task-clock -m 1 sleep 10" ${base}/session-time \
 			 ${base}/session-time/output ${base}/session-time/control \
 			 ${base}/session-time/ack "0"
@@ -193,8 +212,10 @@ test_reconfig()
 {
 	echo "test daemon reconfig"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
@@ -215,10 +236,12 @@ EOF
 
 	# check 2nd session
 	# pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
-	local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -3 | tail -1`
 	check_line_other "${line}" time "-e task-clock -m 1 sleep 10" ${base}/session-time \
 			 ${base}/session-time/output ${base}/session-time/control ${base}/session-time/ack "0"
-	local pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+	local pid
+	pid=`echo "${line}" | awk 'BEGIN { FS = ":" } ; { print $1 }'`
 
 	# prepare new config
 	local config_new=${config}.new
@@ -249,7 +272,8 @@ EOF
 
 	# check reconfigured 2nd session
 	# pid:time:-e task-clock:base/time:base/time/output:base/time/control:base/time/ack:0
-	local line=`perf daemon --config ${config} -x: | head -3 | tail -1`
+	local line
+	line=`perf daemon --config ${config} -x: | head -3 | tail -1`
 	check_line_other "${line}" time "-e cpu-clock -m 1 sleep 10" ${base}/session-time \
 			 ${base}/session-time/output ${base}/session-time/control ${base}/session-time/ack "0"
 
@@ -276,7 +300,8 @@ EOF
 		state=`perf daemon ping --config ${config} --session size | awk '{ print $1 }'`
 	done
 
-	local one=`perf daemon --config ${config} -x: | wc -l`
+	local one
+	one=`perf daemon --config ${config} -x: | wc -l`
 
 	if [ ${one} -ne "1" ]; then
 		echo "FAILED: wrong list output"
@@ -312,8 +337,10 @@ test_stop()
 {
 	echo "test daemon stop"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
@@ -332,8 +359,12 @@ EOF
 	# start daemon
 	daemon_start ${config} size
 
-	local pid_size=`perf daemon --config ${config} -x: | head -2 | tail -1 | awk 'BEGIN { FS = ":" } ; { print $1 }'`
-	local pid_time=`perf daemon --config ${config} -x: | head -3 | tail -1 | awk 'BEGIN { FS = ":" } ; { print $1 }'`
+	local pid_size
+	pid_size=`perf daemon --config ${config} -x: | head -2 | tail -1 |
+		  awk 'BEGIN { FS = ":" } ; { print $1 }'`
+	local pid_time
+	pid_time=`perf daemon --config ${config} -x: | head -3 | tail -1 |
+		  awk 'BEGIN { FS = ":" } ; { print $1 }'`
 
 	# check that sessions are running
 	if [ ! -d "/proc/${pid_size}" ]; then
@@ -364,8 +395,10 @@ test_signal()
 {
 	echo "test daemon signal"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
@@ -389,7 +422,7 @@ EOF
 	daemon_exit ${config}
 
 	# count is 2 perf.data for signals and 1 for perf record finished
-	count=`ls ${base}/session-test/ | grep perf.data | wc -l`
+	count=`ls ${base}/session-test/*perf.data* | wc -l`
 	if [ ${count} -ne 3 ]; then
 		error=1
 		echo "FAILED: perf data no generated"
@@ -403,8 +436,10 @@ test_ping()
 {
 	echo "test daemon ping"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
@@ -426,7 +461,7 @@ EOF
 	size=`perf daemon ping --config ${config} --session size | awk '{ print $1 }'`
 	type=`perf daemon ping --config ${config} --session time | awk '{ print $1 }'`
 
-	if [ ${size} != "OK" -o ${type} != "OK" ]; then
+	if [ ${size} != "OK" ] || [ ${type} != "OK" ]; then
 		error=1
 		echo "FAILED: daemon ping failed"
 	fi
@@ -442,8 +477,10 @@ test_lock()
 {
 	echo "test daemon lock"
 
-	local config=$(mktemp /tmp/perf.daemon.config.XXX)
-	local base=$(mktemp -d /tmp/perf.daemon.base.XXX)
+	local config
+	config=$(mktemp /tmp/perf.daemon.config.XXX)
+	local base
+	base=$(mktemp -d /tmp/perf.daemon.base.XXX)
 
 	# prepare config
 	cat <<EOF > ${config}
-- 
2.39.1

[PATCH 05/17] tools/perf/tests: Fix shellcheck warnings for stat+csv_output

[Athira Rajeev]

From: Korrapati Likhitha <likhitha@linux.ibm.com>

Running the shellcheck on stat+csv_output resulted in the
following warning.

Result with shellcheck  without patch:
=====
$ shellcheck -S warning stat+csv_output.sh

In stat+csv_output.sh line 23:
         [ $(uname -m) = "s390x" ] && exp='^[6-7]$'
           ^---------^ SC2046: Quote this to prevent word splitting.
In stat+csv_output.sh line 51:
[ $(id -u) != 0 ] && [ $(cat /proc/sys/kernel/perf_event_paranoid) -gt $1 ]
  ^------^ SC2046: Quote this to prevent word splitting.
                       ^-- SC2046: Quote this to prevent word splitting.
=====

Fixed the warning SC2046 by adding quotes to prevent word splitting.

Result with shellcheck with patch:
=====
$ shellcheck -S warning tests/shell/stat+csv_output.sh
$ ./perf test "stat CSV output linter"
 96: perf stat CSV output linter                                     : Ok
=====

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Korrapati Likhitha <likhitha@linux.ibm.com>
Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 tools/perf/tests/shell/stat+csv_output.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
index fb78b6251a4e..59607fb3cd98 100755
--- a/tools/perf/tests/shell/stat+csv_output.sh
+++ b/tools/perf/tests/shell/stat+csv_output.sh
@@ -35,7 +35,7 @@ function commachecker()
 	;; "--interval")	exp=7
 	;; "--per-thread")	exp=7
 	;; "--system-wide-no-aggr")	exp=7
-				[ $(uname -m) = "s390x" ] && exp='^[6-7]$'
+				[ "$(uname -m)" = "s390x" ] && exp='^[6-7]$'
 	;; "--per-core")	exp=8
 	;; "--per-socket")	exp=8
 	;; "--per-node")	exp=8
@@ -65,7 +65,7 @@ function commachecker()
 # Return true if perf_event_paranoid is > $1 and not running as root.
 function ParanoidAndNotRoot()
 {
-	 [ $(id -u) != 0 ] && [ $(cat /proc/sys/kernel/perf_event_paranoid) -gt $1 ]
+	 [ "$(id -u)" != 0 ] && [ "$(cat /proc/sys/kernel/perf_event_paranoid)" -gt $1 ]
 }
 
 check_no_args()
-- 
2.39.1

[PATCH 06/17] tools/perf/tests: Fix shellcheck warnings for trace+probe_vfs_getname.sh

[Athira Rajeev]

From: Akanksha J N <akanksha@linux.ibm.com>

Fix the shellcheck warnings on powerpc and x86 for testcase
trace+probe_vfs_getname.sh. Add quotes to prevent word splitting
which are caused by unquoted command expansions.

Before fix:

$ shellcheck -S warning trace+probe_vfs_getname.sh

	In trace+probe_vfs_getname.sh line 13:
	. $(dirname $0)/lib/probe.sh
	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.

	In trace+probe_vfs_getname.sh line 18:
	. $(dirname $0)/lib/probe_vfs_getname.sh
	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.

	In trace+probe_vfs_getname.sh line 21:
		evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
		            ^-- SC2046 (warning): Quote this to prevent word splitting.

	100: Check open filename arg using perf trace + vfs_getname          : Ok

After the fix:

$ shellcheck -S warning trace+probe_vfs_getname.sh

	100: Check open filename arg using perf trace + vfs_getname          : Ok

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Akanksha J N <akanksha@linux.ibm.com>
Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com>
Signed-off-by: Saket <skb99@linux.vnet.ibm.com>
Signed-off-by: Avnish Chouhan <avnish@linux.vnet.ibm.com>
---
 tools/perf/tests/shell/trace+probe_vfs_getname.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/trace+probe_vfs_getname.sh b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
index 0a4bac3dd77e..935eac7efa47 100755
--- a/tools/perf/tests/shell/trace+probe_vfs_getname.sh
+++ b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
@@ -10,15 +10,15 @@
 # SPDX-License-Identifier: GPL-2.0
 # Arnaldo Carvalho de Melo <acme@kernel.org>, 2017
 
-. $(dirname $0)/lib/probe.sh
+. "$(dirname $0)"/lib/probe.sh
 
 skip_if_no_perf_probe || exit 2
 skip_if_no_perf_trace || exit 2
 
-. $(dirname $0)/lib/probe_vfs_getname.sh
+. "$(dirname $0)"/lib/probe_vfs_getname.sh
 
 trace_open_vfs_getname() {
-	evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
+	evts=$(echo "$(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/')" | sed 's/ /,/')
 	perf trace -e $evts touch $file 2>&1 | \
 	grep -E " +[0-9]+\.[0-9]+ +\( +[0-9]+\.[0-9]+ ms\): +touch\/[0-9]+ open(at)?\((dfd: +CWD, +)?filename: +${file}, +flags: CREAT\|NOCTTY\|NONBLOCK\|WRONLY, +mode: +IRUGO\|IWUGO\) += +[0-9]+$"
 }
-- 
2.39.1

Re: [PATCH 06/17] tools/perf/tests: Fix shellcheck warnings for trace+probe_vfs_getname.sh

[Arnaldo Carvalho de Melo]

Em Tue, Jun 13, 2023 at 10:11:34PM +0530, Athira Rajeev escreveu:
> From: Akanksha J N <akanksha@linux.ibm.com>
> 
> Fix the shellcheck warnings on powerpc and x86 for testcase
> trace+probe_vfs_getname.sh. Add quotes to prevent word splitting
> which are caused by unquoted command expansions.
> 
> Before fix:
> 
> $ shellcheck -S warning trace+probe_vfs_getname.sh
> 
> 	In trace+probe_vfs_getname.sh line 13:
> 	. $(dirname $0)/lib/probe.sh
> 	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.
> 
> 	In trace+probe_vfs_getname.sh line 18:
> 	. $(dirname $0)/lib/probe_vfs_getname.sh
> 	  ^-----------^ SC2046 (warning): Quote this to prevent word splitting.
> 
> 	In trace+probe_vfs_getname.sh line 21:
> 		evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
> 		            ^-- SC2046 (warning): Quote this to prevent word splitting.
> 
> 	100: Check open filename arg using perf trace + vfs_getname          : Ok
> 
> After the fix:
> 
> $ shellcheck -S warning trace+probe_vfs_getname.sh
> 
> 	100: Check open filename arg using perf trace + vfs_getname          : Ok

So, I tried this on x86_64, fedora and get:

[root@quaco ~]# perf test "trace + vfs"
115: Check open filename arg using perf trace + vfs_getname          : FAILED!
[root@quaco ~]# 

Then, looking at the change:


> -	evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
> +	evts=$(echo "$(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/')" | sed 's/ /,/')

So, before:

[root@quaco ~]# evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
[root@quaco ~]# echo $evts
open,openat
[root@quaco ~]#

Then after:

[root@quaco ~]# evts=$(echo "$(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/')" | sed 's/ /,/')
[root@quaco ~]# echo $evts
open openat
[root@quaco ~]#

Not equivalent, so I'm removing this patch, please check and resubmit,
please.

- Arnaldo

> 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Signed-off-by: Akanksha J N <akanksha@linux.ibm.com>
> Signed-off-by: Abhishek Singh Tomar <abhishek@linux.ibm.com>
> Signed-off-by: Saket <skb99@linux.vnet.ibm.com>
> Signed-off-by: Avnish Chouhan <avnish@linux.vnet.ibm.com>
> ---
>  tools/perf/tests/shell/trace+probe_vfs_getname.sh | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/trace+probe_vfs_getname.sh b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> index 0a4bac3dd77e..935eac7efa47 100755
> --- a/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> +++ b/tools/perf/tests/shell/trace+probe_vfs_getname.sh
> @@ -10,15 +10,15 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Arnaldo Carvalho de Melo <acme@kernel.org>, 2017
>  
> -. $(dirname $0)/lib/probe.sh
> +. "$(dirname $0)"/lib/probe.sh
>  
>  skip_if_no_perf_probe || exit 2
>  skip_if_no_perf_trace || exit 2
>  
> -. $(dirname $0)/lib/probe_vfs_getname.sh
> +. "$(dirname $0)"/lib/probe_vfs_getname.sh
>  
>  trace_open_vfs_getname() {
> -	evts=$(echo $(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/') | sed 's/ /,/')
> +	evts=$(echo "$(perf list syscalls:sys_enter_open* 2>/dev/null | grep -E 'open(at)? ' | sed -r 's/.*sys_enter_([a-z]+) +\[.*$/\1/')" | sed 's/ /,/')
>  	perf trace -e $evts touch $file 2>&1 | \
>  	grep -E " +[0-9]+\.[0-9]+ +\( +[0-9]+\.[0-9]+ ms\): +touch\/[0-9]+ open(at)?\((dfd: +CWD, +)?filename: +${file}, +flags: CREAT\|NOCTTY\|NONBLOCK\|WRONLY, +mode: +IRUGO\|IWUGO\) += +[0-9]+$"
>  }
> -- 
> 2.39.1
> 

-- 

- Arnaldo

[PATCH 07/17] perf/tests/shell : Shellcheck fixes for perf test "test_arm_coresight.sh"

[Athira Rajeev]

From: Anushree Mathur <anushree.mathur@linux.vnet.ibm.com>

Fixed the following shellcheck issues in test_arm_coresight.sh file:

In tools/perf/tests/shell/test_arm_coresight.sh line 31:
        trap - exit term int
               ^--^ SC2039: In POSIX sh, using lower/mixed case for signal names is undefined.
                    ^--^ SC2039: In POSIX sh, using lower/mixed case for signal names is undefined.
                         ^-^ SC2039: In POSIX sh, using lower/mixed case for signal names is undefined.

In tools/perf/tests/shell/test_arm_coresight.sh line 35:
trap cleanup_files exit term int
                   ^--^ SC2039: In POSIX sh, using lower/mixed case for signal names is undefined.
                        ^--^ SC2039: In POSIX sh, using lower/mixed case for signal names is undefined.
                             ^-^ SC2039: In POSIX sh, using lower/mixed case for signal names is undefined.

In tools/perf/tests/shell/test_arm_coresight.sh line 92:
        if [ $? -eq 0 -a -e "$1/enable_sink" ]; then
                      ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

Fixed above warnings by:
1)Capitalize signals(INT, TERM, EXIT) to avoid mixed/lower case naming of
signals.
2)Expression [p -a q] was not defined,changed it to [p] && [q] to avoid the
ambiguity as this is older format using -a or -o ,now we use [p] && [q] in
place of [p -a q] and [p] || [q] in place of [p -o q].

Result after fixing the issues:

shell$ shellcheck -S warning test_arm_coresight.sh
shell$

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Anushree Mathur <anushree.mathur@linux.vnet.ibm.com>
---
 tools/perf/tests/shell/test_arm_coresight.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
index 482009e17bda..f1bf5621160f 100755
--- a/tools/perf/tests/shell/test_arm_coresight.sh
+++ b/tools/perf/tests/shell/test_arm_coresight.sh
@@ -28,11 +28,11 @@ cleanup_files()
 	rm -f ${perfdata}
 	rm -f ${file}
 	rm -f "${perfdata}.old"
-	trap - exit term int
+	trap - EXIT TERM INT
 	exit $glb_err
 }
 
-trap cleanup_files exit term int
+trap cleanup_files EXIT TERM INT
 
 record_touch_file() {
 	echo "Recording trace (only user mode) with path: CPU$2 => $1"
@@ -89,7 +89,7 @@ is_device_sink() {
 	# cannot support perf PMU.
 	echo "$1" | grep -E -q -v "tpiu"
 
-	if [ $? -eq 0 -a -e "$1/enable_sink" ]; then
+	if [ $? -eq 0 ] && [ -e "$1/enable_sink" ]; then
 
 		pmu_dev="/sys/bus/event_source/devices/cs_etm/sinks/$2"
 
-- 
2.39.1

[PATCH 08/17] tools/perf/tests/shell/stat_all_metrics: Fix shellcheck warning SC2076 in stat_all_metrics.sh

[Athira Rajeev]

From: Barnali Guha Thakurata <barnali@linux.ibm.com>

Fixed shellcheck warning SC2076 in stat_all_metrics.sh.

Before the patch:
shell$ shellcheck stat_all_metrics.sh

In stat_all_metrics.sh line 9:
  if [[ "$result" =~ "${m:0:50}" ]] || [[ "$result" =~ "<not supported>" ]]
                     ^---------^ SC2076: Don't quote right-hand
side of =~, it'll match literally rather than as a regex.

In stat_all_metrics.sh line 15:
  if [[ "$result" =~ "${m:0:50}" ]]
                     ^---------^ SC2076: Don't quote right-hand
side of =~, it'll match literally rather than as a regex.

In stat_all_metrics.sh line 22:
  if [[ "$result" =~ "${m:0:50}" ]]
                     ^---------^ SC2076: Don't quote right-hand
side of =~, it'll match literally rather than as a regex.

After the patch:
shell$ shellcheck stat_all_metrics.sh
shell$

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Barnali Guha Thakurata <barnali@linux.ibm.com>
---
 tools/perf/tests/shell/stat_all_metrics.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/stat_all_metrics.sh b/tools/perf/tests/shell/stat_all_metrics.sh
index 22e9cb294b40..54774525e18a 100755
--- a/tools/perf/tests/shell/stat_all_metrics.sh
+++ b/tools/perf/tests/shell/stat_all_metrics.sh
@@ -6,20 +6,20 @@ err=0
 for m in $(perf list --raw-dump metrics); do
   echo "Testing $m"
   result=$(perf stat -M "$m" true 2>&1)
-  if [[ "$result" =~ "${m:0:50}" ]] || [[ "$result" =~ "<not supported>" ]]
+  if [[ "$result" =~ ${m:0:50} ]] || [[ "$result" =~ "<not supported>" ]]
   then
     continue
   fi
   # Failed so try system wide.
   result=$(perf stat -M "$m" -a sleep 0.01 2>&1)
-  if [[ "$result" =~ "${m:0:50}" ]]
+  if [[ "$result" =~ ${m:0:50} ]]
   then
     continue
   fi
   # Failed again, possibly the workload was too small so retry with something
   # longer.
   result=$(perf stat -M "$m" perf bench internals synthesize 2>&1)
-  if [[ "$result" =~ "${m:0:50}" ]]
+  if [[ "$result" =~ ${m:0:50} ]]
   then
     continue
   fi
-- 
2.39.1

[PATCH 09/17] tools/perf/tests: Fix shellcheck issues in test_task_analyzer.sh file

[Athira Rajeev]

From: Aboorva Devarajan <aboorvad@linux.vnet.ibm.com>

Fixed the following shellcheck issues in test_task_analyzer.sh file:

SC2086: Double quote to prevent globbing and word splitting
warnings in shell-check.
Fixes the following shellcheck issues,

SC2086: Double quote to prevent globbing and word splitting
warnings in shell-check.

Before Patch:

$ shellcheck ./test_task_analyzer.sh  | grep "SC2086" | ...
In ./test_task_analyzer.sh line 13:
SC2086: Double quote to prevent globbing and word splitting.
In ./test_task_analyzer.sh line 24:
SC2086: Double quote to prevent globbing and word splitting.
In ./test_task_analyzer.sh line 39:
SC2086: Double quote to prevent globbing and word splitting.

After Patch:
$ shellcheck ./test_task_analyzer.sh  | grep -i "SC2086"
None

perf test result after patch:

PASS: "test_basic"
PASS: "test_ns_rename"
PASS: "test_ms_filtertasks_highlight"
PASS: "test_extended_times_timelimit_limittasks"
PASS: "test_summary"
PASS: "test_summaryextended"
PASS: "test_summaryonly"
PASS: "test_extended_times_summary_ns"
PASS: "test_extended_times_summary_ns"
PASS: "test_csv"
PASS: "test_csvsummary"
PASS: "test_csv_extended_times"
PASS: "test_csvsummary_extended"

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Aboorva Devarajan <aboorvad@linux.vnet.ibm.com>
---
 tools/perf/tests/shell/test_task_analyzer.sh | 54 ++++++++++----------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/tools/perf/tests/shell/test_task_analyzer.sh b/tools/perf/tests/shell/test_task_analyzer.sh
index a98e4ab66040..4264b54b654b 100755
--- a/tools/perf/tests/shell/test_task_analyzer.sh
+++ b/tools/perf/tests/shell/test_task_analyzer.sh
@@ -10,7 +10,7 @@ cleanup() {
   rm -f perf.data.old
   rm -f csv
   rm -f csvsummary
-  rm -rf $tmpdir
+  rm -rf "$tmpdir"
   trap - exit term int
 }
 
@@ -21,7 +21,7 @@ trap_cleanup() {
 trap trap_cleanup exit term int
 
 report() {
-	if [ $1 = 0 ]; then
+	if [ "$1" = 0 ]; then
 		echo "PASS: \"$2\""
 	else
 		echo "FAIL: \"$2\" Error message: \"$3\""
@@ -36,11 +36,11 @@ check_exec_0() {
 }
 
 find_str_or_fail() {
-	grep -q "$1" $2
-	if [ $? != 0 ]; then
-		report 1 $3 "Failed to find required string:'${1}'."
+	grep -q "$1" "$2"
+	if [ "$?" != 0 ]; then
+		report 1 "$3" "Failed to find required string:'${1}'."
 	else
-		report 0 $3
+		report 0 "$3"
 	fi
 }
 
@@ -52,86 +52,86 @@ prepare_perf_data() {
 # check standard inkvokation with no arguments
 test_basic() {
 	out="$tmpdir/perf.out"
-	perf script report task-analyzer > $out
+	perf script report task-analyzer > "$out"
 	check_exec_0 "perf"
-	find_str_or_fail "Comm" $out ${FUNCNAME[0]}
+	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
 }
 
 test_ns_rename(){
 	out="$tmpdir/perf.out"
-	perf script report task-analyzer --ns --rename-comms-by-tids 0:random > $out
+	perf script report task-analyzer --ns --rename-comms-by-tids 0:random > "$out"
 	check_exec_0 "perf"
-	find_str_or_fail "Comm" $out ${FUNCNAME[0]}
+	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
 }
 
 test_ms_filtertasks_highlight(){
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --ms --filter-tasks perf --highlight-tasks perf \
-	> $out
+	> "$out"
 	check_exec_0 "perf"
-	find_str_or_fail "Comm" $out ${FUNCNAME[0]}
+	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
 }
 
 test_extended_times_timelimit_limittasks() {
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --extended-times --time-limit :99999 \
-	--limit-to-tasks perf > $out
+	--limit-to-tasks perf > "$out"
 	check_exec_0 "perf"
-	find_str_or_fail "Out-Out" $out ${FUNCNAME[0]}
+	find_str_or_fail "Out-Out" "$out" "${FUNCNAME[0]}"
 }
 
 test_summary() {
 	out="$tmpdir/perf.out"
-	perf script report task-analyzer --summary > $out
+	perf script report task-analyzer --summary > "$out"
 	check_exec_0 "perf"
-	find_str_or_fail "Summary" $out ${FUNCNAME[0]}
+	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
 }
 
 test_summaryextended() {
 	out="$tmpdir/perf.out"
-	perf script report task-analyzer --summary-extended > $out
+	perf script report task-analyzer --summary-extended > "$out"
 	check_exec_0 "perf"
-	find_str_or_fail "Inter Task Times" $out ${FUNCNAME[0]}
+	find_str_or_fail "Inter Task Times" "$out" "${FUNCNAME[0]}"
 }
 
 test_summaryonly() {
 	out="$tmpdir/perf.out"
-	perf script report task-analyzer --summary-only > $out
+	perf script report task-analyzer --summary-only > "$out"
 	check_exec_0 "perf"
-	find_str_or_fail "Summary" $out ${FUNCNAME[0]}
+	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
 }
 
 test_extended_times_summary_ns() {
 	out="$tmpdir/perf.out"
-	perf script report task-analyzer --extended-times --summary --ns > $out
+	perf script report task-analyzer --extended-times --summary --ns > "$out"
 	check_exec_0 "perf"
-	find_str_or_fail "Out-Out" $out ${FUNCNAME[0]}
-	find_str_or_fail "Summary" $out ${FUNCNAME[0]}
+	find_str_or_fail "Out-Out" "$out" "${FUNCNAME[0]}"
+	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
 }
 
 test_csv() {
 	perf script report task-analyzer --csv csv > /dev/null
 	check_exec_0 "perf"
-	find_str_or_fail "Comm;" csv ${FUNCNAME[0]}
+	find_str_or_fail "Comm;" csv "${FUNCNAME[0]}"
 }
 
 test_csv_extended_times() {
 	perf script report task-analyzer --csv csv --extended-times > /dev/null
 	check_exec_0 "perf"
-	find_str_or_fail "Out-Out;" csv ${FUNCNAME[0]}
+	find_str_or_fail "Out-Out;" csv "${FUNCNAME[0]}"
 }
 
 test_csvsummary() {
 	perf script report task-analyzer --csv-summary csvsummary > /dev/null
 	check_exec_0 "perf"
-	find_str_or_fail "Comm;" csvsummary ${FUNCNAME[0]}
+	find_str_or_fail "Comm;" csvsummary "${FUNCNAME[0]}"
 }
 
 test_csvsummary_extended() {
 	perf script report task-analyzer --csv-summary csvsummary --summary-extended \
 	>/dev/null
 	check_exec_0 "perf"
-	find_str_or_fail "Out-Out;" csvsummary ${FUNCNAME[0]}
+	find_str_or_fail "Out-Out;" csvsummary "${FUNCNAME[0]}"
 }
 
 prepare_perf_data
-- 
2.39.1

[PATCH 10/17] tools/perf/tests: fix test_arm_spe.sh signal case issues

[Athira Rajeev]

From: Abhirup Deb <abhirupdeb@linux.vnet.ibm.com>

Running shellcheck -S on test_arm_spe.sh throws below warnings:

 #shellcheck -S warning tests/shell/test_arm_spe.sh
In tests/shell/test_arm_spe.sh line 30:
trap cleanup_files exit term int
                   ^--^ SC3049 (warning): In POSIX sh, using lower/mixed case for signal names is undefined.
                        ^--^ SC3049 (warning): In POSIX sh, using lower/mixed case for signal names is undefined.
                             ^-^ SC3049 (warning): In POSIX sh, using lower/mixed case for signal names is undefined.

Fixed this issue by using uppercase for "EXIT", "TERM" and
"INIT" signals to avoid using lower/mixed case for signal
names as input.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Abhirup Deb <abhirupdeb@linux.vnet.ibm.com>
Signed-off-by: Ojaswin Mujoo <ojaswin.mujoo@ibm.com>
Signed-off-by: Piyush Sachdeva <Piyush.Sachdeva@ibm.com>
Signed-off-by: Mukesh Chaurasiya <mukesh.chaurasiya@ibm.com>
---
 tools/perf/tests/shell/test_arm_spe.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/test_arm_spe.sh b/tools/perf/tests/shell/test_arm_spe.sh
index aa094d71f5b4..03d5c7d12ee5 100755
--- a/tools/perf/tests/shell/test_arm_spe.sh
+++ b/tools/perf/tests/shell/test_arm_spe.sh
@@ -27,7 +27,7 @@ cleanup_files()
 	exit $glb_err
 }
 
-trap cleanup_files exit term int
+trap cleanup_files EXIT TERM INT
 
 arm_spe_report() {
 	if [ $2 = 0 ]; then
-- 
2.39.1

[PATCH 11/17] perf/tests/shell: fix shellscript errors for lock_contention.sh

[Athira Rajeev]

From: Abhirup Deb <abhirupdeb@linux.vnet.ibm.com>

Use quotes around variables to prevent POSIX word expansion,
use uppercase for signals(INT, TERM, EXIT) to avoid mixed/lower case
naming of signals and replace "==" with "=" as "==" is not
supported by POSIX shell.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Abhirup Deb <abhirupdeb@linux.vnet.ibm.com>
Signed-off-by: Anushree Mathur <anushree.mathur2@ibm.com>
---
 tools/perf/tests/shell/lock_contention.sh | 70 +++++++++++------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/tools/perf/tests/shell/lock_contention.sh b/tools/perf/tests/shell/lock_contention.sh
index be5fcafb26aa..f2cc187b6186 100755
--- a/tools/perf/tests/shell/lock_contention.sh
+++ b/tools/perf/tests/shell/lock_contention.sh
@@ -11,14 +11,14 @@ result=$(mktemp /tmp/__perf_test.result.XXXXX)
 cleanup() {
 	rm -f ${perfdata}
 	rm -f ${result}
-	trap - exit term int
+	trap - EXIT TERM INT
 }
 
 trap_cleanup() {
 	cleanup
 	exit ${err}
 }
-trap trap_cleanup exit term int
+trap trap_cleanup EXIT TERM INT
 
 check() {
 	if [ `id -u` != 0 ]; then
@@ -40,8 +40,8 @@ test_record()
 	perf lock record -o ${perfdata} -- perf bench sched messaging > /dev/null 2>&1
 	# the output goes to the stderr and we expect only 1 output (-E 1)
 	perf lock contention -i ${perfdata} -E 1 -q 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] Recorded result count is not 1:" $(cat "${result}" | wc -l)
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] Recorded result count is not 1:" "$(cat "${result}" | wc -l)"
 		err=1
 		exit
 	fi
@@ -58,8 +58,8 @@ test_bpf()
 
 	# the perf lock contention output goes to the stderr
 	perf lock con -a -b -E 1 -q -- perf bench sched messaging > /dev/null 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] BPF result count is not 1:" $(cat "${result}" | wc -l)
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] BPF result count is not 1:" "$(cat "${result}" | wc -l)"
 		err=1
 		exit
 	fi
@@ -70,8 +70,8 @@ test_record_concurrent()
 	echo "Testing perf lock record and perf lock contention at the same time"
 	perf lock record -o- -- perf bench sched messaging 2> /dev/null | \
 	perf lock contention -i- -E 1 -q 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] Recorded result count is not 1:" $(cat "${result}" | wc -l)
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] Recorded result count is not 1:" "$(cat "${result}" | wc -l)"
 		err=1
 		exit
 	fi
@@ -81,8 +81,8 @@ test_aggr_task()
 {
 	echo "Testing perf lock contention --threads"
 	perf lock contention -i ${perfdata} -t -E 1 -q 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] Recorded result count is not 1:" $(cat "${result}" | wc -l)
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] Recorded result count is not 1:" "$(cat "${result}" | wc -l)"
 		err=1
 		exit
 	fi
@@ -93,8 +93,8 @@ test_aggr_task()
 
 	# the perf lock contention output goes to the stderr
 	perf lock con -a -b -t -E 1 -q -- perf bench sched messaging > /dev/null 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] BPF result count is not 1:" $(cat "${result}" | wc -l)
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] BPF result count is not 1:" "$(cat "${result}" | wc -l)"
 		err=1
 		exit
 	fi
@@ -104,8 +104,8 @@ test_aggr_addr()
 {
 	echo "Testing perf lock contention --lock-addr"
 	perf lock contention -i ${perfdata} -l -E 1 -q 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] Recorded result count is not 1:" $(cat "${result}" | wc -l)
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] Recorded result count is not 1:" "$(cat "${result}" | wc -l)"
 		err=1
 		exit
 	fi
@@ -116,8 +116,8 @@ test_aggr_addr()
 
 	# the perf lock contention output goes to the stderr
 	perf lock con -a -b -l -E 1 -q -- perf bench sched messaging > /dev/null 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] BPF result count is not 1:" $(cat "${result}" | wc -l)
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] BPF result count is not 1:" "$(cat "${result}" | wc -l)"
 		err=1
 		exit
 	fi
@@ -127,8 +127,8 @@ test_type_filter()
 {
 	echo "Testing perf lock contention --type-filter (w/ spinlock)"
 	perf lock contention -i ${perfdata} -Y spinlock -q 2> ${result}
-	if [ $(grep -c -v spinlock "${result}") != "0" ]; then
-		echo "[Fail] Recorded result should not have non-spinlocks:" $(cat "${result}")
+	if [ "$(grep -c -v spinlock "${result}")" != "0" ]; then
+		echo "[Fail] Recorded result should not have non-spinlocks:" "$(cat "${result}")"
 		err=1
 		exit
 	fi
@@ -138,8 +138,8 @@ test_type_filter()
 	fi
 
 	perf lock con -a -b -Y spinlock -q -- perf bench sched messaging > /dev/null 2> ${result}
-	if [ $(grep -c -v spinlock "${result}") != "0" ]; then
-		echo "[Fail] BPF result should not have non-spinlocks:" $(cat "${result}")
+	if [ "$(grep -c -v spinlock "${result}")" != "0" ]; then
+		echo "[Fail] BPF result should not have non-spinlocks:" "$(cat "${result}")"
 		err=1
 		exit
 	fi
@@ -149,7 +149,7 @@ test_lock_filter()
 {
 	echo "Testing perf lock contention --lock-filter (w/ tasklist_lock)"
 	perf lock contention -i ${perfdata} -l -q 2> ${result}
-	if [ $(grep -c tasklist_lock "${result}") != "1" ]; then
+	if [ "$(grep -c tasklist_lock "${result}")" != "1" ]; then
 		echo "[Skip] Could not find 'tasklist_lock'"
 		return
 	fi
@@ -159,8 +159,8 @@ test_lock_filter()
 	# find out the type of tasklist_lock
 	local type=$(head -1 "${result}" | awk '{ print $8 }' | sed -e 's/:.*//')
 
-	if [ $(grep -c -v "${type}" "${result}") != "0" ]; then
-		echo "[Fail] Recorded result should not have non-${type} locks:" $(cat "${result}")
+	if [ "$(grep -c -v "${type}" "${result}")" != "0" ]; then
+		echo "[Fail] Recorded result should not have non-${type} locks:" "$(cat "${result}")"
 		err=1
 		exit
 	fi
@@ -170,8 +170,8 @@ test_lock_filter()
 	fi
 
 	perf lock con -a -b -L tasklist_lock -q -- perf bench sched messaging > /dev/null 2> ${result}
-	if [ $(grep -c -v "${type}" "${result}") != "0" ]; then
-		echo "[Fail] BPF result should not have non-${type} locks:" $(cat "${result}")
+	if [ "$(grep -c -v "${type}" "${result}")" != "0" ]; then
+		echo "[Fail] BPF result should not have non-${type} locks:" "$(cat "${result}")"
 		err=1
 		exit
 	fi
@@ -181,14 +181,14 @@ test_stack_filter()
 {
 	echo "Testing perf lock contention --callstack-filter (w/ unix_stream)"
 	perf lock contention -i ${perfdata} -v -q 2> ${result}
-	if [ $(grep -c unix_stream "${result}") == "0" ]; then
+	if [ "$(grep -c unix_stream "${result}")" = "0" ]; then
 		echo "[Skip] Could not find 'unix_stream'"
 		return
 	fi
 
 	perf lock contention -i ${perfdata} -E 1 -S unix_stream -q 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] Recorded result should have a lock from unix_stream:" $(cat "${result}")
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] Recorded result should have a lock from unix_stream:" "$(cat "${result}")"
 		err=1
 		exit
 	fi
@@ -198,8 +198,8 @@ test_stack_filter()
 	fi
 
 	perf lock con -a -b -S unix_stream -E 1 -q -- perf bench sched messaging > /dev/null 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] BPF result should have a lock from unix_stream:" $(cat "${result}")
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] BPF result should have a lock from unix_stream:" "$(cat "${result}")"
 		err=1
 		exit
 	fi
@@ -209,14 +209,14 @@ test_aggr_task_stack_filter()
 {
 	echo "Testing perf lock contention --callstack-filter with task aggregation"
 	perf lock contention -i ${perfdata} -v -q 2> ${result}
-	if [ $(grep -c unix_stream "${result}") == "0" ]; then
+	if [ "$(grep -c unix_stream "${result}")" = "0" ]; then
 		echo "[Skip] Could not find 'unix_stream'"
 		return
 	fi
 
 	perf lock contention -i ${perfdata} -t -E 1 -S unix_stream -q 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] Recorded result should have a task from unix_stream:" $(cat "${result}")
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] Recorded result should have a task from unix_stream:" "$(cat "${result}")"
 		err=1
 		exit
 	fi
@@ -226,8 +226,8 @@ test_aggr_task_stack_filter()
 	fi
 
 	perf lock con -a -b -t -S unix_stream -E 1 -q -- perf bench sched messaging > /dev/null 2> ${result}
-	if [ $(cat "${result}" | wc -l) != "1" ]; then
-		echo "[Fail] BPF result should have a task from unix_stream:" $(cat "${result}")
+	if [ "$(cat "${result}" | wc -l)" != "1" ]; then
+		echo "[Fail] BPF result should have a task from unix_stream:" "$(cat "${result}")"
 		err=1
 		exit
 	fi
-- 
2.39.1

[PATCH 12/17] tools/perf/tests: fixed shellcheck warnings for perf shell scripts

[Athira Rajeev]

From: Samir Mulani <samir@linux.vnet.ibm.com>

Fixed the shellcheck warnings in buildid.sh, record+probe_libc_inet_pton.sh
and record+script_probe_vfs_getname.sh perf shell scripts:

1. Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
2. Prefer [ p ] || [ q ] as [ p -o q ] is not well defined.
3. Used * argument to avoid the argument mixes string and array
4. Resolved issue for variable refernce, where the variable is
   being used before it has been initialized.
5. Resolved word splitting issue (syntax error).
6. The "err" variable has been removed from buildid.sh since
   it is not used anywhere in the code.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Samir Mulani <samir@linux.vnet.ibm.com>
---
 tools/perf/tests/shell/buildid.sh                    | 12 ++++++------
 .../perf/tests/shell/record+probe_libc_inet_pton.sh  |  6 +++---
 .../tests/shell/record+script_probe_vfs_getname.sh   |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/perf/tests/shell/buildid.sh b/tools/perf/tests/shell/buildid.sh
index 0ce22ea0a7f1..3383ca3399d4 100755
--- a/tools/perf/tests/shell/buildid.sh
+++ b/tools/perf/tests/shell/buildid.sh
@@ -83,12 +83,12 @@ check()
 	# in case of pe-file.exe file
 	echo $1 | grep ".exe"
 	if [ $? -eq 0 ]; then
-		if [ -x $1  -a ! -x $file ]; then
+		if [ -x $1 ] && [ ! -x $file ]; then
 			echo "failed: file ${file} executable does not exist"
 			exit 1
 		fi
 
-		if [ ! -x $file -a ! -e $file ]; then
+		if [ ! -x $file ] && [ ! -e $file ]; then
 			echo "failed: file ${file} does not exist"
 			exit 1
 		fi
@@ -136,10 +136,10 @@ test_record()
 	log_err=$(mktemp /tmp/perf.log.err.XXX)
 	perf="perf --buildid-dir ${build_id_dir}"
 
-	echo "running: perf record $@"
-	${perf} record --buildid-all -o ${data} $@ 1>${log_out} 2>${log_err}
+	echo "running: perf record $*"
+	${perf} record --buildid-all -o ${data} "$@" 1>${log_out} 2>${log_err}
 	if [ $? -ne 0 ]; then
-		echo "failed: record $@"
+		echo "failed: record $*"
 		echo "see log: ${log_err}"
 		exit 1
 	fi
@@ -172,4 +172,4 @@ if [ ${run_pe} -eq 1 ]; then
 	rm -r ${wineprefix}
 fi
 
-exit ${err}
+exit 0
diff --git a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
index bbb5b3d185fa..0934fb0cd68f 100755
--- a/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
+++ b/tools/perf/tests/shell/record+probe_libc_inet_pton.sh
@@ -10,8 +10,8 @@
 # SPDX-License-Identifier: GPL-2.0
 # Arnaldo Carvalho de Melo <acme@kernel.org>, 2017
 
-. $(dirname $0)/lib/probe.sh
-. $(dirname $0)/lib/probe_vfs_getname.sh
+. "$(dirname "$0")/lib/probe.sh"
+. "$(dirname "$0")/lib/probe_vfs_getname.sh"
 
 libc=$(grep -w libc /proc/self/maps | head -1 | sed -r 's/.*[[:space:]](\/.*)/\1/g')
 nm -Dg $libc 2>/dev/null | fgrep -q inet_pton || exit 254
@@ -23,7 +23,7 @@ add_libc_inet_pton_event() {
 	event_name=$(perf probe -f -x $libc -a inet_pton 2>&1 | tail -n +2 | head -n -5 | \
 			grep -P -o "$event_pattern(?=[[:space:]]\(on inet_pton in $libc\))")
 
-	if [ $? -ne 0 -o -z "$event_name" ] ; then
+	if [ $? -ne 0 ] || [ -z "$event_name" ] ; then
 		printf "FAIL: could not add event\n"
 		return 1
 	fi
diff --git a/tools/perf/tests/shell/record+script_probe_vfs_getname.sh b/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
index 1341437e1bd9..7f664f1889d9 100755
--- a/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
+++ b/tools/perf/tests/shell/record+script_probe_vfs_getname.sh
@@ -9,11 +9,11 @@
 # SPDX-License-Identifier: GPL-2.0
 # Arnaldo Carvalho de Melo <acme@kernel.org>, 2017
 
-. $(dirname $0)/lib/probe.sh
+. "$(dirname "$0")/lib/probe.sh"
 
 skip_if_no_perf_probe || exit 2
 
-. $(dirname $0)/lib/probe_vfs_getname.sh
+. "$(dirname "$0")/lib/probe_vfs_getname.sh"
 
 record_open_file() {
 	echo "Recording open file:"
-- 
2.39.1

[PATCH 13/17] tools/perf/tests: Fix all POSIX sh warnings in perf shell test test_brstack.sh

[Athira Rajeev]

From: Geetika <geetika@linux.ibm.com>

Fix all the POSIX sh warnings in perf shell test test_brstack.sh
Warnings fixed :
* In POSIX sh, using lower/mixed case for signal names is undefined.
Correcting that in this script.
* In POSIX sh, 'local' is undefined.
local is supported in many shells, but it's not in POSIX.
In POSIX sh, you can adopt some convention to avoid accidentally
overwriting variables names, e.g. prefixing with the function name,
that is what I have done here.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Geetika <geetika@linux.ibm.com>
---
 tools/perf/tests/shell/test_brstack.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/shell/test_brstack.sh b/tools/perf/tests/shell/test_brstack.sh
index 1c49d8293003..09908d71c994 100755
--- a/tools/perf/tests/shell/test_brstack.sh
+++ b/tools/perf/tests/shell/test_brstack.sh
@@ -18,7 +18,7 @@ cleanup() {
 	rm -rf $TMPDIR
 }
 
-trap cleanup exit term int
+trap cleanup EXIT TERM INT
 
 test_user_branches() {
 	echo "Testing user branch stack sampling"
@@ -47,17 +47,17 @@ test_user_branches() {
 # first argument <arg0> is the argument passed to "--branch-stack <arg0>,save_type,u"
 # second argument are the expected branch types for the given filter
 test_filter() {
-	local filter=$1
-	local expect=$2
+	test_filter_filter=$1
+	test_filter_expect=$2
 
-	echo "Testing branch stack filtering permutation ($filter,$expect)"
+	echo "Testing branch stack filtering permutation ($test_filter_filter,$test_filter_expect)"
 
-	perf record -o $TMPDIR/perf.data --branch-filter $filter,save_type,u -- ${TESTPROG} > /dev/null 2>&1
+	perf record -o $TMPDIR/perf.data --branch-filter $test_filter_filter,save_type,u -- ${TESTPROG} > /dev/null 2>&1
 	perf script -i $TMPDIR/perf.data --fields brstack | xargs -n1 > $TMPDIR/perf.script
 
 	# fail if we find any branch type that doesn't match any of the expected ones
 	# also consider UNKNOWN branch types (-)
-	if grep -E -vm1 "^[^ ]*/($expect|-|( *))/.*$" $TMPDIR/perf.script; then
+	if grep -E -vm1 "^[^ ]*/($test_filter_expect|-|( *))/.*$" $TMPDIR/perf.script; then
 		return 1
 	fi
 }
-- 
2.39.1

[PATCH 14/17] tools/perf/tests: Fix all POSIX sh warnings in stat+shadow_stat.sh

[Athira Rajeev]

From: Spoorthy S <spoorts2@in.ibm.com>

Running shellcheck -S on stat+shadow_stat.sh testcase, generates
SC2046 and SC2034 warnings,

$ shellcheck -S warning tests/shell/stat+shadow_stat.sh
	res=`printf "%.2f" $(echo "scale=6; $num / $cyc" | bc -q)`
			   : Quote this to prevent word splitting

To address the POSIX shell warnings used quotes in the printf
expressions, to prevent word splitting.

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Spoorthy S <spoorts2@in.ibm.com>
---
 tools/perf/tests/shell/stat+shadow_stat.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/shell/stat+shadow_stat.sh b/tools/perf/tests/shell/stat+shadow_stat.sh
index e6e35fc6c882..0e9cba84e757 100755
--- a/tools/perf/tests/shell/stat+shadow_stat.sh
+++ b/tools/perf/tests/shell/stat+shadow_stat.sh
@@ -33,7 +33,7 @@ test_global_aggr()
 		fi
 
 		# use printf for rounding and a leading zero
-		res=`printf "%.2f" $(echo "scale=6; $num / $cyc" | bc -q)`
+		res=`printf "%.2f" "$(echo "scale=6; $num / $cyc" | bc -q)"`
 		if [ "$ipc" != "$res" ]; then
 			echo "IPC is different: $res != $ipc  ($num / $cyc)"
 			exit 1
@@ -67,7 +67,7 @@ test_no_aggr()
 		fi
 
 		# use printf for rounding and a leading zero
-		res=`printf "%.2f" $(echo "scale=6; $num / $cyc" | bc -q)`
+		res=`printf "%.2f" "$(echo "scale=6; $num / $cyc" | bc -q)"`
 		if [ "$ipc" != "$res" ]; then
 			echo "IPC is different for $cpu: $res != $ipc  ($num / $cyc)"
 			exit 1
-- 
2.39.1

[PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Athira Rajeev]

From: Aditya Gupta <adityag@linux.ibm.com>

${$1} gives bad substitution error on sh, bash, and zsh. This seems like
a typo, and this patch modifies it to $1, since that is what it's usage
looks like from wherever `check_exec_0` is called.

This issue due to ${$1} caused all function calls to give error in
`find_str_or_fail` line, and so no test runs completely. But
'perf test "perf script task-analyzer tests"' wrongly reports
that tests passed with the status OK, which is wrong considering
the tests didn't even run completely

Fixes: e8478b84d6ba ("perf test: add new task-analyzer tests")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/tests/shell/test_task_analyzer.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/test_task_analyzer.sh b/tools/perf/tests/shell/test_task_analyzer.sh
index 4264b54b654b..84ab7e7f57d5 100755
--- a/tools/perf/tests/shell/test_task_analyzer.sh
+++ b/tools/perf/tests/shell/test_task_analyzer.sh
@@ -31,7 +31,7 @@ report() {
 
 check_exec_0() {
 	if [ $? != 0 ]; then
-		report 1 "invokation of ${$1} command failed"
+		report 1 "invocation of $1 command failed"
 	fi
 }
 
-- 
2.39.1

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Arnaldo Carvalho de Melo]

Em Tue, Jun 13, 2023 at 10:11:43PM +0530, Athira Rajeev escreveu:
> From: Aditya Gupta <adityag@linux.ibm.com>
> 
> ${$1} gives bad substitution error on sh, bash, and zsh. This seems like
> a typo, and this patch modifies it to $1, since that is what it's usage
> looks like from wherever `check_exec_0` is called.

Nicely spotted!

Please add the people that last touched the problem to the cc list,
specially when it fixes a bug.

Thanks for adding a Fixes tag, that helps the stable@kernel.org guys to
get this propagated to supported kernel releases.

I've added the test author to the CC list in this message.

thanks!

- Arnaldo
 
> This issue due to ${$1} caused all function calls to give error in
> `find_str_or_fail` line, and so no test runs completely. But
> 'perf test "perf script task-analyzer tests"' wrongly reports
> that tests passed with the status OK, which is wrong considering
> the tests didn't even run completely
> 
> Fixes: e8478b84d6ba ("perf test: add new task-analyzer tests")
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
>  tools/perf/tests/shell/test_task_analyzer.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/shell/test_task_analyzer.sh b/tools/perf/tests/shell/test_task_analyzer.sh
> index 4264b54b654b..84ab7e7f57d5 100755
> --- a/tools/perf/tests/shell/test_task_analyzer.sh
> +++ b/tools/perf/tests/shell/test_task_analyzer.sh
> @@ -31,7 +31,7 @@ report() {
>  
>  check_exec_0() {
>  	if [ $? != 0 ]; then
> -		report 1 "invokation of ${$1} command failed"
> +		report 1 "invocation of $1 command failed"
>  	fi
>  }
>  
> -- 
> 2.39.1
> 

-- 

- Arnaldo

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Namhyung Kim]

Hello,

On Tue, Jun 13, 2023 at 1:06 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Tue, Jun 13, 2023 at 10:11:43PM +0530, Athira Rajeev escreveu:
> > From: Aditya Gupta <adityag@linux.ibm.com>
> >
> > ${$1} gives bad substitution error on sh, bash, and zsh. This seems like
> > a typo, and this patch modifies it to $1, since that is what it's usage
> > looks like from wherever `check_exec_0` is called.
>
> Nicely spotted!
>
> Please add the people that last touched the problem to the cc list,
> specially when it fixes a bug.
>
> Thanks for adding a Fixes tag, that helps the stable@kernel.org guys to
> get this propagated to supported kernel releases.
>
> I've added the test author to the CC list in this message.
>
> thanks!
>
> - Arnaldo
>
> > This issue due to ${$1} caused all function calls to give error in
> > `find_str_or_fail` line, and so no test runs completely. But
> > 'perf test "perf script task-analyzer tests"' wrongly reports
> > that tests passed with the status OK, which is wrong considering
> > the tests didn't even run completely
> >
> > Fixes: e8478b84d6ba ("perf test: add new task-analyzer tests")
> > Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> > Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> > Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> > ---

I'm seeing a different error even after this fix.
Can you please take a look?

Thanks,
Namhyung


$ sudo ./perf test -v task
114: perf script task-analyzer tests                                 :
--- start ---
test child forked, pid 1771042
Please specify a valid report script(see 'perf script -l' for listing)
FAIL: "invocation of perf command failed" Error message: ""
FAIL: "test_basic" Error message: "Failed to find required string:'Comm'."
Please specify a valid report script(see 'perf script -l' for listing)
FAIL: "invocation of perf command failed" Error message: ""
FAIL: "test_ns_rename" Error message: "Failed to find required string:'Comm'."
...
test child finished with -1
---- end ----
perf script task-analyzer tests: FAILED!

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Aditya Gupta]

Hello Namhyung,

On 21/06/23 06:18, Namhyung Kim wrote:
> Hello,
>
> On Tue, Jun 13, 2023 at 1:06 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
>> Em Tue, Jun 13, 2023 at 10:11:43PM +0530, Athira Rajeev escreveu:
>>> This issue due to ${$1} caused all function calls to give error in
>>> `find_str_or_fail` line, and so no test runs completely. But
>>> 'perf test "perf script task-analyzer tests"' wrongly reports
>>> that tests passed with the status OK, which is wrong considering
>>> the tests didn't even run completely
>>>
>>> Fixes: e8478b84d6ba ("perf test: add new task-analyzer tests")
>>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
>>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>>> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
>>> ---
> I'm seeing a different error even after this fix.
> Can you please take a look?
>
> Thanks,
> Namhyung
>
>
> $ sudo ./perf test -v task
> 114: perf script task-analyzer tests                                 :
> --- start ---
> test child forked, pid 1771042
> Please specify a valid report script(see 'perf script -l' for listing)
> FAIL: "invocation of perf command failed" Error message: ""
> FAIL: "test_basic" Error message: "Failed to find required string:'Comm'."
> Please specify a valid report script(see 'perf script -l' for listing)
> FAIL: "invocation of perf command failed" Error message: ""
> FAIL: "test_ns_rename" Error message: "Failed to find required string:'Comm'."
> ...
> test child finished with -1
> ---- end ----
> perf script task-analyzer tests: FAILED!
Can you please check if your environment has libtraceevent devel 
libraries (or did you compile with `make NO_LIBTRACEEVENT=1`) ? When 
libtraceevent support is not there, perf record fails and so perf.data 
doesn't contain the strings it's searching for and hence those errors

The error you mentioned has been mentioned and fixed in patch 17/17 of 
this series.

Thanks
- Aditya

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Namhyung Kim]

Hello Aditya,

On Wed, Jun 21, 2023 at 3:05 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
>
> Hello Namhyung,
>
> On 21/06/23 06:18, Namhyung Kim wrote:
> > Hello,
> >
> > On Tue, Jun 13, 2023 at 1:06 PM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> >> Em Tue, Jun 13, 2023 at 10:11:43PM +0530, Athira Rajeev escreveu:
> >>> This issue due to ${$1} caused all function calls to give error in
> >>> `find_str_or_fail` line, and so no test runs completely. But
> >>> 'perf test "perf script task-analyzer tests"' wrongly reports
> >>> that tests passed with the status OK, which is wrong considering
> >>> the tests didn't even run completely
> >>>
> >>> Fixes: e8478b84d6ba ("perf test: add new task-analyzer tests")
> >>> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> >>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> >>> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> >>> ---
> > I'm seeing a different error even after this fix.
> > Can you please take a look?
> >
> > Thanks,
> > Namhyung
> >
> >
> > $ sudo ./perf test -v task
> > 114: perf script task-analyzer tests                                 :
> > --- start ---
> > test child forked, pid 1771042
> > Please specify a valid report script(see 'perf script -l' for listing)
> > FAIL: "invocation of perf command failed" Error message: ""
> > FAIL: "test_basic" Error message: "Failed to find required string:'Comm'."
> > Please specify a valid report script(see 'perf script -l' for listing)
> > FAIL: "invocation of perf command failed" Error message: ""
> > FAIL: "test_ns_rename" Error message: "Failed to find required string:'Comm'."
> > ...
> > test child finished with -1
> > ---- end ----
> > perf script task-analyzer tests: FAILED!
> Can you please check if your environment has libtraceevent devel
> libraries (or did you compile with `make NO_LIBTRACEEVENT=1`) ? When
> libtraceevent support is not there, perf record fails and so perf.data
> doesn't contain the strings it's searching for and hence those errors
>
> The error you mentioned has been mentioned and fixed in patch 17/17 of
> this series.

Thanks for your reply but It has libtraceevent.  Also, shouldn't it
skip if it's not?

Thanks,
Namhyung

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Aditya Gupta]

Hello Namhyung,

On 21/06/23 20:53, Namhyung Kim wrote:
> Hello Aditya,
>
> On Wed, Jun 21, 2023 at 3:05 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
>> Hello Namhyung,
>>
>> On 21/06/23 06:18, Namhyung Kim wrote:
>>> ...
>>>
>>> $ sudo ./perf test -v task
>>> 114: perf script task-analyzer tests                                 :
>>> --- start ---
>>> test child forked, pid 1771042
>>> Please specify a valid report script(see 'perf script -l' for listing)
>>> FAIL: "invocation of perf command failed" Error message: ""
>>> FAIL: "test_basic" Error message: "Failed to find required string:'Comm'."
>>> Please specify a valid report script(see 'perf script -l' for listing)
>>> FAIL: "invocation of perf command failed" Error message: ""
>>> FAIL: "test_ns_rename" Error message: "Failed to find required string:'Comm'."
>>> ...
>>> test child finished with -1
>>> ---- end ----
>>> perf script task-analyzer tests: FAILED!
>> Can you please check if your environment has libtraceevent devel
>> libraries (or did you compile with `make NO_LIBTRACEEVENT=1`) ? When
>> libtraceevent support is not there, perf record fails and so perf.data
>> doesn't contain the strings it's searching for and hence those errors
>>
>> The error you mentioned has been mentioned and fixed in patch 17/17 of
>> this series.
> Thanks for your reply but It has libtraceevent.  Also, shouldn't it
> skip if it's not?
>
> Thanks,
> Namhyung

The skipping is handled in the 17th patch in this series, and 
considering that patch has also been applied, it will skip the tests if 
perf wasn't built with proper libtraceevent support.

Back to the error, Sorry but I tested again on my system and am unable to reproduce the issue you are seeing when built with libtraceevent support.

This is what I did:

0. git clone --depth=1 https://github.com/torvalds/linux
0. cd linux/tools/perf
0. git am patch_15/17.patch

> I applied ONLY this patch (15/17) of this series, to a fresh linux tree

1. dnf install libtraceevent-devel
2. make clean && make
3. sudo ./perf test -v "perf script task-analyzer tests"       # Working fine, tests passed

4. dnf remove libtraceevent-devel
5. make clean && make                                          # There will also be a warning during build: "libtraceevent is missing limiting functionality"
6. sudo ./perf test -v "perf script task-analyzer tests"       # Fails with the error you posted, which was the case till now, it's skipped when the 17th patch is also applied and perf built without libtraceevent support

The error in the second case (without proper libtraceevent support) is expected, as it was the case till now, that is fixed by the 17th patch, try applying that also and build perf with `make NO_LIBTRACEEVENT=1`, it will skip then.

Can you guide me with the steps to reproduce the error ?

Sidenote: Just in case, please ensure you are running the perf as root here as `perf record -e sched:sched_switch -a -- sleep 1` requires root, which has been used in `prepare_perf_data`

Thanks,
- Aditya

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Namhyung Kim]

Hello Aditya,

On Wed, Jun 21, 2023 at 11:43 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
>
> Hello Namhyung,
>
> On 21/06/23 20:53, Namhyung Kim wrote:
> > Hello Aditya,
> >
> > On Wed, Jun 21, 2023 at 3:05 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
> >> Hello Namhyung,
> >>
> >> On 21/06/23 06:18, Namhyung Kim wrote:
> >>> ...
> >>>
> >>> $ sudo ./perf test -v task
> >>> 114: perf script task-analyzer tests                                 :
> >>> --- start ---
> >>> test child forked, pid 1771042
> >>> Please specify a valid report script(see 'perf script -l' for listing)
> >>> FAIL: "invocation of perf command failed" Error message: ""
> >>> FAIL: "test_basic" Error message: "Failed to find required string:'Comm'."
> >>> Please specify a valid report script(see 'perf script -l' for listing)
> >>> FAIL: "invocation of perf command failed" Error message: ""
> >>> FAIL: "test_ns_rename" Error message: "Failed to find required string:'Comm'."
> >>> ...
> >>> test child finished with -1
> >>> ---- end ----
> >>> perf script task-analyzer tests: FAILED!
> >> Can you please check if your environment has libtraceevent devel
> >> libraries (or did you compile with `make NO_LIBTRACEEVENT=1`) ? When
> >> libtraceevent support is not there, perf record fails and so perf.data
> >> doesn't contain the strings it's searching for and hence those errors
> >>
> >> The error you mentioned has been mentioned and fixed in patch 17/17 of
> >> this series.
> > Thanks for your reply but It has libtraceevent.  Also, shouldn't it
> > skip if it's not?
> >
> > Thanks,
> > Namhyung
>
> The skipping is handled in the 17th patch in this series, and
> considering that patch has also been applied, it will skip the tests if
> perf wasn't built with proper libtraceevent support.

Right, I tested it with the whole series applied.
It skipped to run when I disable libtraceevent build.

>
> Back to the error, Sorry but I tested again on my system and am unable to reproduce the issue you are seeing when built with libtraceevent support.
>
> This is what I did:
>
> 0. git clone --depth=1 https://github.com/torvalds/linux
> 0. cd linux/tools/perf
> 0. git am patch_15/17.patch
>
> > I applied ONLY this patch (15/17) of this series, to a fresh linux tree
>
> 1. dnf install libtraceevent-devel
> 2. make clean && make
> 3. sudo ./perf test -v "perf script task-analyzer tests"       # Working fine, tests passed
>
> 4. dnf remove libtraceevent-devel
> 5. make clean && make                                          # There will also be a warning during build: "libtraceevent is missing limiting functionality"
> 6. sudo ./perf test -v "perf script task-analyzer tests"       # Fails with the error you posted, which was the case till now, it's skipped when the 17th patch is also applied and perf built without libtraceevent support
>
> The error in the second case (without proper libtraceevent support) is expected, as it was the case till now, that is fixed by the 17th patch, try applying that also and build perf with `make NO_LIBTRACEEVENT=1`, it will skip then.
>
> Can you guide me with the steps to reproduce the error ?

You can try the perf-tools-next branch in the perf/perft-tools-next.git repo.

  git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git

  $ make clean all
  $ sudo ./perf test -v task

>
> Sidenote: Just in case, please ensure you are running the perf as root here as `perf record -e sched:sched_switch -a -- sleep 1` requires root, which has been used in `prepare_perf_data`

Sure, I ran the perf test as root.

Thanks,
Namhyung

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Namhyung Kim]

Ok, I found two problems.

On Thu, Jun 22, 2023 at 2:33 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello Aditya,
>
> On Wed, Jun 21, 2023 at 11:43 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
> >
> > Hello Namhyung,
> >
> > On 21/06/23 20:53, Namhyung Kim wrote:
> > > Hello Aditya,
> > >
> > > On Wed, Jun 21, 2023 at 3:05 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
> > >> Hello Namhyung,
> > >>
> > >> On 21/06/23 06:18, Namhyung Kim wrote:
> > >>> ...
> > >>>
> > >>> $ sudo ./perf test -v task
> > >>> 114: perf script task-analyzer tests                                 :
> > >>> --- start ---
> > >>> test child forked, pid 1771042
> > >>> Please specify a valid report script(see 'perf script -l' for listing)

The first one is related to this message.  It couldn't find the script
(task-analyzer.py) because PERF_EXEC_PATH is not set.
Running with --exec-path=$PWD was ok, but I got a segfault.

The other problem is in set_regs_in_dict().  It tries to capture
register values in the sample and save them to a dictionary.
The sample doesn't have registers so it should have no problem.
But the 'bf' was not initialized properly when size is 0, and it led
PyUnicode_FromString() returning NULL.

After the changes, it ran ok:

$ sudo ./perf test -v task
116: perf script task-analyzer tests                                 :
--- start ---
test child forked, pid 204088
PASS: "test_basic"
PASS: "test_ns_rename"
PASS: "test_ms_filtertasks_highlight"
PASS: "test_extended_times_timelimit_limittasks"
PASS: "test_summary"
PASS: "test_summaryextended"
PASS: "test_summaryonly"
PASS: "test_extended_times_summary_ns"
PASS: "test_extended_times_summary_ns"
PASS: "test_csv"
PASS: "test_csvsummary"
PASS: "test_csv_extended_times"
PASS: "test_csvsummary_extended"
test child finished with 0
---- end ----
perf script task-analyzer tests: Ok

I'll send the fixes soon.

Thanks,
Namhyung


> > >>> FAIL: "invocation of perf command failed" Error message: ""
> > >>> FAIL: "test_basic" Error message: "Failed to find required string:'Comm'."
> > >>> Please specify a valid report script(see 'perf script -l' for listing)
> > >>> FAIL: "invocation of perf command failed" Error message: ""
> > >>> FAIL: "test_ns_rename" Error message: "Failed to find required string:'Comm'."
> > >>> ...
> > >>> test child finished with -1
> > >>> ---- end ----
> > >>> perf script task-analyzer tests: FAILED!
> > >> Can you please check if your environment has libtraceevent devel
> > >> libraries (or did you compile with `make NO_LIBTRACEEVENT=1`) ? When
> > >> libtraceevent support is not there, perf record fails and so perf.data
> > >> doesn't contain the strings it's searching for and hence those errors
> > >>
> > >> The error you mentioned has been mentioned and fixed in patch 17/17 of
> > >> this series.
> > > Thanks for your reply but It has libtraceevent.  Also, shouldn't it
> > > skip if it's not?
> > >
> > > Thanks,
> > > Namhyung
> >
> > The skipping is handled in the 17th patch in this series, and
> > considering that patch has also been applied, it will skip the tests if
> > perf wasn't built with proper libtraceevent support.
>
> Right, I tested it with the whole series applied.
> It skipped to run when I disable libtraceevent build.
>
> >
> > Back to the error, Sorry but I tested again on my system and am unable to reproduce the issue you are seeing when built with libtraceevent support.
> >
> > This is what I did:
> >
> > 0. git clone --depth=1 https://github.com/torvalds/linux
> > 0. cd linux/tools/perf
> > 0. git am patch_15/17.patch
> >
> > > I applied ONLY this patch (15/17) of this series, to a fresh linux tree
> >
> > 1. dnf install libtraceevent-devel
> > 2. make clean && make
> > 3. sudo ./perf test -v "perf script task-analyzer tests"       # Working fine, tests passed
> >
> > 4. dnf remove libtraceevent-devel
> > 5. make clean && make                                          # There will also be a warning during build: "libtraceevent is missing limiting functionality"
> > 6. sudo ./perf test -v "perf script task-analyzer tests"       # Fails with the error you posted, which was the case till now, it's skipped when the 17th patch is also applied and perf built without libtraceevent support
> >
> > The error in the second case (without proper libtraceevent support) is expected, as it was the case till now, that is fixed by the 17th patch, try applying that also and build perf with `make NO_LIBTRACEEVENT=1`, it will skip then.
> >
> > Can you guide me with the steps to reproduce the error ?
>
> You can try the perf-tools-next branch in the perf/perft-tools-next.git repo.
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git
>
>   $ make clean all
>   $ sudo ./perf test -v task
>
> >
> > Sidenote: Just in case, please ensure you are running the perf as root here as `perf record -e sched:sched_switch -a -- sleep 1` requires root, which has been used in `prepare_perf_data`
>
> Sure, I ran the perf test as root.
>
> Thanks,
> Namhyung

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Aditya Gupta]

Hello, Namhyung,

On 23/06/23 04:55, Namhyung Kim wrote:
> Ok, I found two problems.
>
> ...
>
> The first one is related to this message.  It couldn't find the script
> (task-analyzer.py) because PERF_EXEC_PATH is not set.
> Running with --exec-path=$PWD was ok, but I got a segfault.
>
> The other problem is in set_regs_in_dict().  It tries to capture
> register values in the sample and save them to a dictionary.
> The sample doesn't have registers so it should have no problem.
> But the 'bf' was not initialized properly when size is 0, and it led
> PyUnicode_FromString() returning NULL.
>
> After the changes, it ran ok:
>
> $ sudo ./perf test -v task
> 116: perf script task-analyzer tests                                 :
> --- start ---
> test child forked, pid 204088
> PASS: "test_basic"
> PASS: "test_ns_rename"
> PASS: "test_ms_filtertasks_highlight"
> PASS: "test_extended_times_timelimit_limittasks"
> PASS: "test_summary"
> PASS: "test_summaryextended"
> PASS: "test_summaryonly"
> PASS: "test_extended_times_summary_ns"
> PASS: "test_extended_times_summary_ns"
> PASS: "test_csv"
> PASS: "test_csvsummary"
> PASS: "test_csv_extended_times"
> PASS: "test_csvsummary_extended"
> test child finished with 0
> ---- end ----
> perf script task-analyzer tests: Ok
This is interesting. I did not encounter these earlier.

> I'll send the fixes soon.
>
Thanks for fixing it.
>>> You can try the perf-tools-next branch in the perf/perft-tools-next.git repo.
>>>
>>>    git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git
>>>
>>>    $ make clean all
>>>    $ sudo ./perf test -v task
>>>

I tested with the steps provided, i.e.. clone, /*apply patch 15 (added 
this step myself)*/, clean, make, ./perf test -v test. But still- test 
passed:

'''
~/temp_clones/perf-tools-next/tools/perf git:(master) sudo ./perf test 
-v test

...

--- start ---
test child forked, pid 75261
PASS: "test_basic"
PASS: "test_ns_rename"
PASS: "test_ms_filtertasks_highlight"
PASS: "test_extended_times_timelimit_limittasks"
PASS: "test_summary"
PASS: "test_summaryextended"
PASS: "test_summaryonly"
PASS: "test_extended_times_summary_ns"
PASS: "test_extended_times_summary_ns"
PASS: "test_csv"
PASS: "test_csvsummary"
PASS: "test_csv_extended_times"
PASS: "test_csvsummary_extended"
test child finished with 0
---- end ----
'''

Maybe my environment had that other things set in a way that, I did not 
face any issue (I don't recall doing any special thing with my 
environment though, and these patches have been tested multiple times by 
more people also).
But thanks for your efforts to debug and fix the issue.

Thanks,
- AdityaG

Re: [PATCH 15/17] perf tests task_analyzer: fix bad substitution ${$1}

[Namhyung Kim]

Hello Aditya,

On Fri, Jun 23, 2023 at 11:19 AM Aditya Gupta <adityag@linux.ibm.com> wrote:
>
> Hello, Namhyung,
>
> On 23/06/23 04:55, Namhyung Kim wrote:
> > Ok, I found two problems.
> >
> > ...
> >
> > The first one is related to this message.  It couldn't find the script
> > (task-analyzer.py) because PERF_EXEC_PATH is not set.
> > Running with --exec-path=$PWD was ok, but I got a segfault.
> >
> > The other problem is in set_regs_in_dict().  It tries to capture
> > register values in the sample and save them to a dictionary.
> > The sample doesn't have registers so it should have no problem.
> > But the 'bf' was not initialized properly when size is 0, and it led
> > PyUnicode_FromString() returning NULL.
> >
> > After the changes, it ran ok:
> >
> > $ sudo ./perf test -v task
> > 116: perf script task-analyzer tests                                 :
> > --- start ---
> > test child forked, pid 204088
> > PASS: "test_basic"
> > PASS: "test_ns_rename"
> > PASS: "test_ms_filtertasks_highlight"
> > PASS: "test_extended_times_timelimit_limittasks"
> > PASS: "test_summary"
> > PASS: "test_summaryextended"
> > PASS: "test_summaryonly"
> > PASS: "test_extended_times_summary_ns"
> > PASS: "test_extended_times_summary_ns"
> > PASS: "test_csv"
> > PASS: "test_csvsummary"
> > PASS: "test_csv_extended_times"
> > PASS: "test_csvsummary_extended"
> > test child finished with 0
> > ---- end ----
> > perf script task-analyzer tests: Ok
> This is interesting. I did not encounter these earlier.
>
> > I'll send the fixes soon.
> >
> Thanks for fixing it.

No problem.

> >>> You can try the perf-tools-next branch in the perf/perft-tools-next.git repo.
> >>>
> >>>    git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git
> >>>
> >>>    $ make clean all
> >>>    $ sudo ./perf test -v task
> >>>
>
> I tested with the steps provided, i.e.. clone, /*apply patch 15 (added
> this step myself)*/, clean, make, ./perf test -v test. But still- test
> passed:
>
> '''
> ~/temp_clones/perf-tools-next/tools/perf git:(master) sudo ./perf test
> -v test
>
> ...
>
> --- start ---
> test child forked, pid 75261
> PASS: "test_basic"
> PASS: "test_ns_rename"
> PASS: "test_ms_filtertasks_highlight"
> PASS: "test_extended_times_timelimit_limittasks"
> PASS: "test_summary"
> PASS: "test_summaryextended"
> PASS: "test_summaryonly"
> PASS: "test_extended_times_summary_ns"
> PASS: "test_extended_times_summary_ns"
> PASS: "test_csv"
> PASS: "test_csvsummary"
> PASS: "test_csv_extended_times"
> PASS: "test_csvsummary_extended"
> test child finished with 0
> ---- end ----
> '''
>
> Maybe my environment had that other things set in a way that, I did not
> face any issue (I don't recall doing any special thing with my
> environment though, and these patches have been tested multiple times by
> more people also).
> But thanks for your efforts to debug and fix the issue.

I don't know but you might either set PERF_EXEC_PATH env or
install the scripts somewhere that can be found by default.

Thanks,
Namhyung

[PATCH 16/17] perf tests task_analyzer: print command on failure

[Athira Rajeev]

From: Aditya Gupta <adityag@linux.ibm.com>

Instead of printing "perf command failed" everytime, print the exact
command that run earlier

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/tests/shell/test_task_analyzer.sh | 24 ++++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/tests/shell/test_task_analyzer.sh b/tools/perf/tests/shell/test_task_analyzer.sh
index 84ab7e7f57d5..b094eeb3bf66 100755
--- a/tools/perf/tests/shell/test_task_analyzer.sh
+++ b/tools/perf/tests/shell/test_task_analyzer.sh
@@ -53,14 +53,14 @@ prepare_perf_data() {
 test_basic() {
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer > "$out"
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer"
 	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
 }
 
 test_ns_rename(){
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --ns --rename-comms-by-tids 0:random > "$out"
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --ns --rename-comms-by-tids 0:random"
 	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
 }
 
@@ -68,7 +68,7 @@ test_ms_filtertasks_highlight(){
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --ms --filter-tasks perf --highlight-tasks perf \
 	> "$out"
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --ms --filter-tasks perf --highlight-tasks perf"
 	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
 }
 
@@ -76,61 +76,61 @@ test_extended_times_timelimit_limittasks() {
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --extended-times --time-limit :99999 \
 	--limit-to-tasks perf > "$out"
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --extended-times --time-limit :99999 --limit-to-tasks perf"
 	find_str_or_fail "Out-Out" "$out" "${FUNCNAME[0]}"
 }
 
 test_summary() {
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --summary > "$out"
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --summary"
 	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
 }
 
 test_summaryextended() {
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --summary-extended > "$out"
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --summary-extended"
 	find_str_or_fail "Inter Task Times" "$out" "${FUNCNAME[0]}"
 }
 
 test_summaryonly() {
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --summary-only > "$out"
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --summary-only"
 	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
 }
 
 test_extended_times_summary_ns() {
 	out="$tmpdir/perf.out"
 	perf script report task-analyzer --extended-times --summary --ns > "$out"
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --extended-times --summary --ns"
 	find_str_or_fail "Out-Out" "$out" "${FUNCNAME[0]}"
 	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
 }
 
 test_csv() {
 	perf script report task-analyzer --csv csv > /dev/null
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --csv csv"
 	find_str_or_fail "Comm;" csv "${FUNCNAME[0]}"
 }
 
 test_csv_extended_times() {
 	perf script report task-analyzer --csv csv --extended-times > /dev/null
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --csv csv --extended-times"
 	find_str_or_fail "Out-Out;" csv "${FUNCNAME[0]}"
 }
 
 test_csvsummary() {
 	perf script report task-analyzer --csv-summary csvsummary > /dev/null
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --csv-summary csvsummary"
 	find_str_or_fail "Comm;" csvsummary "${FUNCNAME[0]}"
 }
 
 test_csvsummary_extended() {
 	perf script report task-analyzer --csv-summary csvsummary --summary-extended \
 	>/dev/null
-	check_exec_0 "perf"
+	check_exec_0 "perf script report task-analyzer --csv-summary csvsummary --summary-extended"
 	find_str_or_fail "Out-Out;" csvsummary "${FUNCNAME[0]}"
 }
 
-- 
2.39.1

Re: [PATCH 16/17] perf tests task_analyzer: print command on failure

[Arnaldo Carvalho de Melo]

Em Tue, Jun 13, 2023 at 10:11:44PM +0530, Athira Rajeev escreveu:
> From: Aditya Gupta <adityag@linux.ibm.com>
> 
> Instead of printing "perf command failed" everytime, print the exact
> command that run earlier

Looks like a nice improvement, added the test authors to the CC list on
this message,

Thanks, applied.

- Arnaldo
 
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
>  tools/perf/tests/shell/test_task_analyzer.sh | 24 ++++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/test_task_analyzer.sh b/tools/perf/tests/shell/test_task_analyzer.sh
> index 84ab7e7f57d5..b094eeb3bf66 100755
> --- a/tools/perf/tests/shell/test_task_analyzer.sh
> +++ b/tools/perf/tests/shell/test_task_analyzer.sh
> @@ -53,14 +53,14 @@ prepare_perf_data() {
>  test_basic() {
>  	out="$tmpdir/perf.out"
>  	perf script report task-analyzer > "$out"
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer"
>  	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
>  }
>  
>  test_ns_rename(){
>  	out="$tmpdir/perf.out"
>  	perf script report task-analyzer --ns --rename-comms-by-tids 0:random > "$out"
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --ns --rename-comms-by-tids 0:random"
>  	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
>  }
>  
> @@ -68,7 +68,7 @@ test_ms_filtertasks_highlight(){
>  	out="$tmpdir/perf.out"
>  	perf script report task-analyzer --ms --filter-tasks perf --highlight-tasks perf \
>  	> "$out"
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --ms --filter-tasks perf --highlight-tasks perf"
>  	find_str_or_fail "Comm" "$out" "${FUNCNAME[0]}"
>  }
>  
> @@ -76,61 +76,61 @@ test_extended_times_timelimit_limittasks() {
>  	out="$tmpdir/perf.out"
>  	perf script report task-analyzer --extended-times --time-limit :99999 \
>  	--limit-to-tasks perf > "$out"
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --extended-times --time-limit :99999 --limit-to-tasks perf"
>  	find_str_or_fail "Out-Out" "$out" "${FUNCNAME[0]}"
>  }
>  
>  test_summary() {
>  	out="$tmpdir/perf.out"
>  	perf script report task-analyzer --summary > "$out"
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --summary"
>  	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
>  }
>  
>  test_summaryextended() {
>  	out="$tmpdir/perf.out"
>  	perf script report task-analyzer --summary-extended > "$out"
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --summary-extended"
>  	find_str_or_fail "Inter Task Times" "$out" "${FUNCNAME[0]}"
>  }
>  
>  test_summaryonly() {
>  	out="$tmpdir/perf.out"
>  	perf script report task-analyzer --summary-only > "$out"
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --summary-only"
>  	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
>  }
>  
>  test_extended_times_summary_ns() {
>  	out="$tmpdir/perf.out"
>  	perf script report task-analyzer --extended-times --summary --ns > "$out"
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --extended-times --summary --ns"
>  	find_str_or_fail "Out-Out" "$out" "${FUNCNAME[0]}"
>  	find_str_or_fail "Summary" "$out" "${FUNCNAME[0]}"
>  }
>  
>  test_csv() {
>  	perf script report task-analyzer --csv csv > /dev/null
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --csv csv"
>  	find_str_or_fail "Comm;" csv "${FUNCNAME[0]}"
>  }
>  
>  test_csv_extended_times() {
>  	perf script report task-analyzer --csv csv --extended-times > /dev/null
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --csv csv --extended-times"
>  	find_str_or_fail "Out-Out;" csv "${FUNCNAME[0]}"
>  }
>  
>  test_csvsummary() {
>  	perf script report task-analyzer --csv-summary csvsummary > /dev/null
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --csv-summary csvsummary"
>  	find_str_or_fail "Comm;" csvsummary "${FUNCNAME[0]}"
>  }
>  
>  test_csvsummary_extended() {
>  	perf script report task-analyzer --csv-summary csvsummary --summary-extended \
>  	>/dev/null
> -	check_exec_0 "perf"
> +	check_exec_0 "perf script report task-analyzer --csv-summary csvsummary --summary-extended"
>  	find_str_or_fail "Out-Out;" csvsummary "${FUNCNAME[0]}"
>  }
>  
> -- 
> 2.39.1
> 

-- 

- Arnaldo

Re: [PATCH 16/17] perf tests task_analyzer: print command on failure

[Hagen Paul Pfeifer]

* Arnaldo Carvalho de Melo | 2023-06-13 17:07:52 [-0300]:

>> From: Aditya Gupta <adityag@linux.ibm.com>
>> 
>> Instead of printing "perf command failed" everytime, print the exact
>> command that run earlier
>
>Looks like a nice improvement, added the test authors to the CC list on
>this message,

Thank you Aditya and really great catch - thank you very much! Shell scripting
can be nasty. This is another example where you can see that with one stupid
typo the whole system can be undermined and you don't get a single hint.

Acked-by: Hagen Paul Pfeifer <hagen@jauu.net>

[PATCH 17/17] perf tests task_analyzer: skip tests if no libtraceevent support

[Athira Rajeev]

From: Aditya Gupta <adityag@linux.ibm.com>

Test "perf script task-analyzer tests" fails in environment with missing
libtraceevent support, as perf record fails to create the perf.data
file, which further tests depend on.

Instead, when perf is not compiled with libtraceevent support, skip those
tests instead of failing them, by checking the output of `perf
record --dry-run` to see if it prints the error "libtraceevent is
necessary for tracepoint support"

For the following output, perf compiled with: `make NO_LIBTRACEEVENT=1`

Before the patch:

108: perf script task-analyzer tests                                 :
test child forked, pid 24105
failed to open perf.data: No such file or directory  (try 'perf record' first)
FAIL: "invokation of perf script report task-analyzer command failed" Error message: ""
FAIL: "test_basic" Error message: "Failed to find required string:'Comm'."
failed to open perf.data: No such file or directory  (try 'perf record' first)
FAIL: "invokation of perf script report task-analyzer --ns --rename-comms-by-tids 0:random command failed" Error message: ""
FAIL: "test_ns_rename" Error message: "Failed to find required string:'Comm'."
failed to open perf.data: No such file or directory  (try 'perf record' first)
<...>
perf script task-analyzer tests: FAILED!

With this patch, the script instead returns 2 signifying SKIP, and after
the patch:

108: perf script task-analyzer tests                                 :
test child forked, pid 26010
libtraceevent is necessary for tracepoint support
WARN: Skipping tests. No libtraceevent support
test child finished with -2
perf script task-analyzer tests: Skip

Fixes: e8478b84d6ba ("perf test: add new task-analyzer tests")
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
---
 tools/perf/tests/shell/test_task_analyzer.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/perf/tests/shell/test_task_analyzer.sh b/tools/perf/tests/shell/test_task_analyzer.sh
index b094eeb3bf66..59785dfc11f8 100755
--- a/tools/perf/tests/shell/test_task_analyzer.sh
+++ b/tools/perf/tests/shell/test_task_analyzer.sh
@@ -44,9 +44,20 @@ find_str_or_fail() {
 	fi
 }
 
+# check if perf is compiled with libtraceevent support
+skip_no_probe_record_support() {
+	perf record -e "sched:sched_switch" -a -- sleep 1 2>&1 | grep "libtraceevent is necessary for tracepoint support" && return 2
+	return 0
+}
+
 prepare_perf_data() {
 	# 1s should be sufficient to catch at least some switches
 	perf record -e sched:sched_switch -a -- sleep 1 > /dev/null 2>&1
+	# check if perf data file got created in above step.
+	if [ ! -e "perf.data" ]; then
+		printf "FAIL: perf record failed to create \"perf.data\" \n"
+		return 1
+	fi
 }
 
 # check standard inkvokation with no arguments
@@ -134,6 +145,13 @@ test_csvsummary_extended() {
 	find_str_or_fail "Out-Out;" csvsummary "${FUNCNAME[0]}"
 }
 
+skip_no_probe_record_support
+err=$?
+if [ $err -ne 0 ]; then
+	echo "WARN: Skipping tests. No libtraceevent support"
+	cleanup
+	exit $err
+fi
 prepare_perf_data
 test_basic
 test_ns_rename
-- 
2.39.1

Re: [PATCH 17/17] perf tests task_analyzer: skip tests if no libtraceevent support

[Ian Rogers]

On Tue, Jun 13, 2023 at 10:04 AM Athira Rajeev
<atrajeev@linux.vnet.ibm.com> wrote:
>
> From: Aditya Gupta <adityag@linux.ibm.com>
>
> Test "perf script task-analyzer tests" fails in environment with missing
> libtraceevent support, as perf record fails to create the perf.data
> file, which further tests depend on.
>
> Instead, when perf is not compiled with libtraceevent support, skip those
> tests instead of failing them, by checking the output of `perf
> record --dry-run` to see if it prints the error "libtraceevent is
> necessary for tracepoint support"
>
> For the following output, perf compiled with: `make NO_LIBTRACEEVENT=1`
>
> Before the patch:
>
> 108: perf script task-analyzer tests                                 :
> test child forked, pid 24105
> failed to open perf.data: No such file or directory  (try 'perf record' first)
> FAIL: "invokation of perf script report task-analyzer command failed" Error message: ""
> FAIL: "test_basic" Error message: "Failed to find required string:'Comm'."
> failed to open perf.data: No such file or directory  (try 'perf record' first)
> FAIL: "invokation of perf script report task-analyzer --ns --rename-comms-by-tids 0:random command failed" Error message: ""
> FAIL: "test_ns_rename" Error message: "Failed to find required string:'Comm'."
> failed to open perf.data: No such file or directory  (try 'perf record' first)
> <...>
> perf script task-analyzer tests: FAILED!
>
> With this patch, the script instead returns 2 signifying SKIP, and after
> the patch:
>
> 108: perf script task-analyzer tests                                 :
> test child forked, pid 26010
> libtraceevent is necessary for tracepoint support
> WARN: Skipping tests. No libtraceevent support
> test child finished with -2
> perf script task-analyzer tests: Skip
>
> Fixes: e8478b84d6ba ("perf test: add new task-analyzer tests")
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> ---
>  tools/perf/tests/shell/test_task_analyzer.sh | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/tools/perf/tests/shell/test_task_analyzer.sh b/tools/perf/tests/shell/test_task_analyzer.sh
> index b094eeb3bf66..59785dfc11f8 100755
> --- a/tools/perf/tests/shell/test_task_analyzer.sh
> +++ b/tools/perf/tests/shell/test_task_analyzer.sh
> @@ -44,9 +44,20 @@ find_str_or_fail() {
>         fi
>  }
>
> +# check if perf is compiled with libtraceevent support
> +skip_no_probe_record_support() {
> +       perf record -e "sched:sched_switch" -a -- sleep 1 2>&1 | grep "libtraceevent is necessary for tracepoint support" && return 2

Fwiw, another way to detect build options used in other shell tests is:
perf version --build-options | grep HAVE_LIBTRACEEVENT | grep -q OFF && return 2

Thanks,
Ian

> +       return 0
> +}
> +
>  prepare_perf_data() {
>         # 1s should be sufficient to catch at least some switches
>         perf record -e sched:sched_switch -a -- sleep 1 > /dev/null 2>&1
> +       # check if perf data file got created in above step.
> +       if [ ! -e "perf.data" ]; then
> +               printf "FAIL: perf record failed to create \"perf.data\" \n"
> +               return 1
> +       fi
>  }
>
>  # check standard inkvokation with no arguments
> @@ -134,6 +145,13 @@ test_csvsummary_extended() {
>         find_str_or_fail "Out-Out;" csvsummary "${FUNCNAME[0]}"
>  }
>
> +skip_no_probe_record_support
> +err=$?
> +if [ $err -ne 0 ]; then
> +       echo "WARN: Skipping tests. No libtraceevent support"
> +       cleanup
> +       exit $err
> +fi
>  prepare_perf_data
>  test_basic
>  test_ns_rename
> --
> 2.39.1
>

Re: [PATCH 17/17] perf tests task_analyzer: skip tests if no libtraceevent support

[Aditya Gupta]

Hello Ian,

On Wed, Jul 12, 2023 at 06:16:28PM -0700, Ian Rogers wrote:
> On Tue, Jun 13, 2023 at 10:04 AM Athira Rajeev
> <atrajeev@linux.vnet.ibm.com> wrote:
> >
> > From: Aditya Gupta <adityag@linux.ibm.com>
> >
> > Test "perf script task-analyzer tests" fails in environment with missing
> > libtraceevent support, as perf record fails to create the perf.data
> > file, which further tests depend on.
> >
> > Instead, when perf is not compiled with libtraceevent support, skip those
> > tests instead of failing them, by checking the output of `perf
> > record --dry-run` to see if it prints the error "libtraceevent is
> > necessary for tracepoint support"
> >
> > ...
> >
> > +# check if perf is compiled with libtraceevent support
> > +skip_no_probe_record_support() {
> > +       perf record -e "sched:sched_switch" -a -- sleep 1 2>&1 | grep "libtraceevent is necessary for tracepoint support" && return 2
> 
> Fwiw, another way to detect build options used in other shell tests is:
> perf version --build-options | grep HAVE_LIBTRACEEVENT | grep -q OFF && return 2
> 
> Thanks,
> Ian
> 

Thanks for the review. That seems more straightforward way to check for
libtraceevent support. I will work on a patch for this, and test it.

Thanks,
Aditya G

Re: [PATCH 00/17] tool/perf/test: Fix shellcheck coding/formatting issues of test shell scripts

[Arnaldo Carvalho de Melo]

Em Tue, Jun 13, 2023 at 10:11:28PM +0530, Athira Rajeev escreveu:
> Patchset covers a set of fixes for coding/formatting issues observed while
> running shellcheck tool on the perf test shell scripts. Shellcheck is a static
> analysis tool that can find semantic/syntax bugs in the shell scripts.

Thanks, applied the series.

- Arnaldo
 
> Patches 1-14 fixes the issues found with shellcheck. Patch 15, 16
> and patch 17 address a fix in task_analyzer test.
> 
> This cleanup is a pre-requisite to include a build option for shellcheck
> discussed here: https://www.spinics.net/lists/linux-perf-users/msg25553.html
> Also this is first set of patches. There will be one more set which will
> include build option for shellcheck as discussed in the mail thread.
> 
> Abhirup Deb (2):
>   tools/perf/tests: fix test_arm_spe.sh signal case issues
>   perf/tests/shell: fix shellscript errors for lock_contention.sh
> 
> Aboorva Devarajan (1):
>   tools/perf/tests: Fix shellcheck issues in test_task_analyzer.sh file
> 
> Aditya Gupta (3):
>   perf tests task_analyzer: fix bad substitution ${$1}
>   perf tests task_analyzer: print command on failure
>   perf tests task_analyzer: skip tests if no libtraceevent support
> 
> Akanksha J N (1):
>   tools/perf/tests: Fix shellcheck warnings for
>     trace+probe_vfs_getname.sh
> 
> Anushree Mathur (1):
>   perf/tests/shell : Shellcheck fixes for perf test
>     "test_arm_coresight.sh"
> 
> Barnali Guha Thakurata (1):
>   tools/perf/tests/shell/stat_all_metrics: Fix shellcheck warning SC2076
>     in stat_all_metrics.sh
> 
> Disha Goel (1):
>   tools/perf/tests: fix shellcheck warning for stat+json_output
> 
> Geetika (1):
>   tools/perf/tests: Fix all POSIX sh warnings in perf shell test
>     test_brstack.sh
> 
> Korrapati Likhitha (1):
>   tools/perf/tests: Fix shellcheck warnings for stat+csv_output
> 
> Samir Mulani (1):
>   tools/perf/tests: fixed shellcheck warnings for perf shell scripts
> 
> Shirisha G (1):
>   tools/perf/tests: fix shellcheck warnings for daemon.sh
> 
> Sourabh Jain (1):
>   perf: get rid of unused import
> 
> Spoorthy S (2):
>   shellcheck : fixing signal names and adding double quotes for
>     expression in test_arm_callgraph_fp
>   tools/perf/tests: Fix all POSIX sh warnings in stat+shadow_stat.sh
> 
>  .../scripts/python/arm-cs-trace-disasm.py     |   1 -
>  tools/perf/tests/shell/buildid.sh             |  12 +-
>  tools/perf/tests/shell/daemon.sh              | 113 ++++++++++++------
>  tools/perf/tests/shell/lock_contention.sh     |  70 +++++------
>  .../shell/record+probe_libc_inet_pton.sh      |   6 +-
>  .../shell/record+script_probe_vfs_getname.sh  |   4 +-
>  tools/perf/tests/shell/stat+csv_output.sh     |   4 +-
>  tools/perf/tests/shell/stat+json_output.sh    |   2 +-
>  tools/perf/tests/shell/stat+shadow_stat.sh    |   4 +-
>  tools/perf/tests/shell/stat_all_metrics.sh    |   6 +-
>  .../perf/tests/shell/test_arm_callgraph_fp.sh |   6 +-
>  tools/perf/tests/shell/test_arm_coresight.sh  |   6 +-
>  tools/perf/tests/shell/test_arm_spe.sh        |   2 +-
>  tools/perf/tests/shell/test_brstack.sh        |  12 +-
>  tools/perf/tests/shell/test_task_analyzer.sh  |  98 ++++++++-------
>  .../tests/shell/trace+probe_vfs_getname.sh    |   6 +-
>  16 files changed, 203 insertions(+), 149 deletions(-)
> 
> -- 
> 2.39.1
> 

-- 

- Arnaldo

Re: [PATCH 00/17] tool/perf/test: Fix shellcheck coding/formatting issues of test shell scripts

[Athira Rajeev]

> On 14-Jun-2023, at 2:04 AM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> Em Tue, Jun 13, 2023 at 10:11:28PM +0530, Athira Rajeev escreveu:
>> Patchset covers a set of fixes for coding/formatting issues observed while
>> running shellcheck tool on the perf test shell scripts. Shellcheck is a static
>> analysis tool that can find semantic/syntax bugs in the shell scripts.
> 
> Thanks, applied the series.

Hi,

Thanks Arnaldo for picking up the patchset.
We will check and resubmit patch 6.

Thanks
Athira
> 
> - Arnaldo
> 
>> Patches 1-14 fixes the issues found with shellcheck. Patch 15, 16
>> and patch 17 address a fix in task_analyzer test.
>> 
>> This cleanup is a pre-requisite to include a build option for shellcheck
>> discussed here: https://www.spinics.net/lists/linux-perf-users/msg25553.html
>> Also this is first set of patches. There will be one more set which will
>> include build option for shellcheck as discussed in the mail thread.
>> 
>> Abhirup Deb (2):
>>  tools/perf/tests: fix test_arm_spe.sh signal case issues
>>  perf/tests/shell: fix shellscript errors for lock_contention.sh
>> 
>> Aboorva Devarajan (1):
>>  tools/perf/tests: Fix shellcheck issues in test_task_analyzer.sh file
>> 
>> Aditya Gupta (3):
>>  perf tests task_analyzer: fix bad substitution ${$1}
>>  perf tests task_analyzer: print command on failure
>>  perf tests task_analyzer: skip tests if no libtraceevent support
>> 
>> Akanksha J N (1):
>>  tools/perf/tests: Fix shellcheck warnings for
>>    trace+probe_vfs_getname.sh
>> 
>> Anushree Mathur (1):
>>  perf/tests/shell : Shellcheck fixes for perf test
>>    "test_arm_coresight.sh"
>> 
>> Barnali Guha Thakurata (1):
>>  tools/perf/tests/shell/stat_all_metrics: Fix shellcheck warning SC2076
>>    in stat_all_metrics.sh
>> 
>> Disha Goel (1):
>>  tools/perf/tests: fix shellcheck warning for stat+json_output
>> 
>> Geetika (1):
>>  tools/perf/tests: Fix all POSIX sh warnings in perf shell test
>>    test_brstack.sh
>> 
>> Korrapati Likhitha (1):
>>  tools/perf/tests: Fix shellcheck warnings for stat+csv_output
>> 
>> Samir Mulani (1):
>>  tools/perf/tests: fixed shellcheck warnings for perf shell scripts
>> 
>> Shirisha G (1):
>>  tools/perf/tests: fix shellcheck warnings for daemon.sh
>> 
>> Sourabh Jain (1):
>>  perf: get rid of unused import
>> 
>> Spoorthy S (2):
>>  shellcheck : fixing signal names and adding double quotes for
>>    expression in test_arm_callgraph_fp
>>  tools/perf/tests: Fix all POSIX sh warnings in stat+shadow_stat.sh
>> 
>> .../scripts/python/arm-cs-trace-disasm.py     |   1 -
>> tools/perf/tests/shell/buildid.sh             |  12 +-
>> tools/perf/tests/shell/daemon.sh              | 113 ++++++++++++------
>> tools/perf/tests/shell/lock_contention.sh     |  70 +++++------
>> .../shell/record+probe_libc_inet_pton.sh      |   6 +-
>> .../shell/record+script_probe_vfs_getname.sh  |   4 +-
>> tools/perf/tests/shell/stat+csv_output.sh     |   4 +-
>> tools/perf/tests/shell/stat+json_output.sh    |   2 +-
>> tools/perf/tests/shell/stat+shadow_stat.sh    |   4 +-
>> tools/perf/tests/shell/stat_all_metrics.sh    |   6 +-
>> .../perf/tests/shell/test_arm_callgraph_fp.sh |   6 +-
>> tools/perf/tests/shell/test_arm_coresight.sh  |   6 +-
>> tools/perf/tests/shell/test_arm_spe.sh        |   2 +-
>> tools/perf/tests/shell/test_brstack.sh        |  12 +-
>> tools/perf/tests/shell/test_task_analyzer.sh  |  98 ++++++++-------
>> .../tests/shell/trace+probe_vfs_getname.sh    |   6 +-
>> 16 files changed, 203 insertions(+), 149 deletions(-)
>> 
>> -- 
>> 2.39.1
>> 
> 
> -- 
> 
> - Arnaldo