* [HELP] failed to resolve CO-RE relocation
@ 2023-04-27 0:14 Namhyung Kim
2023-04-27 1:19 ` Andrii Nakryiko
0 siblings, 1 reply; 10+ messages in thread
From: Namhyung Kim @ 2023-04-27 0:14 UTC (permalink / raw)
To: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
Yonghong Song
Cc: bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo,
Ian Rogers
Hello,
I'm having a problem of loading perf lock contention BPF program [1]
on old kernels. It has collect_lock_syms() to get the address of each
CPU's run-queue lock. The kernel 5.14 changed the name of the field
so there's bpf_core_field_exists to check the name like below.
if (bpf_core_field_exists(rq_new->__lock))
lock_addr = (__u64)&rq_new->__lock;
else
lock_addr = (__u64)&rq_old->lock;
Note that I've applied a patch [2] to fix an issue with this code.
It works fine on my machine (with a newer kernel), but failed on the
old kernels. I guess it'd go to the else part without a problem but
it didn't for some reason.
Then I change the code to check the rq_old first. It works well on
the old kernels but fails on newer kernels.. :(
libbpf: prog 'collect_lock_syms': BPF program load failed: Invalid argument
libbpf: prog 'collect_lock_syms': -- BEGIN PROG LOAD LOG --
reg type unsupported for arg#0 function collect_lock_syms#380
0: R1=ctx(off=0,imm=0) R10=fp0
; int BPF_PROG(collect_lock_syms)
0: (b7) r6 = 0 ; R6_w=0
1: (b7) r7 = 0 ; R7_w=0
2: (b7) r9 = 1 ; R9_w=1
3: <invalid CO-RE relocation>
failed to resolve CO-RE relocation <byte_off> [381] struct rq___old.lock (0:0 @ offset 0)
processed 4 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
I'm curious what went wrong with this. I guess it's supposed to work
on any kernel verions by definition. Not sure the compiler generated
a wrong reloc or something. Maybe I just made silly mistakes..
Do you see anything wrong? Any hints to debug this issue?
Thanks,
Namhyung
[1] file://linux/tools/perf/util/bpf_skel/lock_contention.bpf.c
[2] https://lore.kernel.org/lkml/20230423215650.287812-1-namhyung@kernel.org/
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 0:14 [HELP] failed to resolve CO-RE relocation Namhyung Kim @ 2023-04-27 1:19 ` Andrii Nakryiko 2023-04-27 2:21 ` Namhyung Kim 0 siblings, 1 reply; 10+ messages in thread From: Andrii Nakryiko @ 2023-04-27 1:19 UTC (permalink / raw) To: Namhyung Kim Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers On Wed, Apr 26, 2023 at 5:14 PM Namhyung Kim <namhyung@kernel.org> wrote: > > Hello, > > I'm having a problem of loading perf lock contention BPF program [1] > on old kernels. It has collect_lock_syms() to get the address of each > CPU's run-queue lock. The kernel 5.14 changed the name of the field > so there's bpf_core_field_exists to check the name like below. > > if (bpf_core_field_exists(rq_new->__lock)) > lock_addr = (__u64)&rq_new->__lock; > else > lock_addr = (__u64)&rq_old->lock; I suspect compiler rewrites it to something like lock_addr = (__u64)&rq_old->lock; if (bpf_core_field_exists(rq_new->__lock)) lock_addr = (__u64)&rq_new->__lock; so rq_old relocation always happens and ends up being not guarded properly. You can try adding barrier_var(rq_new) and barrier_var(rq_old) around if and inside branches, that should pessimize compiler alternatively if you do if (bpf_core_field_exists(rq_new->__lock)) lock_addr = (__u64)&rq_new->__lock; else if (bpf_core_field_exists(rq_old->lock)) lock_addr = (__u64)&rq_old->lock; else lock_addr = 0; /* or signal error somehow */ It might work as well. > > Note that I've applied a patch [2] to fix an issue with this code. > > It works fine on my machine (with a newer kernel), but failed on the > old kernels. I guess it'd go to the else part without a problem but > it didn't for some reason. > > Then I change the code to check the rq_old first. It works well on > the old kernels but fails on newer kernels.. :( > > libbpf: prog 'collect_lock_syms': BPF program load failed: Invalid argument > libbpf: prog 'collect_lock_syms': -- BEGIN PROG LOAD LOG -- > reg type unsupported for arg#0 function collect_lock_syms#380 > 0: R1=ctx(off=0,imm=0) R10=fp0 > ; int BPF_PROG(collect_lock_syms) > 0: (b7) r6 = 0 ; R6_w=0 > 1: (b7) r7 = 0 ; R7_w=0 > 2: (b7) r9 = 1 ; R9_w=1 > 3: <invalid CO-RE relocation> > failed to resolve CO-RE relocation <byte_off> [381] struct rq___old.lock (0:0 @ offset 0) > processed 4 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > > I'm curious what went wrong with this. I guess it's supposed to work > on any kernel verions by definition. Not sure the compiler generated > a wrong reloc or something. Maybe I just made silly mistakes.. > > Do you see anything wrong? Any hints to debug this issue? > > Thanks, > Namhyung > > > [1] file://linux/tools/perf/util/bpf_skel/lock_contention.bpf.c > [2] https://lore.kernel.org/lkml/20230423215650.287812-1-namhyung@kernel.org/ ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 1:19 ` Andrii Nakryiko @ 2023-04-27 2:21 ` Namhyung Kim 2023-04-27 4:19 ` Hengqi Chen 2023-04-27 4:26 ` Andrii Nakryiko 0 siblings, 2 replies; 10+ messages in thread From: Namhyung Kim @ 2023-04-27 2:21 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers Hello Andrii, On Wed, Apr 26, 2023 at 6:19 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Apr 26, 2023 at 5:14 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > Hello, > > > > I'm having a problem of loading perf lock contention BPF program [1] > > on old kernels. It has collect_lock_syms() to get the address of each > > CPU's run-queue lock. The kernel 5.14 changed the name of the field > > so there's bpf_core_field_exists to check the name like below. > > > > if (bpf_core_field_exists(rq_new->__lock)) > > lock_addr = (__u64)&rq_new->__lock; > > else > > lock_addr = (__u64)&rq_old->lock; > > I suspect compiler rewrites it to something like > > lock_addr = (__u64)&rq_old->lock; > if (bpf_core_field_exists(rq_new->__lock)) > lock_addr = (__u64)&rq_new->__lock; > > so rq_old relocation always happens and ends up being not guarded > properly. You can try adding barrier_var(rq_new) and > barrier_var(rq_old) around if and inside branches, that should > pessimize compiler > > alternatively if you do > > if (bpf_core_field_exists(rq_new->__lock)) > lock_addr = (__u64)&rq_new->__lock; > else if (bpf_core_field_exists(rq_old->lock)) > lock_addr = (__u64)&rq_old->lock; > else > lock_addr = 0; /* or signal error somehow */ > > It might work as well. Thanks a lot for your comment! I've tried the below code but no luck. :( barrier_var(rq_old); barrier_var(rq_new); if (bpf_core_field_exists(rq_old->lock)) { barrier_var(rq_old); lock_addr = (__u64)&rq_old->lock; } else if (bpf_core_field_exists(rq_new->__lock)) { barrier_var(rq_new); lock_addr = (__u64)&rq_new->__lock; } else lock_addr = 0; ; int BPF_PROG(collect_lock_syms) 0: (b7) r8 = 0 ; R8_w=0 1: (b7) r7 = 1 ; R7_w=1 2: <invalid CO-RE relocation> failed to resolve CO-RE relocation <byte_off> [381] struct rq___old.lock (0:0 @ offset 0) processed 3 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read Thanks, Namhyung ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 2:21 ` Namhyung Kim @ 2023-04-27 4:19 ` Hengqi Chen 2023-04-27 4:27 ` Namhyung Kim 2023-04-27 4:26 ` Andrii Nakryiko 1 sibling, 1 reply; 10+ messages in thread From: Hengqi Chen @ 2023-04-27 4:19 UTC (permalink / raw) To: Namhyung Kim, Andrii Nakryiko Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers Hi, Namhyung On 2023/4/27 10:21, Namhyung Kim wrote: > Hello Andrii, > > On Wed, Apr 26, 2023 at 6:19 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: >> >> On Wed, Apr 26, 2023 at 5:14 PM Namhyung Kim <namhyung@kernel.org> wrote: >>> >>> Hello, >>> >>> I'm having a problem of loading perf lock contention BPF program [1] >>> on old kernels. It has collect_lock_syms() to get the address of each >>> CPU's run-queue lock. The kernel 5.14 changed the name of the field >>> so there's bpf_core_field_exists to check the name like below. >>> >>> if (bpf_core_field_exists(rq_new->__lock)) >>> lock_addr = (__u64)&rq_new->__lock; >>> else >>> lock_addr = (__u64)&rq_old->lock; >> >> I suspect compiler rewrites it to something like >> >> lock_addr = (__u64)&rq_old->lock; >> if (bpf_core_field_exists(rq_new->__lock)) >> lock_addr = (__u64)&rq_new->__lock; >> >> so rq_old relocation always happens and ends up being not guarded >> properly. You can try adding barrier_var(rq_new) and >> barrier_var(rq_old) around if and inside branches, that should >> pessimize compiler >> >> alternatively if you do >> >> if (bpf_core_field_exists(rq_new->__lock)) >> lock_addr = (__u64)&rq_new->__lock; >> else if (bpf_core_field_exists(rq_old->lock)) >> lock_addr = (__u64)&rq_old->lock; >> else >> lock_addr = 0; /* or signal error somehow */ >> >> It might work as well. > > Thanks a lot for your comment! > > I've tried the below code but no luck. :( > > barrier_var(rq_old); > barrier_var(rq_new); > > if (bpf_core_field_exists(rq_old->lock)) { > barrier_var(rq_old); > lock_addr = (__u64)&rq_old->lock; Have you tried `BPF_CORE_READ(rq_old, lock)` ? > } else if (bpf_core_field_exists(rq_new->__lock)) { > barrier_var(rq_new); > lock_addr = (__u64)&rq_new->__lock; > } else > lock_addr = 0; > > > ; int BPF_PROG(collect_lock_syms) > 0: (b7) r8 = 0 ; R8_w=0 > 1: (b7) r7 = 1 ; R7_w=1 > 2: <invalid CO-RE relocation> > failed to resolve CO-RE relocation <byte_off> [381] struct > rq___old.lock (0:0 @ offset 0) > processed 3 insns (limit 1000000) max_states_per_insn 0 total_states 0 > peak_states 0 mark_read > > Thanks, > Namhyung Cheers, Hengqi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 4:19 ` Hengqi Chen @ 2023-04-27 4:27 ` Namhyung Kim 0 siblings, 0 replies; 10+ messages in thread From: Namhyung Kim @ 2023-04-27 4:27 UTC (permalink / raw) To: Hengqi Chen Cc: Andrii Nakryiko, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers Hi Hengqi, On Wed, Apr 26, 2023 at 9:20 PM Hengqi Chen <hengqi.chen@gmail.com> wrote: > > Hi, Namhyung > > On 2023/4/27 10:21, Namhyung Kim wrote: > > Hello Andrii, > > > > On Wed, Apr 26, 2023 at 6:19 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > >> > >> On Wed, Apr 26, 2023 at 5:14 PM Namhyung Kim <namhyung@kernel.org> wrote: > >>> > >>> Hello, > >>> > >>> I'm having a problem of loading perf lock contention BPF program [1] > >>> on old kernels. It has collect_lock_syms() to get the address of each > >>> CPU's run-queue lock. The kernel 5.14 changed the name of the field > >>> so there's bpf_core_field_exists to check the name like below. > >>> > >>> if (bpf_core_field_exists(rq_new->__lock)) > >>> lock_addr = (__u64)&rq_new->__lock; > >>> else > >>> lock_addr = (__u64)&rq_old->lock; > >> > >> I suspect compiler rewrites it to something like > >> > >> lock_addr = (__u64)&rq_old->lock; > >> if (bpf_core_field_exists(rq_new->__lock)) > >> lock_addr = (__u64)&rq_new->__lock; > >> > >> so rq_old relocation always happens and ends up being not guarded > >> properly. You can try adding barrier_var(rq_new) and > >> barrier_var(rq_old) around if and inside branches, that should > >> pessimize compiler > >> > >> alternatively if you do > >> > >> if (bpf_core_field_exists(rq_new->__lock)) > >> lock_addr = (__u64)&rq_new->__lock; > >> else if (bpf_core_field_exists(rq_old->lock)) > >> lock_addr = (__u64)&rq_old->lock; > >> else > >> lock_addr = 0; /* or signal error somehow */ > >> > >> It might work as well. > > > > Thanks a lot for your comment! > > > > I've tried the below code but no luck. :( > > > > barrier_var(rq_old); > > barrier_var(rq_new); > > > > if (bpf_core_field_exists(rq_old->lock)) { > > barrier_var(rq_old); > > lock_addr = (__u64)&rq_old->lock; > > Have you tried `BPF_CORE_READ(rq_old, lock)` ? No, but I think it'd dereference the lock, right? I just want to get the address of the lock field. Thanks, Namhyung > > > } else if (bpf_core_field_exists(rq_new->__lock)) { > > barrier_var(rq_new); > > lock_addr = (__u64)&rq_new->__lock; > > } else > > lock_addr = 0; > > > > > > ; int BPF_PROG(collect_lock_syms) > > 0: (b7) r8 = 0 ; R8_w=0 > > 1: (b7) r7 = 1 ; R7_w=1 > > 2: <invalid CO-RE relocation> > > failed to resolve CO-RE relocation <byte_off> [381] struct > > rq___old.lock (0:0 @ offset 0) > > processed 3 insns (limit 1000000) max_states_per_insn 0 total_states 0 > > peak_states 0 mark_read > > > > Thanks, > > Namhyung > > Cheers, > Hengqi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 2:21 ` Namhyung Kim 2023-04-27 4:19 ` Hengqi Chen @ 2023-04-27 4:26 ` Andrii Nakryiko 2023-04-27 4:53 ` Namhyung Kim 1 sibling, 1 reply; 10+ messages in thread From: Andrii Nakryiko @ 2023-04-27 4:26 UTC (permalink / raw) To: Namhyung Kim Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers On Wed, Apr 26, 2023 at 7:21 PM Namhyung Kim <namhyung@kernel.org> wrote: > > Hello Andrii, > > On Wed, Apr 26, 2023 at 6:19 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Wed, Apr 26, 2023 at 5:14 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > Hello, > > > > > > I'm having a problem of loading perf lock contention BPF program [1] > > > on old kernels. It has collect_lock_syms() to get the address of each > > > CPU's run-queue lock. The kernel 5.14 changed the name of the field > > > so there's bpf_core_field_exists to check the name like below. > > > > > > if (bpf_core_field_exists(rq_new->__lock)) > > > lock_addr = (__u64)&rq_new->__lock; > > > else > > > lock_addr = (__u64)&rq_old->lock; > > > > I suspect compiler rewrites it to something like > > > > lock_addr = (__u64)&rq_old->lock; > > if (bpf_core_field_exists(rq_new->__lock)) > > lock_addr = (__u64)&rq_new->__lock; > > > > so rq_old relocation always happens and ends up being not guarded > > properly. You can try adding barrier_var(rq_new) and > > barrier_var(rq_old) around if and inside branches, that should > > pessimize compiler > > > > alternatively if you do > > > > if (bpf_core_field_exists(rq_new->__lock)) > > lock_addr = (__u64)&rq_new->__lock; > > else if (bpf_core_field_exists(rq_old->lock)) > > lock_addr = (__u64)&rq_old->lock; > > else > > lock_addr = 0; /* or signal error somehow */ > > > > It might work as well. > > Thanks a lot for your comment! > > I've tried the below code but no luck. :( Can you post an output of llvm-objdump -d <your.bpf.o> of the program (collect_lock_syms?) containing above code (or at least relevant portions with some buffer before/after to get a sense of what's going on) > > barrier_var(rq_old); > barrier_var(rq_new); > > if (bpf_core_field_exists(rq_old->lock)) { > barrier_var(rq_old); > lock_addr = (__u64)&rq_old->lock; > } else if (bpf_core_field_exists(rq_new->__lock)) { > barrier_var(rq_new); > lock_addr = (__u64)&rq_new->__lock; > } else > lock_addr = 0; > > > ; int BPF_PROG(collect_lock_syms) > 0: (b7) r8 = 0 ; R8_w=0 > 1: (b7) r7 = 1 ; R7_w=1 > 2: <invalid CO-RE relocation> > failed to resolve CO-RE relocation <byte_off> [381] struct > rq___old.lock (0:0 @ offset 0) > processed 3 insns (limit 1000000) max_states_per_insn 0 total_states 0 > peak_states 0 mark_read > > Thanks, > Namhyung ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 4:26 ` Andrii Nakryiko @ 2023-04-27 4:53 ` Namhyung Kim 2023-04-27 22:15 ` Andrii Nakryiko 0 siblings, 1 reply; 10+ messages in thread From: Namhyung Kim @ 2023-04-27 4:53 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers On Wed, Apr 26, 2023 at 09:26:59PM -0700, Andrii Nakryiko wrote: > On Wed, Apr 26, 2023 at 7:21 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > Hello Andrii, > > > > On Wed, Apr 26, 2023 at 6:19 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Wed, Apr 26, 2023 at 5:14 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > Hello, > > > > > > > > I'm having a problem of loading perf lock contention BPF program [1] > > > > on old kernels. It has collect_lock_syms() to get the address of each > > > > CPU's run-queue lock. The kernel 5.14 changed the name of the field > > > > so there's bpf_core_field_exists to check the name like below. > > > > > > > > if (bpf_core_field_exists(rq_new->__lock)) > > > > lock_addr = (__u64)&rq_new->__lock; > > > > else > > > > lock_addr = (__u64)&rq_old->lock; > > > > > > I suspect compiler rewrites it to something like > > > > > > lock_addr = (__u64)&rq_old->lock; > > > if (bpf_core_field_exists(rq_new->__lock)) > > > lock_addr = (__u64)&rq_new->__lock; > > > > > > so rq_old relocation always happens and ends up being not guarded > > > properly. You can try adding barrier_var(rq_new) and > > > barrier_var(rq_old) around if and inside branches, that should > > > pessimize compiler > > > > > > alternatively if you do > > > > > > if (bpf_core_field_exists(rq_new->__lock)) > > > lock_addr = (__u64)&rq_new->__lock; > > > else if (bpf_core_field_exists(rq_old->lock)) > > > lock_addr = (__u64)&rq_old->lock; > > > else > > > lock_addr = 0; /* or signal error somehow */ > > > > > > It might work as well. > > > > Thanks a lot for your comment! > > > > I've tried the below code but no luck. :( > > Can you post an output of llvm-objdump -d <your.bpf.o> of the program > (collect_lock_syms?) containing above code (or at least relevant > portions with some buffer before/after to get a sense of what's going > on) Sure. Here's the full source code: SEC("raw_tp/bpf_test_finish") int BPF_PROG(collect_lock_syms) { __u64 lock_addr; __u32 lock_flag; for (int i = 0; i < MAX_CPUS; i++) { struct rq *rq = bpf_per_cpu_ptr(&runqueues, i); struct rq___new *rq_new = (void *)rq; struct rq___old *rq_old = (void *)rq; if (rq == NULL) break; barrier_var(rq_old); barrier_var(rq_new); if (bpf_core_field_exists(rq_old->lock)) { barrier_var(rq_old); lock_addr = (__u64)&rq_old->lock; } else if (bpf_core_field_exists(rq_new->__lock)) { barrier_var(rq_new); lock_addr = (__u64)&rq_new->__lock; } else continue; lock_flag = LOCK_CLASS_RQLOCK; bpf_map_update_elem(&lock_syms, &lock_addr, &lock_flag, BPF_ANY); } return 0; } And the disassembly is: $ llvm-objdump -d util/bpf_skel/.tmp/lock_contention.bpf.o ... Disassembly of section raw_tp/bpf_test_finish: 0000000000000000 <collect_lock_syms>: 0: b7 06 00 00 00 00 00 00 r6 = 0 1: b7 07 00 00 01 00 00 00 r7 = 1 2: b7 09 00 00 01 00 00 00 r9 = 1 3: b7 08 00 00 00 00 00 00 r8 = 0 0000000000000020 <LBB3_1>: 4: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll 6: bf 62 00 00 00 00 00 00 r2 = r6 7: 85 00 00 00 99 00 00 00 call 153 8: 15 00 12 00 00 00 00 00 if r0 == 0 goto +18 <LBB3_8> 9: bf 01 00 00 00 00 00 00 r1 = r0 10: 15 07 12 00 00 00 00 00 if r7 == 0 goto +18 <LBB3_4> 11: 0f 81 00 00 00 00 00 00 r1 += r8 0000000000000060 <LBB3_6>: 12: 63 9a f4 ff 00 00 00 00 *(u32 *)(r10 - 12) = r9 13: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1 14: bf a2 00 00 00 00 00 00 r2 = r10 15: 07 02 00 00 f8 ff ff ff r2 += -8 16: bf a3 00 00 00 00 00 00 r3 = r10 17: 07 03 00 00 f4 ff ff ff r3 += -12 18: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll 20: b7 04 00 00 00 00 00 00 r4 = 0 21: 85 00 00 00 02 00 00 00 call 2 00000000000000b0 <LBB3_7>: 22: 07 06 00 00 01 00 00 00 r6 += 1 23: bf 61 00 00 00 00 00 00 r1 = r6 24: 67 01 00 00 20 00 00 00 r1 <<= 32 25: 77 01 00 00 20 00 00 00 r1 >>= 32 26: 55 01 e9 ff 00 04 00 00 if r1 != 1024 goto -23 <LBB3_1> 00000000000000d8 <LBB3_8>: 27: b7 00 00 00 00 00 00 00 r0 = 0 28: 95 00 00 00 00 00 00 00 exit 00000000000000e8 <LBB3_4>: 29: b7 01 00 00 01 00 00 00 r1 = 1 30: 15 01 f7 ff 00 00 00 00 if r1 == 0 goto -9 <LBB3_7> 31: b7 01 00 00 00 00 00 00 r1 = 0 32: 0f 10 00 00 00 00 00 00 r0 += r1 33: bf 01 00 00 00 00 00 00 r1 = r0 34: 05 00 e9 ff 00 00 00 00 goto -23 <LBB3_6> The error message is like this: libbpf: prog 'collect_lock_syms': BPF program load failed: Invalid argument libbpf: prog 'collect_lock_syms': -- BEGIN PROG LOAD LOG -- reg type unsupported for arg#0 function collect_lock_syms#380 0: R1=ctx(off=0,imm=0) R10=fp0 ; int BPF_PROG(collect_lock_syms) 0: (b7) r6 = 0 ; R6_w=0 1: (b7) r7 = 0 ; R7_w=0 2: (b7) r9 = 1 ; R9_w=1 3: <invalid CO-RE relocation> failed to resolve CO-RE relocation <byte_off> [381] struct rq___old.lock (0:0 @ offset 0) processed 4 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 -- END PROG LOAD LOG -- libbpf: prog 'collect_lock_syms': failed to load: -22 libbpf: failed to load object 'lock_contention_bpf' libbpf: failed to load BPF skeleton 'lock_contention_bpf': -22 Failed to load lock-contention BPF skeleton lock contention BPF setup failed Thanks, Namhyung ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 4:53 ` Namhyung Kim @ 2023-04-27 22:15 ` Andrii Nakryiko 2023-04-27 23:27 ` Namhyung Kim 0 siblings, 1 reply; 10+ messages in thread From: Andrii Nakryiko @ 2023-04-27 22:15 UTC (permalink / raw) To: Namhyung Kim Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers On Wed, Apr 26, 2023 at 9:53 PM Namhyung Kim <namhyung@gmail.com> wrote: > > On Wed, Apr 26, 2023 at 09:26:59PM -0700, Andrii Nakryiko wrote: > > On Wed, Apr 26, 2023 at 7:21 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > Hello Andrii, > > > > > > On Wed, Apr 26, 2023 at 6:19 PM Andrii Nakryiko > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > On Wed, Apr 26, 2023 at 5:14 PM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > > > Hello, > > > > > > > > > > I'm having a problem of loading perf lock contention BPF program [1] > > > > > on old kernels. It has collect_lock_syms() to get the address of each > > > > > CPU's run-queue lock. The kernel 5.14 changed the name of the field > > > > > so there's bpf_core_field_exists to check the name like below. > > > > > > > > > > if (bpf_core_field_exists(rq_new->__lock)) > > > > > lock_addr = (__u64)&rq_new->__lock; > > > > > else > > > > > lock_addr = (__u64)&rq_old->lock; > > > > > > > > I suspect compiler rewrites it to something like > > > > > > > > lock_addr = (__u64)&rq_old->lock; > > > > if (bpf_core_field_exists(rq_new->__lock)) > > > > lock_addr = (__u64)&rq_new->__lock; > > > > > > > > so rq_old relocation always happens and ends up being not guarded > > > > properly. You can try adding barrier_var(rq_new) and > > > > barrier_var(rq_old) around if and inside branches, that should > > > > pessimize compiler > > > > > > > > alternatively if you do > > > > > > > > if (bpf_core_field_exists(rq_new->__lock)) > > > > lock_addr = (__u64)&rq_new->__lock; > > > > else if (bpf_core_field_exists(rq_old->lock)) > > > > lock_addr = (__u64)&rq_old->lock; > > > > else > > > > lock_addr = 0; /* or signal error somehow */ > > > > > > > > It might work as well. > > > > > > Thanks a lot for your comment! > > > > > > I've tried the below code but no luck. :( > > > > Can you post an output of llvm-objdump -d <your.bpf.o> of the program > > (collect_lock_syms?) containing above code (or at least relevant > > portions with some buffer before/after to get a sense of what's going > > on) > > Sure. > > Here's the full source code: > > SEC("raw_tp/bpf_test_finish") > int BPF_PROG(collect_lock_syms) > { > __u64 lock_addr; > __u32 lock_flag; > > for (int i = 0; i < MAX_CPUS; i++) { > struct rq *rq = bpf_per_cpu_ptr(&runqueues, i); > struct rq___new *rq_new = (void *)rq; > struct rq___old *rq_old = (void *)rq; > > if (rq == NULL) > break; > > barrier_var(rq_old); > barrier_var(rq_new); > > if (bpf_core_field_exists(rq_old->lock)) { > barrier_var(rq_old); > lock_addr = (__u64)&rq_old->lock; > } else if (bpf_core_field_exists(rq_new->__lock)) { > barrier_var(rq_new); > lock_addr = (__u64)&rq_new->__lock; > } else > continue; > > lock_flag = LOCK_CLASS_RQLOCK; > bpf_map_update_elem(&lock_syms, &lock_addr, &lock_flag, BPF_ANY); > } > return 0; > } > > And the disassembly is: > > $ llvm-objdump -d util/bpf_skel/.tmp/lock_contention.bpf.o > ... > Disassembly of section raw_tp/bpf_test_finish: > > 0000000000000000 <collect_lock_syms>: > 0: b7 06 00 00 00 00 00 00 r6 = 0 > 1: b7 07 00 00 01 00 00 00 r7 = 1 > 2: b7 09 00 00 01 00 00 00 r9 = 1 > 3: b7 08 00 00 00 00 00 00 r8 = 0 > > 0000000000000020 <LBB3_1>: > 4: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll > 6: bf 62 00 00 00 00 00 00 r2 = r6 > 7: 85 00 00 00 99 00 00 00 call 153 > 8: 15 00 12 00 00 00 00 00 if r0 == 0 goto +18 <LBB3_8> > 9: bf 01 00 00 00 00 00 00 r1 = r0 > 10: 15 07 12 00 00 00 00 00 if r7 == 0 goto +18 <LBB3_4> > 11: 0f 81 00 00 00 00 00 00 r1 += r8 > > 0000000000000060 <LBB3_6>: > 12: 63 9a f4 ff 00 00 00 00 *(u32 *)(r10 - 12) = r9 > 13: 7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1 > 14: bf a2 00 00 00 00 00 00 r2 = r10 > 15: 07 02 00 00 f8 ff ff ff r2 += -8 > 16: bf a3 00 00 00 00 00 00 r3 = r10 > 17: 07 03 00 00 f4 ff ff ff r3 += -12 > 18: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll > 20: b7 04 00 00 00 00 00 00 r4 = 0 > 21: 85 00 00 00 02 00 00 00 call 2 > > 00000000000000b0 <LBB3_7>: > 22: 07 06 00 00 01 00 00 00 r6 += 1 > 23: bf 61 00 00 00 00 00 00 r1 = r6 > 24: 67 01 00 00 20 00 00 00 r1 <<= 32 > 25: 77 01 00 00 20 00 00 00 r1 >>= 32 > 26: 55 01 e9 ff 00 04 00 00 if r1 != 1024 goto -23 <LBB3_1> > > 00000000000000d8 <LBB3_8>: > 27: b7 00 00 00 00 00 00 00 r0 = 0 > 28: 95 00 00 00 00 00 00 00 exit > > 00000000000000e8 <LBB3_4>: > 29: b7 01 00 00 01 00 00 00 r1 = 1 > 30: 15 01 f7 ff 00 00 00 00 if r1 == 0 goto -9 <LBB3_7> > 31: b7 01 00 00 00 00 00 00 r1 = 0 > 32: 0f 10 00 00 00 00 00 00 r0 += r1 > 33: bf 01 00 00 00 00 00 00 r1 = r0 > 34: 05 00 e9 ff 00 00 00 00 goto -23 <LBB3_6> > > The error message is like this: > > libbpf: prog 'collect_lock_syms': BPF program load failed: Invalid argument > libbpf: prog 'collect_lock_syms': -- BEGIN PROG LOAD LOG -- > reg type unsupported for arg#0 function collect_lock_syms#380 > 0: R1=ctx(off=0,imm=0) R10=fp0 > ; int BPF_PROG(collect_lock_syms) > 0: (b7) r6 = 0 ; R6_w=0 > 1: (b7) r7 = 0 ; R7_w=0 > 2: (b7) r9 = 1 ; R9_w=1 > 3: <invalid CO-RE relocation> > failed to resolve CO-RE relocation <byte_off> [381] struct rq___old.lock (0:0 @ offset 0) > processed 4 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 > -- END PROG LOAD LOG -- > libbpf: prog 'collect_lock_syms': failed to load: -22 > libbpf: failed to load object 'lock_contention_bpf' > libbpf: failed to load BPF skeleton 'lock_contention_bpf': -22 > Failed to load lock-contention BPF skeleton > lock contention BPF setup failed > > Ok, I didn't manage to force compiler to behave as long as `&rq_old->lock` pattern was used. So I went for a different approach. This works: $ git diff diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c index 8911e2a077d8..8d3cfbb3cc65 100644 --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c @@ -418,32 +418,32 @@ int contention_end(u64 *ctx) extern struct rq runqueues __ksym; -struct rq__old { +struct rq___old { raw_spinlock_t lock; } __attribute__((preserve_access_index)); -struct rq__new { +struct rq___new { raw_spinlock_t __lock; } __attribute__((preserve_access_index)); SEC("raw_tp/bpf_test_finish") int BPF_PROG(collect_lock_syms) { - __u64 lock_addr; + __u64 lock_addr, lock_off; __u32 lock_flag; + if (bpf_core_field_exists(struct rq___new, __lock)) + lock_off = offsetof(struct rq___new, __lock); + else + lock_off = offsetof(struct rq___old, lock); + for (int i = 0; i < MAX_CPUS; i++) { struct rq *rq = bpf_per_cpu_ptr(&runqueues, i); - struct rq__new *rq_new = (void *)rq; - struct rq__old *rq_old = (void *)rq; if (rq == NULL) break; - if (bpf_core_field_exists(rq_new->__lock)) - lock_addr = (__u64)&rq_new->__lock; - else - lock_addr = (__u64)&rq_old->lock; + lock_addr = (__u64)(void *)rq + lock_off; lock_flag = LOCK_CLASS_RQLOCK; bpf_map_update_elem(&lock_syms, &lock_addr, &lock_flag, BPF_ANY); } > Thanks, > Namhyung > > ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 22:15 ` Andrii Nakryiko @ 2023-04-27 23:27 ` Namhyung Kim 2023-04-27 23:38 ` Andrii Nakryiko 0 siblings, 1 reply; 10+ messages in thread From: Namhyung Kim @ 2023-04-27 23:27 UTC (permalink / raw) To: Andrii Nakryiko Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers On Thu, Apr 27, 2023 at 3:15 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > Ok, I didn't manage to force compiler to behave as long as > `&rq_old->lock` pattern was used. So I went for a different approach. > This works: Thanks! It works for me too! Can I use this patch with your Co-developed-by tag ? Thanks, Namhyung > > $ git diff > diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c > b/tools/perf/util/bpf_skel/lock_contention.bpf.c > index 8911e2a077d8..8d3cfbb3cc65 100644 > --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c > +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c > @@ -418,32 +418,32 @@ int contention_end(u64 *ctx) > > extern struct rq runqueues __ksym; > > -struct rq__old { > +struct rq___old { > raw_spinlock_t lock; > } __attribute__((preserve_access_index)); > > -struct rq__new { > +struct rq___new { > raw_spinlock_t __lock; > } __attribute__((preserve_access_index)); > > SEC("raw_tp/bpf_test_finish") > int BPF_PROG(collect_lock_syms) > { > - __u64 lock_addr; > + __u64 lock_addr, lock_off; > __u32 lock_flag; > > + if (bpf_core_field_exists(struct rq___new, __lock)) > + lock_off = offsetof(struct rq___new, __lock); > + else > + lock_off = offsetof(struct rq___old, lock); > + > for (int i = 0; i < MAX_CPUS; i++) { > struct rq *rq = bpf_per_cpu_ptr(&runqueues, i); > - struct rq__new *rq_new = (void *)rq; > - struct rq__old *rq_old = (void *)rq; > > if (rq == NULL) > break; > > - if (bpf_core_field_exists(rq_new->__lock)) > - lock_addr = (__u64)&rq_new->__lock; > - else > - lock_addr = (__u64)&rq_old->lock; > + lock_addr = (__u64)(void *)rq + lock_off; > lock_flag = LOCK_CLASS_RQLOCK; > bpf_map_update_elem(&lock_syms, &lock_addr, > &lock_flag, BPF_ANY); > } > > > > Thanks, > > Namhyung > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [HELP] failed to resolve CO-RE relocation 2023-04-27 23:27 ` Namhyung Kim @ 2023-04-27 23:38 ` Andrii Nakryiko 0 siblings, 0 replies; 10+ messages in thread From: Andrii Nakryiko @ 2023-04-27 23:38 UTC (permalink / raw) To: Namhyung Kim Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, Yonghong Song, bpf, Song Liu, Jiri Olsa, Arnaldo Carvalho de Melo, Hao Luo, Ian Rogers On Thu, Apr 27, 2023 at 4:28 PM Namhyung Kim <namhyung@gmail.com> wrote: > > On Thu, Apr 27, 2023 at 3:15 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > Ok, I didn't manage to force compiler to behave as long as > > `&rq_old->lock` pattern was used. So I went for a different approach. > > This works: > > Thanks! It works for me too! > > Can I use this patch with your Co-developed-by tag ? of course > > Thanks, > Namhyung > > > > > > $ git diff > > diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c > > b/tools/perf/util/bpf_skel/lock_contention.bpf.c > > index 8911e2a077d8..8d3cfbb3cc65 100644 > > --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c > > +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c > > @@ -418,32 +418,32 @@ int contention_end(u64 *ctx) > > > > extern struct rq runqueues __ksym; > > > > -struct rq__old { > > +struct rq___old { > > raw_spinlock_t lock; > > } __attribute__((preserve_access_index)); > > > > -struct rq__new { > > +struct rq___new { > > raw_spinlock_t __lock; > > } __attribute__((preserve_access_index)); > > > > SEC("raw_tp/bpf_test_finish") > > int BPF_PROG(collect_lock_syms) > > { > > - __u64 lock_addr; > > + __u64 lock_addr, lock_off; > > __u32 lock_flag; > > > > + if (bpf_core_field_exists(struct rq___new, __lock)) > > + lock_off = offsetof(struct rq___new, __lock); > > + else > > + lock_off = offsetof(struct rq___old, lock); > > + > > for (int i = 0; i < MAX_CPUS; i++) { > > struct rq *rq = bpf_per_cpu_ptr(&runqueues, i); > > - struct rq__new *rq_new = (void *)rq; > > - struct rq__old *rq_old = (void *)rq; > > > > if (rq == NULL) > > break; > > > > - if (bpf_core_field_exists(rq_new->__lock)) > > - lock_addr = (__u64)&rq_new->__lock; > > - else > > - lock_addr = (__u64)&rq_old->lock; > > + lock_addr = (__u64)(void *)rq + lock_off; > > lock_flag = LOCK_CLASS_RQLOCK; > > bpf_map_update_elem(&lock_syms, &lock_addr, > > &lock_flag, BPF_ANY); > > } > > > > > > > Thanks, > > > Namhyung > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-04-27 23:39 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-27 0:14 [HELP] failed to resolve CO-RE relocation Namhyung Kim 2023-04-27 1:19 ` Andrii Nakryiko 2023-04-27 2:21 ` Namhyung Kim 2023-04-27 4:19 ` Hengqi Chen 2023-04-27 4:27 ` Namhyung Kim 2023-04-27 4:26 ` Andrii Nakryiko 2023-04-27 4:53 ` Namhyung Kim 2023-04-27 22:15 ` Andrii Nakryiko 2023-04-27 23:27 ` Namhyung Kim 2023-04-27 23:38 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox