* [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters
@ 2021-06-17 18:42 Ian Rogers
2021-06-17 18:42 ` [PATCH 2/4] perf test: Pass the verbose option to shell tests Ian Rogers
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Ian Rogers @ 2021-06-17 18:42 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Song Liu, linux-perf-users, linux-kernel, bpf
Cc: Ian Rogers
$(( .. )) is a bash feature but the test's interpreter is !/bin/sh,
switch the code to use expr.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/shell/stat_bpf_counters.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh
index 22eb31e48ca7..2f9948b3d943 100755
--- a/tools/perf/tests/shell/stat_bpf_counters.sh
+++ b/tools/perf/tests/shell/stat_bpf_counters.sh
@@ -11,9 +11,9 @@ compare_number()
second_num=$2
# upper bound is first_num * 110%
- upper=$(( $first_num + $first_num / 10 ))
+ upper=$(expr $first_num + $first_num / 10 )
# lower bound is first_num * 90%
- lower=$(( $first_num - $first_num / 10 ))
+ lower=$(expr $first_num - $first_num / 10 )
if [ $second_num -gt $upper ] || [ $second_num -lt $lower ]; then
echo "The difference between $first_num and $second_num are greater than 10%."
--
2.32.0.288.g62a8d224e6-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] perf test: Pass the verbose option to shell tests
2021-06-17 18:42 [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Ian Rogers
@ 2021-06-17 18:42 ` Ian Rogers
2021-06-17 19:19 ` Arnaldo Carvalho de Melo
2021-06-17 18:42 ` [PATCH 3/4] perf test: Add verbose skip output for bpf counters Ian Rogers
` (3 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2021-06-17 18:42 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Song Liu, linux-perf-users, linux-kernel, bpf
Cc: 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 | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index cbbfe48ab802..a8160b1684de 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -577,11 +577,14 @@ 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);
+ if (verbose)
+ strncat(script, " -v", sizeof(script));
+
err = system(script);
if (!err)
return TEST_OK;
--
2.32.0.288.g62a8d224e6-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] perf test: Add verbose skip output for bpf counters
2021-06-17 18:42 [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Ian Rogers
2021-06-17 18:42 ` [PATCH 2/4] perf test: Pass the verbose option to shell tests Ian Rogers
@ 2021-06-17 18:42 ` Ian Rogers
2021-06-17 19:20 ` Arnaldo Carvalho de Melo
2021-06-17 18:42 ` [PATCH 4/4] perf test: Make stat bpf counters test more robust Ian Rogers
` (2 subsequent siblings)
4 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2021-06-17 18:42 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Song Liu, linux-perf-users, linux-kernel, bpf
Cc: 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 2f9948b3d943..81d61b6e1208 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] perf test: Make stat bpf counters test more robust
2021-06-17 18:42 [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Ian Rogers
2021-06-17 18:42 ` [PATCH 2/4] perf test: Pass the verbose option to shell tests Ian Rogers
2021-06-17 18:42 ` [PATCH 3/4] perf test: Add verbose skip output for bpf counters Ian Rogers
@ 2021-06-17 18:42 ` Ian Rogers
2021-06-17 19:21 ` Arnaldo Carvalho de Melo
2021-06-17 19:18 ` [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Arnaldo Carvalho de Melo
2021-06-20 21:55 ` Stephen Rothwell
4 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2021-06-17 18:42 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Song Liu, linux-perf-users, linux-kernel, bpf
Cc: 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 81d61b6e1208..2aed20dc2262 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters
2021-06-17 18:42 [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Ian Rogers
` (2 preceding siblings ...)
2021-06-17 18:42 ` [PATCH 4/4] perf test: Make stat bpf counters test more robust Ian Rogers
@ 2021-06-17 19:18 ` Arnaldo Carvalho de Melo
2021-06-20 21:55 ` Stephen Rothwell
4 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-17 19:18 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Song Liu, linux-perf-users,
linux-kernel, bpf
Em Thu, Jun 17, 2021 at 11:42:13AM -0700, Ian Rogers escreveu:
> $(( .. )) is a bash feature but the test's interpreter is !/bin/sh,
> switch the code to use expr.
Thanks, applied to perf/urgent.
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/tests/shell/stat_bpf_counters.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh
> index 22eb31e48ca7..2f9948b3d943 100755
> --- a/tools/perf/tests/shell/stat_bpf_counters.sh
> +++ b/tools/perf/tests/shell/stat_bpf_counters.sh
> @@ -11,9 +11,9 @@ compare_number()
> second_num=$2
>
> # upper bound is first_num * 110%
> - upper=$(( $first_num + $first_num / 10 ))
> + upper=$(expr $first_num + $first_num / 10 )
> # lower bound is first_num * 90%
> - lower=$(( $first_num - $first_num / 10 ))
> + lower=$(expr $first_num - $first_num / 10 )
>
> if [ $second_num -gt $upper ] || [ $second_num -lt $lower ]; then
> echo "The difference between $first_num and $second_num are greater than 10%."
> --
> 2.32.0.288.g62a8d224e6-goog
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] perf test: Pass the verbose option to shell tests
2021-06-17 18:42 ` [PATCH 2/4] perf test: Pass the verbose option to shell tests Ian Rogers
@ 2021-06-17 19:19 ` Arnaldo Carvalho de Melo
2021-06-18 16:49 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-17 19:19 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Song Liu, linux-perf-users,
linux-kernel, bpf
Em Thu, Jun 17, 2021 at 11:42:14AM -0700, Ian Rogers escreveu:
> Having a verbose option will allow shell tests to provide extra failure
> details when the fail or skip.
Thanks, applied to perf/core.
- Arnaldo
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/tests/builtin-test.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index cbbfe48ab802..a8160b1684de 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -577,11 +577,14 @@ 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);
>
> + if (verbose)
> + strncat(script, " -v", sizeof(script));
> +
> err = system(script);
> if (!err)
> return TEST_OK;
> --
> 2.32.0.288.g62a8d224e6-goog
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] perf test: Add verbose skip output for bpf counters
2021-06-17 18:42 ` [PATCH 3/4] perf test: Add verbose skip output for bpf counters Ian Rogers
@ 2021-06-17 19:20 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-17 19:20 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Song Liu, linux-perf-users,
linux-kernel, bpf
Em Thu, Jun 17, 2021 at 11:42:15AM -0700, Ian Rogers escreveu:
> Provide additional context for when the stat bpf counters test skips.
Thanks, applied.
- Arnaldo
> 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 2f9948b3d943..81d61b6e1208 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] perf test: Make stat bpf counters test more robust
2021-06-17 18:42 ` [PATCH 4/4] perf test: Make stat bpf counters test more robust Ian Rogers
@ 2021-06-17 19:21 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-17 19:21 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Song Liu, linux-perf-users,
linux-kernel, bpf
Em Thu, Jun 17, 2021 at 11:42:16AM -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.
Thanks, applied.
- 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 81d61b6e1208..2aed20dc2262 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] perf test: Pass the verbose option to shell tests
2021-06-17 19:19 ` Arnaldo Carvalho de Melo
@ 2021-06-18 16:49 ` Arnaldo Carvalho de Melo
2021-06-19 4:45 ` Ian Rogers
0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-18 16:49 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Song Liu, linux-perf-users,
linux-kernel, bpf
Em Thu, Jun 17, 2021 at 04:19:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Jun 17, 2021 at 11:42:14AM -0700, Ian Rogers escreveu:
> > Having a verbose option will allow shell tests to provide extra failure
> > details when the fail or skip.
>
>
> Thanks, applied to perf/core.
>
> - Arnaldo
>
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/tests/builtin-test.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index cbbfe48ab802..a8160b1684de 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -577,11 +577,14 @@ 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);
probably you need to add a '- 3' after the sizeof above, right?
> >
> > + if (verbose)
> > + strncat(script, " -v", sizeof(script));
> > +
Seemed simple enough, but gcc knows better, I'm removing this one:
tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size]
strncat(script, " -v", sizeof(script));
^~~~~~~~~~~~~~
tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte
strncat(script, " -v", sizeof(script));
^~~~~~~~~~~~~~
sizeof(script) - strlen(script) - 1
1 error generated.
make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2
77 31.98 ubuntu:21.04 : FAIL gcc version 10.3.0 (Ubuntu 10.3.0-1ubuntu1)
tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size]
strncat(script, " -v", sizeof(script));
^~~~~~~~~~~~~~
tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte
strncat(script, " -v", sizeof(script));
^~~~~~~~~~~~~~
sizeof(script) - strlen(script) - 1
1 error generated.
make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2
> > err = system(script);
> > if (!err)
> > return TEST_OK;
> > --
> > 2.32.0.288.g62a8d224e6-goog
> >
>
> --
>
> - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] perf test: Pass the verbose option to shell tests
2021-06-18 16:49 ` Arnaldo Carvalho de Melo
@ 2021-06-19 4:45 ` Ian Rogers
2021-06-19 13:03 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2021-06-19 4:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Song Liu, linux-perf-users,
linux-kernel, bpf
On Fri, Jun 18, 2021 at 9:49 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Thu, Jun 17, 2021 at 04:19:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Jun 17, 2021 at 11:42:14AM -0700, Ian Rogers escreveu:
> > > Having a verbose option will allow shell tests to provide extra failure
> > > details when the fail or skip.
> >
> >
> > Thanks, applied to perf/core.
> >
> > - Arnaldo
> >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > > tools/perf/tests/builtin-test.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > > index cbbfe48ab802..a8160b1684de 100644
> > > --- a/tools/perf/tests/builtin-test.c
> > > +++ b/tools/perf/tests/builtin-test.c
> > > @@ -577,11 +577,14 @@ 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);
>
> probably you need to add a '- 3' after the sizeof above, right?
Either way is fine, but -3 is ok with me.
> > >
> > > + if (verbose)
> > > + strncat(script, " -v", sizeof(script));
> > > +
>
> Seemed simple enough, but gcc knows better, I'm removing this one:
>
> tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size]
> strncat(script, " -v", sizeof(script));
> ^~~~~~~~~~~~~~
> tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte
> strncat(script, " -v", sizeof(script));
> ^~~~~~~~~~~~~~
> sizeof(script) - strlen(script) - 1
> 1 error generated.
> make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2
> 77 31.98 ubuntu:21.04 : FAIL gcc version 10.3.0 (Ubuntu 10.3.0-1ubuntu1)
> tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size]
> strncat(script, " -v", sizeof(script));
> ^~~~~~~~~~~~~~
> tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte
> strncat(script, " -v", sizeof(script));
> ^~~~~~~~~~~~~~
> sizeof(script) - strlen(script) - 1
> 1 error generated.
> make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2
Thanks gcc :-) Do you want me to resend the patch?
Ian
> > > err = system(script);
> > > if (!err)
> > > return TEST_OK;
> > > --
> > > 2.32.0.288.g62a8d224e6-goog
> > >
> >
> > --
> >
> > - Arnaldo
>
> --
>
> - Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] perf test: Pass the verbose option to shell tests
2021-06-19 4:45 ` Ian Rogers
@ 2021-06-19 13:03 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-06-19 13:03 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
Jiri Olsa, Namhyung Kim, Song Liu, linux-perf-users,
linux-kernel, bpf
Em Fri, Jun 18, 2021 at 09:45:05PM -0700, Ian Rogers escreveu:
> On Fri, Jun 18, 2021 at 9:49 AM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Thu, Jun 17, 2021 at 04:19:06PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Thu, Jun 17, 2021 at 11:42:14AM -0700, Ian Rogers escreveu:
> > > > Having a verbose option will allow shell tests to provide extra failure
> > > > details when the fail or skip.
> > >
> > >
> > > Thanks, applied to perf/core.
> > >
> > > - Arnaldo
> > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > ---
> > > > tools/perf/tests/builtin-test.c | 5 ++++-
> > > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > > > index cbbfe48ab802..a8160b1684de 100644
> > > > --- a/tools/perf/tests/builtin-test.c
> > > > +++ b/tools/perf/tests/builtin-test.c
> > > > @@ -577,11 +577,14 @@ 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);
> >
> > probably you need to add a '- 3' after the sizeof above, right?
>
> Either way is fine, but -3 is ok with me.
>
> > > >
> > > > + if (verbose)
> > > > + strncat(script, " -v", sizeof(script));
> > > > +
> >
> > Seemed simple enough, but gcc knows better, I'm removing this one:
> >
> > tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size]
> > strncat(script, " -v", sizeof(script));
> > ^~~~~~~~~~~~~~
> > tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte
> > strncat(script, " -v", sizeof(script));
> > ^~~~~~~~~~~~~~
> > sizeof(script) - strlen(script) - 1
> > 1 error generated.
> > make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2
> > 77 31.98 ubuntu:21.04 : FAIL gcc version 10.3.0 (Ubuntu 10.3.0-1ubuntu1)
> > tests/builtin-test.c:586:26: error: the value of the size argument in 'strncat' is too large, might lead to a buffer overflow [-Werror,-Wstrncat-size]
> > strncat(script, " -v", sizeof(script));
> > ^~~~~~~~~~~~~~
> > tests/builtin-test.c:586:26: note: change the argument to be the free space in the destination buffer minus the terminating null byte
> > strncat(script, " -v", sizeof(script));
> > ^~~~~~~~~~~~~~
> > sizeof(script) - strlen(script) - 1
> > 1 error generated.
> > make[3]: *** [/git/perf-5.13.0-rc4/tools/build/Makefile.build:139: tests] Error 2
>
> Thanks gcc :-) Do you want me to resend the patch?
In such cases please do,
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters
2021-06-17 18:42 [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Ian Rogers
` (3 preceding siblings ...)
2021-06-17 19:18 ` [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Arnaldo Carvalho de Melo
@ 2021-06-20 21:55 ` Stephen Rothwell
2021-06-21 21:30 ` Ian Rogers
4 siblings, 1 reply; 13+ messages in thread
From: Stephen Rothwell @ 2021-06-20 21:55 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Song Liu, linux-perf-users, linux-kernel, bpf
[-- Attachment #1: Type: text/plain, Size: 437 bytes --]
Hi Ian,
On Thu, 17 Jun 2021 11:42:13 -0700 Ian Rogers <irogers@google.com> wrote:
>
> $(( .. )) is a bash feature but the test's interpreter is !/bin/sh,
> switch the code to use expr.
The $(( .. )) syntax is specified in POSIX (see
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_04),
so unless this caused an actual problem, this change is unnecessary.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters
2021-06-20 21:55 ` Stephen Rothwell
@ 2021-06-21 21:30 ` Ian Rogers
0 siblings, 0 replies; 13+ messages in thread
From: Ian Rogers @ 2021-06-21 21:30 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Song Liu, linux-perf-users, linux-kernel, bpf
On Sun, Jun 20, 2021 at 2:55 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Hi Ian,
>
> On Thu, 17 Jun 2021 11:42:13 -0700 Ian Rogers <irogers@google.com> wrote:
> >
> > $(( .. )) is a bash feature but the test's interpreter is !/bin/sh,
> > switch the code to use expr.
>
> The $(( .. )) syntax is specified in POSIX (see
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_04),
> so unless this caused an actual problem, this change is unnecessary.
Agreed. The issue I was seeing was:
./tests/shell/stat_bpf_counters.sh: line 14: <not + <not / 10 : syntax
error: operand expected (error token is "<not + <not / 10 ")
but that syntax error is caused by running the test within a
hypervisor. I'll resend the patch set with this one dropped.
Thanks,
Ian
> --
> Cheers,
> Stephen Rothwell
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-06-21 21:31 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-17 18:42 [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Ian Rogers
2021-06-17 18:42 ` [PATCH 2/4] perf test: Pass the verbose option to shell tests Ian Rogers
2021-06-17 19:19 ` Arnaldo Carvalho de Melo
2021-06-18 16:49 ` Arnaldo Carvalho de Melo
2021-06-19 4:45 ` Ian Rogers
2021-06-19 13:03 ` Arnaldo Carvalho de Melo
2021-06-17 18:42 ` [PATCH 3/4] perf test: Add verbose skip output for bpf counters Ian Rogers
2021-06-17 19:20 ` Arnaldo Carvalho de Melo
2021-06-17 18:42 ` [PATCH 4/4] perf test: Make stat bpf counters test more robust Ian Rogers
2021-06-17 19:21 ` Arnaldo Carvalho de Melo
2021-06-17 19:18 ` [PATCH 1/4] perf test: Fix non-bash issue with stat bpf counters Arnaldo Carvalho de Melo
2021-06-20 21:55 ` Stephen Rothwell
2021-06-21 21:30 ` Ian Rogers
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 NNTP newsgroup(s).