[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADxym3bp-KeFX4Mxo8Xhh9_NUWp5hw_bTQBOhTgJzskH7d_k_A@mail.gmail.com>
Date: Mon, 5 Jun 2023 10:49:54 +0800
From: Menglong Dong <menglong8.dong@...il.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Jiri Olsa <olsajiri@...il.com>,
"David S. Miller" <davem@...emloft.net>,
David Ahern <dsahern@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <martin.lau@...ux.dev>,
Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
KP Singh <kpsingh@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
X86 ML <x86@...nel.org>, Biao Jiang <benbjiang@...cent.com>,
Network Development <netdev@...r.kernel.org>,
bpf <bpf@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH bpf-next v2 1/5] bpf: make MAX_BPF_FUNC_ARGS 14
On Sat, Jun 3, 2023 at 2:17 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Fri, Jun 2, 2023 at 12:01 AM <menglong8.dong@...il.com> wrote:
> >
> > From: Menglong Dong <imagedong@...cent.com>
> >
> > According to the current kernel version, below is a statistics of the
> > function arguments count:
> >
> > argument count | FUNC_PROTO count
> > 7 | 367
> > 8 | 196
> > 9 | 71
> > 10 | 43
> > 11 | 22
> > 12 | 10
> > 13 | 15
> > 14 | 4
> > 15 | 0
> > 16 | 1
> >
> > It's hard to statisics the function count, so I use FUNC_PROTO in the btf
> > of vmlinux instead. The function with 16 arguments is ZSTD_buildCTable(),
> > which I think can be ignored.
> >
> > Therefore, let's make the maximum of function arguments count 14. It used
> > to be 12, but it seems that there is no harm to make it big enough.
>
> I think we're just fine at 12.
> People need to fix their code. ZSTD_buildCTable should be first in line.
> Passing arguments on the stack is not efficient from performance pov.
But we still need to keep this part:
@@ -2273,7 +2273,8 @@ bool btf_ctx_access(int off, int size, enum
bpf_access_type type,
static inline bool bpf_tracing_ctx_access(int off, int size,
enum bpf_access_type type)
{
- if (off < 0 || off >= sizeof(__u64) * MAX_BPF_FUNC_ARGS)
+ /* "+1" here is for FEXIT return value. */
+ if (off < 0 || off >= sizeof(__u64) * (MAX_BPF_FUNC_ARGS + 1))
return false;
if (type != BPF_READ)
return false;
Isn't it? Otherwise, it will make that the maximum arguments
is 12 for FENTRY, but 11 for FEXIT, as FEXIT needs to store
the return value in ctx.
How about that we change bpf_tracing_ctx_access() into:
static inline bool bpf_tracing_ctx_access(int off, int size,
enum bpf_access_type type,
int max_args)
And the caller can pass MAX_BPF_FUNC_ARGS to
it on no-FEXIT, and 'MAX_BPF_FUNC_ARGS + 1'
on FEXIT.
What do you think?
Thanks!
Menglong Dong
Powered by blists - more mailing lists