Re: [PATCH bpf-next v2 0/4] Add ftrace direct call for arm64

From: Florent Revest <revest@chromium.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Xu Kuohai <xukuohai@huawei.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Xu Kuohai <xukuohai@huaweicloud.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
	Will Deacon <will@kernel.org>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Ingo Molnar <mingo@redhat.com>, Oleg Nesterov <oleg@redhat.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>, Zi Shen Lim <zlim.lnx@gmail.com>,
	Pasha Tatashin <pasha.tatashin@soleen.com>,
	Ard Biesheuvel <ardb@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Guo Ren <guoren@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>
Subject: Re: [PATCH bpf-next v2 0/4] Add ftrace direct call for arm64
Date: Thu, 6 Oct 2022 18:19:12 +0200	[thread overview]
Message-ID: <CABRcYmL0bDkgYP3tSwhZYaGUSbsUR3Gq85QCRiUAdXt65czzmg@mail.gmail.com> (raw)
In-Reply-To: <20221005113019.18aeda76@gandalf.local.home>

On Wed, Oct 5, 2022 at 5:30 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 5 Oct 2022 17:10:33 +0200
> Florent Revest <revest@chromium.org> wrote:
>
> > On Wed, Oct 5, 2022 at 5:07 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> > >
> > > Can you show the implementation of the indirect call you used?
> >
> > Xu used my development branch here
> > https://github.com/FlorentRevest/linux/commits/fprobe-min-args
>
> That looks like it could be optimized quite a bit too.
>
> Specifically this part:
>
> static bool bpf_fprobe_entry(struct fprobe *fp, unsigned long ip, struct ftrace_regs *regs, void *private)
> {
>         struct bpf_fprobe_call_context *call_ctx = private;
>         struct bpf_fprobe_context *fprobe_ctx = fp->ops.private;
>         struct bpf_tramp_links *links = fprobe_ctx->links;
>         struct bpf_tramp_links *fentry = &links[BPF_TRAMP_FENTRY];
>         struct bpf_tramp_links *fmod_ret = &links[BPF_TRAMP_MODIFY_RETURN];
>         struct bpf_tramp_links *fexit = &links[BPF_TRAMP_FEXIT];
>         int i, ret;
>
>         memset(&call_ctx->ctx, 0, sizeof(call_ctx->ctx));
>         call_ctx->ip = ip;
>         for (i = 0; i < fprobe_ctx->nr_args; i++)
>                 call_ctx->args[i] = ftrace_regs_get_argument(regs, i);
>
>         for (i = 0; i < fentry->nr_links; i++)
>                 call_bpf_prog(fentry->links[i], &call_ctx->ctx, call_ctx->args);
>
>         call_ctx->args[fprobe_ctx->nr_args] = 0;
>         for (i = 0; i < fmod_ret->nr_links; i++) {
>                 ret = call_bpf_prog(fmod_ret->links[i], &call_ctx->ctx,
>                                       call_ctx->args);
>
>                 if (ret) {
>                         ftrace_regs_set_return_value(regs, ret);
>                         ftrace_override_function_with_return(regs);
>
>                         bpf_fprobe_exit(fp, ip, regs, private);
>                         return false;
>                 }
>         }
>
>         return fexit->nr_links;
> }
>
> There's a lot of low hanging fruit to speed up there. I wouldn't be too
> fast to throw out this solution if it hasn't had the care that direct calls
> have had to speed that up.
>
> For example, trampolines currently only allow to attach to functions with 6
> parameters or less (3 on x86_32). You could make 7 specific callbacks, with
> zero to 6 parameters, and unroll the argument loop.

Sure, we can give this a try, I'll work on a macro that generates the
7 callbacks and we can check how much that helps. My belief right now
is that ftrace's iteration over all ops on arm64 is where we lose most
time but now that we have numbers it's pretty easy to check hypothesis
:)