Re: [PATCH bpf-next v8 11/11] selftests: Remove fmod_ret from benchmarks and test_overhead - Toke Høiland-Jørgensen

From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
	Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Eelco Chaudron <echaudro@redhat.com>,
	KP Singh <kpsingh@chromium.org>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next v8 11/11] selftests: Remove fmod_ret from benchmarks and test_overhead
Date: Fri, 25 Sep 2020 01:19:22 +0200	[thread overview]
Message-ID: <87k0wibw5h.fsf@toke.dk> (raw)
In-Reply-To: <CAEf4BzZ8f9osvJ3CD7kL4-SmdH9v_EQ73A9c=oGOBgeTziiFzw@mail.gmail.com>

Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:

> On Wed, Sep 23, 2020 at 6:08 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
>>
>> On Tue, Sep 22, 2020 at 08:38:45PM +0200, Toke Høiland-Jørgensen wrote:
>> > -const struct bench bench_trig_fmodret = {
>> > -     .name = "trig-fmodret",
>> > -     .validate = trigger_validate,
>> > -     .setup = trigger_fmodret_setup,
>> > -     .producer_thread = trigger_producer,
>> > -     .consumer_thread = trigger_consumer,
>> > -     .measure = trigger_measure,
>> > -     .report_progress = hits_drops_report_progress,
>> > -     .report_final = hits_drops_report_final,
>> > -};
>> > diff --git a/tools/testing/selftests/bpf/progs/trigger_bench.c b/tools/testing/selftests/bpf/progs/trigger_bench.c
>> > index 9a4d09590b3d..1af23ac0c37c 100644
>> > --- a/tools/testing/selftests/bpf/progs/trigger_bench.c
>> > +++ b/tools/testing/selftests/bpf/progs/trigger_bench.c
>> > @@ -45,10 +45,3 @@ int bench_trigger_fentry_sleep(void *ctx)
>> >       __sync_add_and_fetch(&hits, 1);
>> >       return 0;
>> >  }
>> > -
>> > -SEC("fmod_ret/__x64_sys_getpgid")
>> > -int bench_trigger_fmodret(void *ctx)
>> > -{
>> > -     __sync_add_and_fetch(&hits, 1);
>> > -     return -22;
>> > -}
>>
>> why are you removing this? There is no problem here.
>> All syscalls are error-injectable.
>> I'm surprised Andrii acked this :(
>
> Andrii didn't know that all syscalls are error-injectable, thanks for
> catching :) after fmod_ret/__set_task_comm I just assumed that I've
> been abusing fmod_ret all this time...

I didn't know that either. Shall I just drop your ACK from the next
version so you can take another look?

-Toke