Re: [PATCH V3] tools/perf/tests: Fix string substitutions in build id test
Em Fri, Jan 20, 2023 at 08:51:59AM +, David Laight escreveu: > From: Arnaldo Carvalho de Melo > > Sent: 19 January 2023 17:00 > > > > Em Thu, Jan 19, 2023 at 03:49:15PM +, David Laight escreveu: > > > From: Athira Rajeev > > > > Sent: 19 January 2023 14:27 > > > ... > > > > The test script "tests/shell/buildid.sh" uses some of the > > > > string substitution ways which are supported in bash, but not in > > > > "sh" or other shells. Above error on line number 69 that reports > > > > "Bad substitution" is: > > > > > > Looks better - assuming it works :-) > > > > :-) > > > > Can I take this as an: > > > > Acked-by: David Laight > > I'm not sure that is worth anything. > You can add a Reviewed-by Thanks, I'll then add: Reviewed-by: David Laight - Arnaldo
RE: [PATCH V3] tools/perf/tests: Fix string substitutions in build id test
From: Arnaldo Carvalho de Melo > Sent: 19 January 2023 17:00 > > Em Thu, Jan 19, 2023 at 03:49:15PM +, David Laight escreveu: > > From: Athira Rajeev > > > Sent: 19 January 2023 14:27 > > ... > > > The test script "tests/shell/buildid.sh" uses some of the > > > string substitution ways which are supported in bash, but not in > > > "sh" or other shells. Above error on line number 69 that reports > > > "Bad substitution" is: > > > > Looks better - assuming it works :-) > > :-) > > Can I take this as an: > > Acked-by: David Laight I'm not sure that is worth anything. You can add a Reviewed-by David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: [PATCH V3] tools/perf/tests: Fix string substitutions in build id test
Environment with /bin/sh
# readlink -f $(which sh)
/bin/dash
Running perf test from tmp.perf/urgent
# ./perf test -v "build id cache operations"
73: build id cache operations :
--- start ---
test child forked, pid 71063
WARNING: wine not found. PE binaries will not be run.
test binaries: /tmp/perf.ex.SHA1.RNm /tmp/perf.ex.MD5.Flx
./tests/shell/../pe-file.exe
DEBUGINFOD_URLS=
Adding 4abd406f041feb4f10ecde3fc30fd0639e1a91cb /tmp/perf.ex.SHA1.RNm: Ok
build id: 4abd406f041feb4f10ecde3fc30fd0639e1a91cb
./tests/shell/buildid.sh: 69: ./tests/shell/buildid.sh: Bad substitution
test child finished with -2
end
After applying the patch in same environment, error is not seen and test runs
fine.
Tested the same patch in bash as well.
# readlink -f $(which sh)
/usr/bin/bash
The test works fine with the changes.
Tested-by: Disha Goel
On 1/19/23 7:57 PM, Athira Rajeev wrote:
The perf test named “build id cache operations” skips with below
error on some distros:
<<>>
78: build id cache operations :
test child forked, pid 01
WARNING: wine not found. PE binaries will not be run.
test binaries: /tmp/perf.ex.SHA1.PKz /tmp/perf.ex.MD5.Gt3
./tests/shell/../pe-file.exe
DEBUGINFOD_URLS=
Adding 4abd406f041feb4f10ecde3fc30fd0639e1a91cb /tmp/perf.ex.SHA1.PKz: Ok
build id: 4abd406f041feb4f10ecde3fc30fd0639e1a91cb
./tests/shell/buildid.sh: 69: ./tests/shell/buildid.sh: Bad substitution
test child finished with -2
build id cache operations: Skip
<<>>
The test script "tests/shell/buildid.sh" uses some of the
string substitution ways which are supported in bash, but not in
"sh" or other shells. Above error on line number 69 that reports
"Bad substitution" is:
<<>>
link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
<<>>
Here the way of getting first two characters from id ie,
${id:0:2} and similarly expressions like ${id:2} is not
recognised in "sh". So the line errors and instead of
hitting failure, the test gets skipped as shown in logs.
So the syntax issue causes test not to be executed in
such cases. Similarly usage : "${@: -1}" [ to pick last
argument passed to a function] in “test_record” doesn’t
work in all distros.
Fix this by using alternative way with shell substitution
to pick required characters from the string. Also fix the
usage of “${@: -1}” to work in all cases.
Another usage in “test_record” is:
<<>>
${perf} record --buildid-all -o ${data} $@ &> ${log}
<<>>
This causes the perf record to start in background and
Results in the data file not being created by the time
"check" function is invoked. Below log shows perf record
result getting displayed after the call to "check" function.
<<>>
running: perf record /tmp/perf.ex.SHA1.EAU
build id: 4abd406f041feb4f10ecde3fc30fd0639e1a91cb
link: /tmp/perf.debug.mLT/.build-id/4a/bd406f041feb4f10ecde3fc30fd0639e1a91cb
failed: link
/tmp/perf.debug.mLT/.build-id/4a/bd406f041feb4f10ecde3fc30fd0639e1a91cb does
not exist
test child finished with -1
build id cache operations: FAILED!
root@machine:~/athira/linux/tools/perf# Couldn't synthesize bpf events.
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.010 MB /tmp/perf.data.bFF ]
<<>>
Fix this by redirecting output instead of using “&” which
starts the command in background.
Signed-off-by: Athira Rajeev
Acked-by: Ian Rogers
---
Changelog:
From v2 -> v3
- Addressed review comments from David Laight
for using shell substitutions.
From v1 -> v2
- Added Acked-by from Ian.
- Rebased to tmp.perf/urgent of:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
tools/perf/tests/shell/buildid.sh | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/perf/tests/shell/buildid.sh
b/tools/perf/tests/shell/buildid.sh
index aaf851108ca3..0ce22ea0a7f1 100755
--- a/tools/perf/tests/shell/buildid.sh
+++ b/tools/perf/tests/shell/buildid.sh
@@ -66,7 +66,9 @@ check()
esac
echo "build id: ${id}"
- link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
+ id_file=${id#??}
+ id_dir=${id%$id_file}
+ link=$build_id_dir/.build-id/$id_dir/$id_file
echo "link: ${link}" if [ ! -h $link ]; then @@ -74,7 +76,7 @@ check() exit 1 fi -
file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf +
file=${build_id_dir}/.build-id/$id_dir/`readlink ${link}`/elf echo "file: ${file}"
# Check for file permission of original file
@@ -130,20 +132,22 @@ test_record()
{
data=$(mktemp /tmp/perf.data.XXX)
build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
- log=$(mktemp /tmp/perf.log.XXX)
+ log_out=$(mktemp /tmp/perf.log.out.XXX)
+ 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} $@ &> ${log}
+ ${perf} record --buildid-all -o ${data} $@ 1>${log_out} 2>${log_err}
if [
Re: [PATCH V3] tools/perf/tests: Fix string substitutions in build id test
Em Thu, Jan 19, 2023 at 03:49:15PM +, David Laight escreveu: > From: Athira Rajeev > > Sent: 19 January 2023 14:27 > ... > > The test script "tests/shell/buildid.sh" uses some of the > > string substitution ways which are supported in bash, but not in > > "sh" or other shells. Above error on line number 69 that reports > > "Bad substitution" is: > > Looks better - assuming it works :-) :-) Can I take this as an: Acked-by: David Laight ? - Arnaldo
RE: [PATCH V3] tools/perf/tests: Fix string substitutions in build id test
From: Athira Rajeev > Sent: 19 January 2023 14:27 ... > The test script "tests/shell/buildid.sh" uses some of the > string substitution ways which are supported in bash, but not in > "sh" or other shells. Above error on line number 69 that reports > "Bad substitution" is: Looks better - assuming it works :-) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH V3] tools/perf/tests: Fix string substitutions in build id test
The perf test named “build id cache operations” skips with below
error on some distros:
<<>>
78: build id cache operations :
test child forked, pid 01
WARNING: wine not found. PE binaries will not be run.
test binaries: /tmp/perf.ex.SHA1.PKz /tmp/perf.ex.MD5.Gt3
./tests/shell/../pe-file.exe
DEBUGINFOD_URLS=
Adding 4abd406f041feb4f10ecde3fc30fd0639e1a91cb /tmp/perf.ex.SHA1.PKz: Ok
build id: 4abd406f041feb4f10ecde3fc30fd0639e1a91cb
./tests/shell/buildid.sh: 69: ./tests/shell/buildid.sh: Bad substitution
test child finished with -2
build id cache operations: Skip
<<>>
The test script "tests/shell/buildid.sh" uses some of the
string substitution ways which are supported in bash, but not in
"sh" or other shells. Above error on line number 69 that reports
"Bad substitution" is:
<<>>
link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
<<>>
Here the way of getting first two characters from id ie,
${id:0:2} and similarly expressions like ${id:2} is not
recognised in "sh". So the line errors and instead of
hitting failure, the test gets skipped as shown in logs.
So the syntax issue causes test not to be executed in
such cases. Similarly usage : "${@: -1}" [ to pick last
argument passed to a function] in “test_record” doesn’t
work in all distros.
Fix this by using alternative way with shell substitution
to pick required characters from the string. Also fix the
usage of “${@: -1}” to work in all cases.
Another usage in “test_record” is:
<<>>
${perf} record --buildid-all -o ${data} $@ &> ${log}
<<>>
This causes the perf record to start in background and
Results in the data file not being created by the time
"check" function is invoked. Below log shows perf record
result getting displayed after the call to "check" function.
<<>>
running: perf record /tmp/perf.ex.SHA1.EAU
build id: 4abd406f041feb4f10ecde3fc30fd0639e1a91cb
link: /tmp/perf.debug.mLT/.build-id/4a/bd406f041feb4f10ecde3fc30fd0639e1a91cb
failed: link
/tmp/perf.debug.mLT/.build-id/4a/bd406f041feb4f10ecde3fc30fd0639e1a91cb does
not exist
test child finished with -1
build id cache operations: FAILED!
root@machine:~/athira/linux/tools/perf# Couldn't synthesize bpf events.
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.010 MB /tmp/perf.data.bFF ]
<<>>
Fix this by redirecting output instead of using “&” which
starts the command in background.
Signed-off-by: Athira Rajeev
Acked-by: Ian Rogers
---
Changelog:
>From v2 -> v3
- Addressed review comments from David Laight
for using shell substitutions.
>From v1 -> v2
- Added Acked-by from Ian.
- Rebased to tmp.perf/urgent of:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git
tools/perf/tests/shell/buildid.sh | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/tools/perf/tests/shell/buildid.sh
b/tools/perf/tests/shell/buildid.sh
index aaf851108ca3..0ce22ea0a7f1 100755
--- a/tools/perf/tests/shell/buildid.sh
+++ b/tools/perf/tests/shell/buildid.sh
@@ -66,7 +66,9 @@ check()
esac
echo "build id: ${id}"
- link=${build_id_dir}/.build-id/${id:0:2}/${id:2}
+ id_file=${id#??}
+ id_dir=${id%$id_file}
+ link=$build_id_dir/.build-id/$id_dir/$id_file
echo "link: ${link}"
if [ ! -h $link ]; then
@@ -74,7 +76,7 @@ check()
exit 1
fi
- file=${build_id_dir}/.build-id/${id:0:2}/`readlink ${link}`/elf
+ file=${build_id_dir}/.build-id/$id_dir/`readlink ${link}`/elf
echo "file: ${file}"
# Check for file permission of original file
@@ -130,20 +132,22 @@ test_record()
{
data=$(mktemp /tmp/perf.data.XXX)
build_id_dir=$(mktemp -d /tmp/perf.debug.XXX)
- log=$(mktemp /tmp/perf.log.XXX)
+ log_out=$(mktemp /tmp/perf.log.out.XXX)
+ 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} $@ &> ${log}
+ ${perf} record --buildid-all -o ${data} $@ 1>${log_out} 2>${log_err}
if [ $? -ne 0 ]; then
echo "failed: record $@"
- echo "see log: ${log}"
+ echo "see log: ${log_err}"
exit 1
fi
- check ${@: -1}
+ args="$*"
+ check ${args##* }
- rm -f ${log}
+ rm -f ${log_out} ${log_err}
rm -rf ${build_id_dir}
rm -rf ${data}
}
--
2.39.0
