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@xxxxxxxxx> wrote:
>>
>> On Wed, Apr 26, 2023 at 5:14 PM Namhyung Kim <namhyung@xxxxxxxxxx> 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
