On Tue, Jan 30, 2024 at 11:41 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:
>
>
> On 1/30/24 11:36 AM, Andrii Nakryiko wrote:
> > Some benchmarks don't anticipate "consumer" and/or "producer" sides. Add
>
> For this, you mean some future potential benchmarks, right?
No, existing ones as well. Like trig-tp and other "trigger"
benchmarks. I ran into this when I was trying to set consumers to 0
explicitly, which wasn't allowed due to <= check. Then I fixed the
check, and I ran into SIGSEGV. So I decided to fix that up.
>
> > NULL checks in corresponding places and warn about inappropriate
> > consumer/producer count argument values.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
>
> > ---
> > tools/testing/selftests/bpf/bench.c | 10 +++++++++-
> > 1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/bench.c b/tools/testing/selftests/bpf/bench.c
> > index 73ce11b0547d..36962fc305eb 100644
> > --- a/tools/testing/selftests/bpf/bench.c
> > +++ b/tools/testing/selftests/bpf/bench.c
> > @@ -330,7 +330,7 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
> > break;
> > case 'c':
> > env.consumer_cnt = strtol(arg, NULL, 10);
> > - if (env.consumer_cnt <= 0) {
> > + if (env.consumer_cnt < 0) {
> > fprintf(stderr, "Invalid consumer count: %s\n", arg);
> > argp_usage(state);
> > }
> > @@ -607,6 +607,10 @@ static void setup_benchmark(void)
> > bench->setup();
> >
> > for (i = 0; i < env.consumer_cnt; i++) {
> > + if (!bench->consumer_thread) {
> > + fprintf(stderr, "benchmark doesn't have consumers!\n");
> > + exit(1);
> > + }
> > err = pthread_create(&state.consumers[i], NULL,
> > bench->consumer_thread, (void *)(long)i);
> > if (err) {
> > @@ -626,6 +630,10 @@ static void setup_benchmark(void)
> > env.prod_cpus.next_cpu = env.cons_cpus.next_cpu;
> >
> > for (i = 0; i < env.producer_cnt; i++) {
> > + if (!bench->producer_thread) {
> > + fprintf(stderr, "benchmark doesn't have producers!\n");
> > + exit(1);
> > + }
> > err = pthread_create(&state.producers[i], NULL,
> > bench->producer_thread, (void *)(long)i);
> > if (err) {
>
