From: Veronika Molnarova <vmolnaro@redhat.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: linux-perf-users@vger.kernel.org, acme@redhat.com, mpetlan@redhat.com
Subject: Re: [PATCH v2] perf test stat_all_pmu.sh: Parse return value of perf stat
Date: Wed, 8 May 2024 12:28:59 +0200 [thread overview]
Message-ID: <0374bfb4-8da7-4711-ad0b-0ef57c15715b@redhat.com> (raw)
In-Reply-To: <ZjVIQa3JouJde7I1@x1>
On 5/3/24 22:25, Arnaldo Carvalho de Melo wrote:
> On Mon, Apr 29, 2024 at 12:56:24PM +0200, vmolnaro@redhat.com wrote:
>> From: Veronika Molnarova <vmolnaro@redhat.com>
>>
>> With the upstream MR !3916 of commit ad86d7ee43b22aa2 ('powerpc/hv-gpci:
>> Fix the H_GET_PERF_COUNTER_INFO hcall return value checks') the perf
>> stat for hv_gpci events without required permission set returns an
>> error value of -1 to differentiate the output from the unsupported
>> events. The stat_all_pmu test was designed in a way, that if any
>> command exits with a non-zero value the test exits with an error
>> value without any information provided due to the 'set -e' option.
>
> Is this v2 or v3? Kajol, can I have your tested-by?
>
> Thanks,
>
> - Arnaldo
>
From my point of view it was v2 of the patch, but now I noticed that the a v2
was created when the commit hash was firstly fixed, so maybe its then a v3?
Sorry for confusion,
Veronika
>> Running stat_all_pmu test on powerpc machine with unsupported hv_gpci
>> event causes failure after the MR as the zero return value was required.
>> The issue propagated upstream as the list of the files that affected perf
>> did not cover the changed files and was updated after the issue was
>> discovered. It was caught by CKI testing where it was triaged to stop
>> blocking further MRs, as most of the powerpc machines do not support
>> some of the hv_gpci events.
>>
>> Remove the 'set -e' option from the test and rework the test case to log
>> the status of the event for better maintainability. Instead of exiting
>> immediately after 'perf stat' ends with a non-zero value, check the
>> return value and output of the 'perf stat' command with the appropriate action.
>>
>> Link to the MR !3916 of commit ad86d7ee43b22aa2:
>> https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/3916
>>
>> Signed-off-by: Veronika Molnarova <vmolnaro@redhat.com>
>> ---
>> Fixed the issue with applying due to a fixed comment typo upstream and
>> added Ian's suggestion for the 'device busy' issue during parallel
>> testing.
>>
>> tools/perf/tests/shell/stat_all_pmu.sh | 41 ++++++++++++++++++--------
>> 1 file changed, 29 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/perf/tests/shell/stat_all_pmu.sh b/tools/perf/tests/shell/stat_all_pmu.sh
>> index c77955419173..a75beddda4db 100755
>> --- a/tools/perf/tests/shell/stat_all_pmu.sh
>> +++ b/tools/perf/tests/shell/stat_all_pmu.sh
>> @@ -2,21 +2,38 @@
>> # perf all PMU test
>> # SPDX-License-Identifier: GPL-2.0
>>
>> -set -e
>>
>> # Test all PMU events; however exclude parameterized ones (name contains '?')
>> for p in $(perf list --raw-dump pmu | sed 's/[[:graph:]]\+?[[:graph:]]\+[[:space:]]//g'); do
>> - echo "Testing $p"
>> - result=$(perf stat -e "$p" true 2>&1)
>> - if ! echo "$result" | grep -q "$p" && ! echo "$result" | grep -q "<not supported>" ; then
>> - # We failed to see the event and it is supported. Possibly the workload was
>> - # too small so retry with something longer.
>> - result=$(perf stat -e "$p" perf bench internals synthesize 2>&1)
>> - if ! echo "$result" | grep -q "$p" ; then
>> - echo "Event '$p' not printed in:"
>> - echo "$result"
>> - exit 1
>> + echo -n "Testing event '$p' -- "
>> + stat_output=$(perf stat -e "$p" true 2>&1)
>> + stat_result=$?
>> + if echo "$stat_output" | grep -q "$p"; then
>> + # return value 0 if counters get printed either if the event is supported or not
>> + if [ $stat_result -eq 0 ] && ! echo "$stat_output" | grep -q "<not supported>"; then
>> + echo "supported"
>> + elif [ $stat_result -eq 0 ]; then
>> + echo "not supported"
>> + # return value 255 when the required permission for the event is not set
>> + elif [ $stat_result -eq 255 ] && echo "$stat_output" | grep -q "No permission"; then
>> + echo "no permission to enable"
>> + # return value 255 in case of resource busy during parallel testing of events
>> + elif [ $stat_result -eq 255 ] && echo "$stat_output" | grep -q "Device or resource busy"; then
>> + echo "resource busy"
>> + # return value 129 when trying to run 'perf stat' with a non-existent event
>> + elif [ $stat_result -eq 129 ] && echo "$stat_output" | grep -q "Bad event name"; then
>> + echo "Fail: Bad event name"
>> + echo "$stat_output"
>> + exit 1
>> + else
>> + echo "Fail: Unknown return value $stat_result"
>> + echo "$stat_output"
>> + exit 1
>> fi
>> + else
>> + echo "Fail: Event '$p' not printed in:"
>> + echo "$stat_output"
>> + exit 1
>> fi
>> done
>>
>> --
>> 2.43.0
>>
>
next prev parent reply other threads:[~2024-05-08 10:29 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 13:02 [PATCH] perf test stat_all_pmu.sh: Parse return value of perf stat vmolnaro
2024-04-11 18:18 ` Arnaldo Carvalho de Melo
2024-04-15 9:42 ` [PATCH v2] " vmolnaro
2024-04-15 17:41 ` Arnaldo Carvalho de Melo
2024-04-16 14:33 ` Arnaldo Carvalho de Melo
2024-04-18 7:06 ` kajoljain
2024-04-23 8:01 ` kajoljain
2024-04-26 16:30 ` Veronika Molnarova
2024-04-16 15:03 ` [PATCH] " Ian Rogers
2024-04-16 15:28 ` Arnaldo Carvalho de Melo
2024-04-29 10:56 ` [PATCH v2] " vmolnaro
2024-05-03 20:25 ` Arnaldo Carvalho de Melo
2024-05-08 10:28 ` Veronika Molnarova [this message]
2024-04-26 16:19 ` [PATCH] " Veronika Molnarova
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0374bfb4-8da7-4711-ad0b-0ef57c15715b@redhat.com \
--to=vmolnaro@redhat.com \
--cc=acme@kernel.org \
--cc=acme@redhat.com \
--cc=linux-perf-users@vger.kernel.org \
--cc=mpetlan@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).