Re: [PATCH bpf-next v2 4/5] selftests/bpf: Fix kprobe_multi_bench_attach test failure with LTO kernel

[Yonghong Song <yonghong.song@linux.dev>]

On 3/22/24 2:53 PM, Andrii Nakryiko wrote:
> On Fri, Mar 22, 2024 at 9:01 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> On 3/22/24 5:37 AM, Jiri Olsa wrote:
>>> On Thu, Mar 21, 2024 at 01:01:19PM -0700, Yonghong Song wrote:
>>>> In my locally build clang LTO kernel (enabling CONFIG_LTO and
>>>> CONFIG_LTO_CLANG_THIN), kprobe_multi_bench_attach/kernel subtest
>>>> failed like:
>>>>     test_kprobe_multi_bench_attach:PASS:get_syms 0 nsec
>>>>     test_kprobe_multi_bench_attach:PASS:kprobe_multi_empty__open_and_load 0 nsec
>>>>     libbpf: prog 'test_kprobe_empty': failed to attach: No such process
>>>>     test_kprobe_multi_bench_attach:FAIL:bpf_program__attach_kprobe_multi_opts unexpected error: -3
>>>>     #117/1   kprobe_multi_bench_attach/kernel:FAIL
>>>>
>>>> There are multiple symbols in /sys/kernel/debug/tracing/available_filter_functions
>>>> are renamed in /proc/kallsyms due to cross file inlining. One example is for
>>>>     static function __access_remote_vm in mm/memory.c.
>>>> In a non-LTO kernel, we have the following call stack:
>>>>     ptrace_access_vm (global, kernel/ptrace.c)
>>>>       access_remote_vm (global, mm/memory.c)
>>>>         __access_remote_vm (static, mm/memory.c)
>>>>
>>>> With LTO kernel, it is possible that access_remote_vm() is inlined by
>>>> ptrace_access_vm(). So we end up with the following call stack:
>>>>     ptrace_access_vm (global, kernel/ptrace.c)
>>>>       __access_remote_vm (static, mm/memory.c)
>>>> The compiler renames __access_remote_vm to __access_remote_vm.llvm.<hash>
>>>> to prevent potential name collision.
>>>>
>>>> The kernel bpf_kprobe_multi_link_attach() and ftrace_lookup_symbols() try
>>>> to find addresses based on /proc/kallsyms, hence the current test failed
>>>> with LTO kenrel.
>>>>
>>>> This patch removed __access_remote_vm and other similar functions from
>>>> kprobe_multi_attach by checking if the symbol like __access_remote_vm
>>>> does not exist in kallsyms with LTO kernel. The test succeeded after this change:
>>>>     #117/1   kprobe_multi_bench_attach/kernel:OK
>>>>
>>>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>>>> ---
>>>>    .../selftests/bpf/prog_tests/kprobe_multi_test.c     | 12 ++++++++++++
>>>>    1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
>>>> index 05000810e28e..f6130f4f3d88 100644
>>>> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
>>>> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
>>>> @@ -345,6 +345,9 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
>>>>       FILE *f;
>>>>       int err = 0;
>>>>
>>>> +    if (!ASSERT_OK(load_kallsyms(), "load_kallsyms"))
>>>> +            return -EINVAL;
>>>> +
>>>>       /*
>>>>        * The available_filter_functions contains many duplicates,
>>>>        * but other than that all symbols are usable in kprobe multi
>>>> @@ -393,6 +396,15 @@ static int get_syms(char ***symsp, size_t *cntp, bool kernel)
>>>>               if (!strncmp(name, "__ftrace_invalid_address__",
>>>>                            sizeof("__ftrace_invalid_address__") - 1))
>>>>                       continue;
>>>> +            /*
>>>> +             * In certain cases, e.g., clang lto kernel, the 'name' here
>>>> +             * may be different from the one in /proc/kallsyms due to
>>>> +             * /proc/kallsyms name might be "<name>.llvm.<hash>" instead
>>>> +             * of "<name>". Exclude these 'name's since they will cause
>>>> +             * later kprobe_multi_attach failure.
>>>> +             */
>>>> +            if (ksym_get_addr(name) == 0)
>>>> +                    continue;
>>> curious how many symbols like that are there?
>> The number of entries in /sys/kernel/tracing/available_filter_functions: 50654
>> After existing filtering ('arch_cpu_idle') etc: 50513 (filtering 141)
>> After above ksym_get_addr(name) check: 49437 (further filtering 1076)
>>
> alternatively, you could have found matching func.llvm.* for any func
> in available_filter_functions. Have you considered that?

Looks like you prefer not skipping those functions who have .llvm.* in
/proc/kallsyms in this patch set. Yes, I can do that.

>
>>> Acked-by: Jiri Olsa <jolsa@kernel.org>
>>>
>>> jirka
>>>
>>>>               err = hashmap__add(map, name, 0);
>>>>               if (err == -EEXIST) {
>>>> --
>>>> 2.43.0
>>>>
>>>>