| Message ID | 20230602065958.2869555-2-imagedong@tencent.com |
|---|---|
| State | New |
| Headers | show |
| Series | [bpf-next,v2,1/5] bpf: make MAX_BPF_FUNC_ARGS 14 | expand |
On Fri, Jun 02, 2023 at 02:59:54PM +0800, menglong8.dong@gmail.com wrote: > From: Menglong Dong <imagedong@tencent.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. > > Reviewed-by: Jiang Biao <benbjiang@tencent.com> > Signed-off-by: Menglong Dong <imagedong@tencent.com> > --- > include/linux/bpf.h | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index f58895830ada..8b997779faf7 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -961,10 +961,10 @@ enum bpf_cgroup_storage_type { > > #define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX > > -/* The longest tracepoint has 12 args. > - * See include/trace/bpf_probe.h > +/* The maximun number of the kernel function arguments. Hi Menglong Dong, as it looks like there will be a v3 anyway, please consider correcting the spelling of maximum. ...
On Sat, Jun 3, 2023 at 2:17 AM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Jun 2, 2023 at 12:01 AM <menglong8.dong@gmail.com> wrote: > > > > From: Menglong Dong <imagedong@tencent.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
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index f58895830ada..8b997779faf7 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -961,10 +961,10 @@ enum bpf_cgroup_storage_type { #define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX -/* The longest tracepoint has 12 args. - * See include/trace/bpf_probe.h +/* The maximun number of the kernel function arguments. + * Let's make it 14, for now. */ -#define MAX_BPF_FUNC_ARGS 12 +#define MAX_BPF_FUNC_ARGS 14 /* The maximum number of arguments passed through registers * a single function may have. @@ -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;