[bpf-next] selftests/bpf: fix ringbuf benchmark output

Commit Message

Andrew Werner July 19, 2023, 1:47 a.m. UTC

The headers were confusing.
---
 tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alexei Starovoitov July 19, 2023, 5:35 p.m. UTC | #1

On Tue, Jul 18, 2023 at 6:50 PM Andrew Werner <[e-mail]> wrote:
>
> The headers were confusing.
> ---

SOB is missing.

commit log is too terse.
Pls explain what you're fixing.

>  tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh b/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
> index 91e3567962ff..8dd97f5108f0 100755
> --- a/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
> +++ b/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
> @@ -6,12 +6,12 @@ set -eufo pipefail
>
>  RUN_RB_BENCH="$RUN_BENCH -c1"
>
> -header "Single-producer, parallel producer"
> +header "Single-consumer, parallel producer"
>  for b in rb-libbpf rb-custom pb-libbpf pb-custom; do
>         summarize $b "$($RUN_RB_BENCH $b)"
>  done
>
> -header "Single-producer, parallel producer, sampled notification"
> +header "Single-consumer, parallel producer, sampled notification"

Single-producer, consumer/producer competing on the same CPU, low batch count

should also be fixed?

Andrew Werner July 19, 2023, 6:57 p.m. UTC | #2

On Wed, Jul 19, 2023 at 1:35 PM Alexei Starovoitov
<[e-mail]> wrote:
>
> On Tue, Jul 18, 2023 at 6:50 PM Andrew Werner <[e-mail]> wrote:
> >
> > The headers were confusing.
> > ---
>
> SOB is missing.

Ack.

>
> commit log is too terse.
> Pls explain what you're fixing.

Ack.

> >  tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh b/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
> > index 91e3567962ff..8dd97f5108f0 100755
> > --- a/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
> > +++ b/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
> > @@ -6,12 +6,12 @@ set -eufo pipefail
> >
> >  RUN_RB_BENCH="$RUN_BENCH -c1"
> >
> > -header "Single-producer, parallel producer"
> > +header "Single-consumer, parallel producer"
> >  for b in rb-libbpf rb-custom pb-libbpf pb-custom; do
> >         summarize $b "$($RUN_RB_BENCH $b)"
> >  done
> >
> > -header "Single-producer, parallel producer, sampled notification"
> > +header "Single-consumer, parallel producer, sampled notification"
>
> Single-producer, consumer/producer competing on the same CPU, low batch count
>
> should also be fixed?

In retrospect, having now reread these, I think that they were not
mistakes. Ringbuf is always implicitly a single consumer concept. The
point was to highlight that there is a single producer, and that it is
running in parallel.

I'll more generally take a more in depth pass at making these headers
clearer and more consistent, and I'll address the other items in v2.
Thanks for the review!

Patch

diff --git a/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh b/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
index 91e3567962ff..8dd97f5108f0 100755
--- a/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
+++ b/tools/testing/selftests/bpf/benchs/run_bench_ringbufs.sh
@@ -6,12 +6,12 @@  set -eufo pipefail
 
 RUN_RB_BENCH="$RUN_BENCH -c1"
 
-header "Single-producer, parallel producer"
+header "Single-consumer, parallel producer"
 for b in rb-libbpf rb-custom pb-libbpf pb-custom; do
 	summarize $b "$($RUN_RB_BENCH $b)"
 done
 
-header "Single-producer, parallel producer, sampled notification"
+header "Single-consumer, parallel producer, sampled notification"
 for b in rb-libbpf rb-custom pb-libbpf pb-custom; do
 	summarize $b "$($RUN_RB_BENCH --rb-sampled $b)"
 done