On Tue, Sep 8, 2020 at 8:13 PM Yonghong Song <yhs@xxxxxx> wrote: > > Andrii reported that with latest clang, when building selftests, we have > error likes: > error: progs/test_sysctl_loop1.c:23:16: in function sysctl_tcp_mem i32 (%struct.bpf_sysctl*): > Looks like the BPF stack limit of 512 bytes is exceeded. > Please move large on stack variables into BPF per-cpu array map. > > The error is triggered by the following LLVM patch: > https://reviews.llvm.org/D87134 > > For example, the following code is from test_sysctl_loop1.c: > static __always_inline int is_tcp_mem(struct bpf_sysctl *ctx) > { > volatile char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string"; > ... > } > Without the above LLVM patch, the compiler did optimization to load the string > (59 bytes long) with 7 64bit loads, 1 8bit load and 1 16bit load, > occupying 64 byte stack size. > > With the above LLVM patch, the compiler only uses 8bit loads, but subregister is 32bit. > So stack requirements become 4 * 59 = 236 bytes. Together with other stuff on > the stack, total stack size exceeds 512 bytes, hence compiler complains and quits. > > To fix the issue, removing "volatile" key word or changing "volatile" to > "const"/"static const" does not work, the string is put in .rodata.str1.1 section, > which libbpf did not process it and errors out with Yeah, those .str1.1 sections are becoming more prominent, I think I'll be able to fix it soon, just need the right BTF APIs implemented first. > libbpf: elf: skipping unrecognized data section(6) .rodata.str1.1 > libbpf: prog 'sysctl_tcp_mem': bad map relo against '.L__const.is_tcp_mem.tcp_mem_name' > in section '.rodata.str1.1' > > Defining the string const as global variable can fix the issue as it puts the string constant > in '.rodata' section which is recognized by libbpf. In the future, when libbpf can process > '.rodata.str*.*' properly, the global definition can be changed back to local definition. > > Reported-by: Andrii Nakryiko <andriin@xxxxxx> > Signed-off-by: Yonghong Song <yhs@xxxxxx> > --- thanks! Acked-by: Andrii Nakryiko <andriin@xxxxxx> > tools/testing/selftests/bpf/progs/test_sysctl_loop1.c | 2 +- > tools/testing/selftests/bpf/progs/test_sysctl_loop2.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c b/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c > index 458b0d69133e..4b600b1f522f 100644 > --- a/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c > +++ b/tools/testing/selftests/bpf/progs/test_sysctl_loop1.c > @@ -18,9 +18,9 @@ > #define MAX_ULONG_STR_LEN 7 > #define MAX_VALUE_STR_LEN (TCP_MEM_LOOPS * MAX_ULONG_STR_LEN) > > +const char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string"; Do you think we should put "volatile" there just in case Clang decides to be very optimizing and smart? > static __always_inline int is_tcp_mem(struct bpf_sysctl *ctx) > { > - volatile char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string"; > unsigned char i; > char name[64]; > int ret; > diff --git a/tools/testing/selftests/bpf/progs/test_sysctl_loop2.c b/tools/testing/selftests/bpf/progs/test_sysctl_loop2.c > index b2e6f9b0894d..3c292c087395 100644 > --- a/tools/testing/selftests/bpf/progs/test_sysctl_loop2.c > +++ b/tools/testing/selftests/bpf/progs/test_sysctl_loop2.c > @@ -18,9 +18,9 @@ > #define MAX_ULONG_STR_LEN 7 > #define MAX_VALUE_STR_LEN (TCP_MEM_LOOPS * MAX_ULONG_STR_LEN) > > +const char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string_to_stress_byte_loop"; > static __attribute__((noinline)) int is_tcp_mem(struct bpf_sysctl *ctx) > { > - volatile char tcp_mem_name[] = "net/ipv4/tcp_mem/very_very_very_very_long_pointless_string_to_stress_byte_loop"; > unsigned char i; > char name[64]; > int ret; > -- > 2.24.1 >
