[PATCH v2 1/3] perf test: Pass the verbose option to shell tests

[PATCH v2 1/3] perf test: Pass the verbose option to shell tests

[Ian Rogers]

Having a verbose option will allow shell tests to provide extra failure
details when the fail or skip.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/builtin-test.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index cbbfe48ab802..e1ed60567b2f 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -577,10 +577,13 @@ struct shell_test {
 static int shell_test__run(struct test *test, int subdir __maybe_unused)
 {
 	int err;
-	char script[PATH_MAX];
+	char script[PATH_MAX + 3];
 	struct shell_test *st = test->priv;
 
-	path__join(script, sizeof(script), st->dir, st->file);
+	path__join(script, sizeof(script) - 3, st->dir, st->file);
+
+	if (verbose)
+		strncat(script, " -v", sizeof(script) - strlen(script) - 1);
 
 	err = system(script);
 	if (!err)
-- 
2.32.0.288.g62a8d224e6-goog

[PATCH v2 2/3] perf test: Add verbose skip output for bpf counters

[Ian Rogers]

Provide additional context for when the stat bpf counters test skips.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/stat_bpf_counters.sh | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh
index 22eb31e48ca7..85eb689fe202 100755
--- a/tools/perf/tests/shell/stat_bpf_counters.sh
+++ b/tools/perf/tests/shell/stat_bpf_counters.sh
@@ -22,7 +22,13 @@ compare_number()
 }
 
 # skip if --bpf-counters is not supported
-perf stat --bpf-counters true > /dev/null 2>&1 || exit 2
+if ! perf stat --bpf-counters true > /dev/null 2>&1; then
+	if [ "$1" == "-v" ]; then
+		echo "Skipping: --bpf-counters not supported"
+		perf --no-pager stat --bpf-counters true || true
+	fi
+	exit 2
+fi
 
 base_cycles=$(perf stat --no-big-num -e cycles -- perf bench sched messaging -g 1 -l 100 -t 2>&1 | awk '/cycles/ {print $1}')
 bpf_cycles=$(perf stat --no-big-num --bpf-counters -e cycles -- perf bench sched messaging -g 1 -l 100 -t 2>&1 | awk '/cycles/ {print $1}')
-- 
2.32.0.288.g62a8d224e6-goog

[PATCH v2 3/3] perf test: Make stat bpf counters test more robust

[Ian Rogers]

If the test is run on a hypervisor then the cycles event may not be
counted, skip the test in this situation. Fail the test if cycles are
not counted in the subsequent bpf counter run.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/shell/stat_bpf_counters.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh
index 85eb689fe202..6b156dd85469 100755
--- a/tools/perf/tests/shell/stat_bpf_counters.sh
+++ b/tools/perf/tests/shell/stat_bpf_counters.sh
@@ -31,7 +31,15 @@ if ! perf stat --bpf-counters true > /dev/null 2>&1; then
 fi
 
 base_cycles=$(perf stat --no-big-num -e cycles -- perf bench sched messaging -g 1 -l 100 -t 2>&1 | awk '/cycles/ {print $1}')
+if [ "$base_cycles" == "<not" ]; then
+	echo "Skipping: cycles event not counted"
+	exit 2
+fi
 bpf_cycles=$(perf stat --no-big-num --bpf-counters -e cycles -- perf bench sched messaging -g 1 -l 100 -t 2>&1 | awk '/cycles/ {print $1}')
+if [ "$bpf_cycles" == "<not" ]; then
+	echo "Failed: cycles not counted with --bpf-counters"
+	exit 1
+fi
 
 compare_number $base_cycles $bpf_cycles
 exit 0
-- 
2.32.0.288.g62a8d224e6-goog

Re: [PATCH v2 1/3] perf test: Pass the verbose option to shell tests

[Arnaldo Carvalho de Melo]

Em Mon, Jun 21, 2021 at 02:56:46PM -0700, Ian Rogers escreveu:
> Having a verbose option will allow shell tests to provide extra failure
> details when the fail or skip.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/builtin-test.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index cbbfe48ab802..e1ed60567b2f 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -577,10 +577,13 @@ struct shell_test {
>  static int shell_test__run(struct test *test, int subdir __maybe_unused)
>  {
>  	int err;
> -	char script[PATH_MAX];
> +	char script[PATH_MAX + 3];

This looks strange, i.e. if it is a _path_ _MAX_, why add 3 chars past
that max when generating a _path_? I'll drop the above hunk and keep the
rest, ok?

- Arnaldo

>  	struct shell_test *st = test->priv;
>  
> -	path__join(script, sizeof(script), st->dir, st->file);
> +	path__join(script, sizeof(script) - 3, st->dir, st->file);
> +
> +	if (verbose)
> +		strncat(script, " -v", sizeof(script) - strlen(script) - 1);
>  
>  	err = system(script);
>  	if (!err)

Re: [PATCH v2 1/3] perf test: Pass the verbose option to shell tests

[Arnaldo Carvalho de Melo]

Em Tue, Jun 22, 2021 at 02:45:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Jun 21, 2021 at 02:56:46PM -0700, Ian Rogers escreveu:
> > Having a verbose option will allow shell tests to provide extra failure
> > details when the fail or skip.
> > 
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/builtin-test.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index cbbfe48ab802..e1ed60567b2f 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -577,10 +577,13 @@ struct shell_test {
> >  static int shell_test__run(struct test *test, int subdir __maybe_unused)
> >  {
> >  	int err;
> > -	char script[PATH_MAX];
> > +	char script[PATH_MAX + 3];
> 
> This looks strange, i.e. if it is a _path_ _MAX_, why add 3 chars past
> that max when generating a _path_? I'll drop the above hunk and keep the
> rest, ok?

Oh well, its not a path after all, its something that is passed to
system(), the use of PATH_MAX seems arbitrary, so your patch wasn't
wrong, but since it is arbitrary, I'll keep it at PATH_MAX and reduce
the patch size 8-)

- Arnaldo

> >  	struct shell_test *st = test->priv;
> >  
> > -	path__join(script, sizeof(script), st->dir, st->file);
> > +	path__join(script, sizeof(script) - 3, st->dir, st->file);
> > +
> > +	if (verbose)
> > +		strncat(script, " -v", sizeof(script) - strlen(script) - 1);
> >  
> >  	err = system(script);
> >  	if (!err)

-- 

- Arnaldo

Re: [PATCH v2 3/3] perf test: Make stat bpf counters test more robust

[Arnaldo Carvalho de Melo]

Em Mon, Jun 21, 2021 at 02:56:48PM -0700, Ian Rogers escreveu:
> If the test is run on a hypervisor then the cycles event may not be
> counted, skip the test in this situation. Fail the test if cycles are
> not counted in the subsequent bpf counter run.

This one was already in,

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/shell/stat_bpf_counters.sh | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh
> index 85eb689fe202..6b156dd85469 100755
> --- a/tools/perf/tests/shell/stat_bpf_counters.sh
> +++ b/tools/perf/tests/shell/stat_bpf_counters.sh
> @@ -31,7 +31,15 @@ if ! perf stat --bpf-counters true > /dev/null 2>&1; then
>  fi
>  
>  base_cycles=$(perf stat --no-big-num -e cycles -- perf bench sched messaging -g 1 -l 100 -t 2>&1 | awk '/cycles/ {print $1}')
> +if [ "$base_cycles" == "<not" ]; then
> +	echo "Skipping: cycles event not counted"
> +	exit 2
> +fi
>  bpf_cycles=$(perf stat --no-big-num --bpf-counters -e cycles -- perf bench sched messaging -g 1 -l 100 -t 2>&1 | awk '/cycles/ {print $1}')
> +if [ "$bpf_cycles" == "<not" ]; then
> +	echo "Failed: cycles not counted with --bpf-counters"
> +	exit 1
> +fi
>  
>  compare_number $base_cycles $bpf_cycles
>  exit 0
> -- 
> 2.32.0.288.g62a8d224e6-goog
> 

-- 

- Arnaldo

Re: [PATCH v2 2/3] perf test: Add verbose skip output for bpf counters

[Arnaldo Carvalho de Melo]

Em Mon, Jun 21, 2021 at 02:56:47PM -0700, Ian Rogers escreveu:
> Provide additional context for when the stat bpf counters test skips.

Ditto
 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/shell/stat_bpf_counters.sh | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh
> index 22eb31e48ca7..85eb689fe202 100755
> --- a/tools/perf/tests/shell/stat_bpf_counters.sh
> +++ b/tools/perf/tests/shell/stat_bpf_counters.sh
> @@ -22,7 +22,13 @@ compare_number()
>  }
>  
>  # skip if --bpf-counters is not supported
> -perf stat --bpf-counters true > /dev/null 2>&1 || exit 2
> +if ! perf stat --bpf-counters true > /dev/null 2>&1; then
> +	if [ "$1" == "-v" ]; then
> +		echo "Skipping: --bpf-counters not supported"
> +		perf --no-pager stat --bpf-counters true || true
> +	fi
> +	exit 2
> +fi
>  
>  base_cycles=$(perf stat --no-big-num -e cycles -- perf bench sched messaging -g 1 -l 100 -t 2>&1 | awk '/cycles/ {print $1}')
>  bpf_cycles=$(perf stat --no-big-num --bpf-counters -e cycles -- perf bench sched messaging -g 1 -l 100 -t 2>&1 | awk '/cycles/ {print $1}')
> -- 
> 2.32.0.288.g62a8d224e6-goog
> 

-- 

- Arnaldo

Re: [PATCH v2 1/3] perf test: Pass the verbose option to shell tests

[Ian Rogers]

On Tue, Jun 22, 2021 at 10:54 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Tue, Jun 22, 2021 at 02:45:51PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Jun 21, 2021 at 02:56:46PM -0700, Ian Rogers escreveu:
> > > Having a verbose option will allow shell tests to provide extra failure
> > > details when the fail or skip.
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > >  tools/perf/tests/builtin-test.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > > index cbbfe48ab802..e1ed60567b2f 100644
> > > --- a/tools/perf/tests/builtin-test.c
> > > +++ b/tools/perf/tests/builtin-test.c
> > > @@ -577,10 +577,13 @@ struct shell_test {
> > >  static int shell_test__run(struct test *test, int subdir __maybe_unused)
> > >  {
> > >     int err;
> > > -   char script[PATH_MAX];
> > > +   char script[PATH_MAX + 3];
> >
> > This looks strange, i.e. if it is a _path_ _MAX_, why add 3 chars past
> > that max when generating a _path_? I'll drop the above hunk and keep the
> > rest, ok?
>
> Oh well, its not a path after all, its something that is passed to
> system(), the use of PATH_MAX seems arbitrary, so your patch wasn't
> wrong, but since it is arbitrary, I'll keep it at PATH_MAX and reduce
> the patch size 8-)
>
> - Arnaldo

Works for me. Thanks,

Ian

> > >     struct shell_test *st = test->priv;
> > >
> > > -   path__join(script, sizeof(script), st->dir, st->file);
> > > +   path__join(script, sizeof(script) - 3, st->dir, st->file);
> > > +
> > > +   if (verbose)
> > > +           strncat(script, " -v", sizeof(script) - strlen(script) - 1);
> > >
> > >     err = system(script);
> > >     if (!err)
>
> --
>
> - Arnaldo