On Tue, 2022-08-30 at 18:46 +0200, Ilya Leoshkevich wrote: > On Tue, 2022-08-30 at 15:48 +0200, Jiri Olsa wrote: > > On Tue, Aug 30, 2022 at 12:25:25AM +0200, Daniel Borkmann wrote: > > > On 8/26/22 8:46 PM, Jiri Olsa wrote: > > > > hi, > > > > as discussed [1] sending fix that moves bpf dispatcher function > > > > of out > > > > ftrace locations together with Peter's > > > > HAVE_DYNAMIC_FTRACE_NO_PATCHABLE > > > > dependency change. > > > > > > Looks like the series breaks s390x builds; BPF CI link: > > > > > > https://github.com/kernel-patches/bpf/runs/8079411784?check_suite_focus=true > > > > > > [...] > > > CC net/xfrm/xfrm_state.o > > > CC net/packet/af_packet.o > > > {standard input}: Assembler messages: > > > {standard input}:16055: Error: bad expression > > > {standard input}:16056: Error: bad expression > > > {standard input}:16057: Error: bad expression > > > {standard input}:16058: Error: bad expression > > > {standard input}:16059: Error: bad expression > > > CC drivers/s390/char/raw3270.o > > > CC net/ipv6/ip6_output.o > > > [...] > > > CC net/xfrm/xfrm_output.o > > > CC net/ipv6/ip6_input.o > > > {standard input}:16055: Error: invalid operands (*ABS* and > > > *UND* > > > sections) for `%' > > > {standard input}:16056: Error: invalid operands (*ABS* and > > > *UND* > > > sections) for `%' > > > {standard input}:16057: Error: invalid operands (*ABS* and > > > *UND* > > > sections) for `%' > > > {standard input}:16058: Error: invalid operands (*ABS* and > > > *UND* > > > sections) for `%' > > > {standard input}:16059: Error: invalid operands (*ABS* and > > > *UND* > > > sections) for `%' > > > make[3]: *** [scripts/Makefile.build:249: net/core/filter.o] > > > Error 1 > > > make[2]: *** [scripts/Makefile.build:465: net/core] Error 2 > > > make[2]: *** Waiting for unfinished jobs.... > > > CC net/ipv4/tcp_fastopen.o > > > [...] > > > CC lib/percpu-refcount.o > > > make[1]: *** [Makefile:1855: net] Error 2 > > > CC lib/rhashtable.o > > > make[1]: *** Waiting for unfinished jobs.... > > > CC lib/base64.o > > > [...] > > > AR lib/built-in.a > > > CC kernel/kheaders.o > > > AR kernel/built-in.a > > > make: *** [Makefile:353: __build_one_by_one] Error 2 > > > Error: Process completed with exit code 2. > > > > > > it does not break on my cross build with gcc 12, but I can > > reproduce with gcc 8 (CI seems to be on gcc 9) > > > > the problem seems to be wrong assembler code with extra '%' > > that's generated for patchable_function_entry(5) > > > > gcc 8 generates: > > > > .LPFE1: > > nopr %%r0 > > nopr %%r0 > > nopr %%r0 > > nopr %%r0 > > nopr %%r0 > > > > and gcc 12 generates: > > > > .LPFE1: > > nopr %r0 > > nopr %r0 > > nopr %r0 > > nopr %r0 > > nopr %r0 > > > > perhaps we need to upgrade gcc in CI? cc-ing Ilya, any idea? > > > > thanks, > > jirka > > It's not obvious to me which gcc commit fixed this; I will bisect and > find out. This will take some time. > > However, officially, the kernel must be buildable by gcc 5.1+. > Whatever I find, it's unlikely that we'll be able to backport it > that far. > > Therefore I think we need to find a way to conditionally > do something else when using broken gccs. Or maybe just keep this > x86-only after all. > > Best regards, > Ilya FWIW, bisect points to https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=45d06a4045bebc3dbaaf0b1c676f4e22b7c6aca1 which makes perfect sense. Still, as I mentioned above, it's probably worth tolerating brokens gccs instead of spending time backporting this everywhere. And upgrading the CI machine will only paper over the issue. At a closer look, it looks weird to me that we have patchable_function_entry(5) in a common header. If this optimization is ever implemented for another architecture, a different number will be required. For simplicity, would it make sense to hide this under an #ifdef? Something like this (untested): #ifdef CONFIG_X86 #define BPF_DISPATCHER_ATTRIBUTES __attribute__((patchable_function_entry(5))) #else #define BPF_DISPATCHER_ATTRIBUTES #endif Best regards, Ilya
