Re: [PATCH v3 bpf-next 5/5] selftests/bpf: Add bench for task storage creation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 29, 2023 at 11:03 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 3/27/23 8:51 PM, James Hilliard wrote:
> >> diff --git a/tools/testing/selftests/bpf/progs/bench_local_storage_create.c b/tools/testing/selftests/bpf/progs/bench_local_storage_create.c
> >> index 2814bab54d28..7c851c9d5e47 100644
> >> --- a/tools/testing/selftests/bpf/progs/bench_local_storage_create.c
> >> +++ b/tools/testing/selftests/bpf/progs/bench_local_storage_create.c
> >> @@ -22,6 +22,13 @@ struct {
> >>          __type(value, struct storage);
> >>   } sk_storage_map SEC(".maps");
> >>
> >> +struct {
> >> +       __uint(type, BPF_MAP_TYPE_TASK_STORAGE);
> >> +       __uint(map_flags, BPF_F_NO_PREALLOC);
> >> +       __type(key, int);
> >> +       __type(value, struct storage);
> >> +} task_storage_map SEC(".maps");
> >> +
> >>   SEC("raw_tp/kmalloc")
> >>   int BPF_PROG(kmalloc, unsigned long call_site, const void *ptr,
> >>               size_t bytes_req, size_t bytes_alloc, gfp_t gfp_flags,
> >> @@ -32,6 +39,24 @@ int BPF_PROG(kmalloc, unsigned long call_site, const void *ptr,
> >>          return 0;
> >>   }
> >>
> >> +SEC("tp_btf/sched_process_fork")
> >> +int BPF_PROG(fork, struct task_struct *parent, struct task_struct *child)
> >
> > Apparently fork is a built-in function in bpf-gcc:
>
> It is also failing in a plain C program
>
> #>  gcc -Werror=builtin-declaration-mismatch -o test test.c
> test.c:14:35: error: conflicting types for built-in function ‘fork’; expected
> ‘int(void)’ [-Werror=builtin-declaration-mismatch]
>     14 | int __attribute__((__noinline__)) fork(long x, long y)
>        |                                   ^~~~
> cc1: some warnings being treated as errors
>
> #> clang -o test test.c
> succeed
>
> I am not too attached to the name but it seems something should be addressed in
> the gcc instead.

Hmm, so it looks like it's marked as a builtin here:
https://github.com/gcc-mirror/gcc/blob/releases/gcc-12.1.0/gcc/builtins.def#L875

The macro for that is here:
https://github.com/gcc-mirror/gcc/blob/releases/gcc-12.1.0/gcc/builtins.def#L104-L111

Which has this comment:
/* Like DEF_LIB_BUILTIN, except that the function is not one that is
specified by ANSI/ISO C. So, when we're being fully conformant we
ignore the version of these builtins that does not begin with
__builtin. */

Looks like this builtin was originally added here:
https://github.com/gcc-mirror/gcc/commit/d1c38823924506d389ca58d02926ace21bdf82fa

Based on this issue it looks like fork is treated as a builtin for
libgcov support:
https://gcc.gnu.org/bugzilla//show_bug.cgi?id=82457

So from my understanding fork is a gcc builtin when building with -std=gnu11
but is not a builtin when building with -std=c11.

So it looks like fork is translated to __gcov_fork when -std=gnu* is set which
is why we get this error.

As this appears to be intended behavior for gcc I think the best option is
to just rename the function so that we don't run into issues when building
with gnu extensions like -std=gnu11.

>
> >
> > In file included from progs/bench_local_storage_create.c:6:
> > progs/bench_local_storage_create.c:43:14: error: conflicting types for
> > built-in function 'fork'; expected 'int(void)'
> > [-Werror=builtin-declaration-mismatch]
> >     43 | int BPF_PROG(fork, struct task_struct *parent, struct
> > task_struct *child)
> >        |              ^~~~
> >
> > I haven't been able to find this documented anywhere however.
>




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux