| Message ID | 20240321200119.2220027-1-yonghong.song@linux.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Fix a couple of test failures with LTO kernel | expand |
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? Acked-by: Jiri Olsa <jolsa@kernel.org> jirka > > err = hashmap__add(map, name, 0); > if (err == -EEXIST) { > -- > 2.43.0 > >
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) > > Acked-by: Jiri Olsa <jolsa@kernel.org> > > jirka > >> >> err = hashmap__add(map, name, 0); >> if (err == -EEXIST) { >> -- >> 2.43.0 >> >>
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? > > > > Acked-by: Jiri Olsa <jolsa@kernel.org> > > > > jirka > > > >> > >> err = hashmap__add(map, name, 0); > >> if (err == -EEXIST) { > >> -- > >> 2.43.0 > >> > >>
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 >>>> >>>>
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; err = hashmap__add(map, name, 0); if (err == -EEXIST) {
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(+)