Message ID | 20201125035255.17970-1-andreimatei1@gmail.com (mailing list archive) |
---|---|
State | Accepted |
Delegated to: | BPF |
Headers | show |
Series | [bpf-next] selftest/bpf: fix compilation on clang 11 | expand |
Context | Check | Description |
---|---|---|
netdev/cover_letter | success | Link |
netdev/fixes_present | success | Link |
netdev/patch_count | success | Link |
netdev/tree_selection | success | Clearly marked for bpf-next |
netdev/subject_prefix | success | Link |
netdev/source_inline | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Link |
netdev/module_param | success | Was 0 now: 0 |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/verify_fixes | success | Link |
netdev/checkpatch | warning | CHECK: Please don't use multiple blank lines |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/header_inline | success | Link |
netdev/stable | success | Stable not CCed |
On 11/24/20 7:52 PM, Andrei Matei wrote: > Before this patch, profiler.inc.h wouldn't compile with clang-11 (before > the __builtin_preserve_enum_value LLVM builtin was introduced in > https://reviews.llvm.org/D83242 ). > Another test that uses this builtin (test_core_enumval) is conditionally > skipped if the compiler is too old. In that spirit, this patch inhibits > part of populate_cgroup_info(), which needs this CO-RE builtin. The > selftests build again on clang-11. The affected test (the profiler > test) doesn't pass on clang-11 because it's missing > https://reviews.llvm.org/D85570 , but at least the test suite as a whole > compiles. The test's expected failure is already called out in the > README. > > Signed-off-by: Andrei Matei <andreimatei1@gmail.com> Thanks for the fix! This change won't impact correctness as profiler.c mostly to test verifier. Acked-by: Yonghong Song <yhs@fb.com>
On Tue, Nov 24, 2020 at 10:52:55PM -0500, Andrei Matei wrote: > Before this patch, profiler.inc.h wouldn't compile with clang-11 (before > the __builtin_preserve_enum_value LLVM builtin was introduced in > https://reviews.llvm.org/D83242). > Another test that uses this builtin (test_core_enumval) is conditionally > skipped if the compiler is too old. In that spirit, this patch inhibits > part of populate_cgroup_info(), which needs this CO-RE builtin. The > selftests build again on clang-11. The affected test (the profiler > test) doesn't pass on clang-11 because it's missing > https://reviews.llvm.org/D85570, but at least the test suite as a whole > compiles. The test's expected failure is already called out in the > README. > > Signed-off-by: Andrei Matei <andreimatei1@gmail.com> Tested-by: Florian Lehner <dev@der-flo.net> Thanks for the fix! > --- > tools/testing/selftests/bpf/progs/profiler.inc.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h > index 30982a7e4d0f..b79d7f688655 100644 > --- a/tools/testing/selftests/bpf/progs/profiler.inc.h > +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h > @@ -256,6 +256,8 @@ static INLINE void* populate_cgroup_info(struct cgroup_data_t* cgroup_data, > BPF_CORE_READ(task, nsproxy, cgroup_ns, root_cset, dfl_cgrp, kn); > struct kernfs_node* proc_kernfs = BPF_CORE_READ(task, cgroups, dfl_cgrp, kn); > > + > +#if __has_builtin(__builtin_preserve_enum_value) > if (ENABLE_CGROUP_V1_RESOLVER && CONFIG_CGROUP_PIDS) { > int cgrp_id = bpf_core_enum_value(enum cgroup_subsys_id___local, > pids_cgrp_id___local); > @@ -275,6 +277,7 @@ static INLINE void* populate_cgroup_info(struct cgroup_data_t* cgroup_data, > } > } > } > +#endif > > cgroup_data->cgroup_root_inode = get_inode_from_kernfs(root_kernfs); > cgroup_data->cgroup_proc_inode = get_inode_from_kernfs(proc_kernfs); > -- > 2.27.0 >
On 11/25/20 4:52 AM, Andrei Matei wrote: > Before this patch, profiler.inc.h wouldn't compile with clang-11 (before > the __builtin_preserve_enum_value LLVM builtin was introduced in > https://reviews.llvm.org/D83242). > Another test that uses this builtin (test_core_enumval) is conditionally > skipped if the compiler is too old. In that spirit, this patch inhibits > part of populate_cgroup_info(), which needs this CO-RE builtin. The > selftests build again on clang-11. The affected test (the profiler > test) doesn't pass on clang-11 because it's missing > https://reviews.llvm.org/D85570, but at least the test suite as a whole > compiles. The test's expected failure is already called out in the > README. > > Signed-off-by: Andrei Matei <andreimatei1@gmail.com> Looks good, applied, thanks!
diff --git a/tools/testing/selftests/bpf/progs/profiler.inc.h b/tools/testing/selftests/bpf/progs/profiler.inc.h index 30982a7e4d0f..b79d7f688655 100644 --- a/tools/testing/selftests/bpf/progs/profiler.inc.h +++ b/tools/testing/selftests/bpf/progs/profiler.inc.h @@ -256,6 +256,8 @@ static INLINE void* populate_cgroup_info(struct cgroup_data_t* cgroup_data, BPF_CORE_READ(task, nsproxy, cgroup_ns, root_cset, dfl_cgrp, kn); struct kernfs_node* proc_kernfs = BPF_CORE_READ(task, cgroups, dfl_cgrp, kn); + +#if __has_builtin(__builtin_preserve_enum_value) if (ENABLE_CGROUP_V1_RESOLVER && CONFIG_CGROUP_PIDS) { int cgrp_id = bpf_core_enum_value(enum cgroup_subsys_id___local, pids_cgrp_id___local); @@ -275,6 +277,7 @@ static INLINE void* populate_cgroup_info(struct cgroup_data_t* cgroup_data, } } } +#endif cgroup_data->cgroup_root_inode = get_inode_from_kernfs(root_kernfs); cgroup_data->cgroup_proc_inode = get_inode_from_kernfs(proc_kernfs);
Before this patch, profiler.inc.h wouldn't compile with clang-11 (before the __builtin_preserve_enum_value LLVM builtin was introduced in https://reviews.llvm.org/D83242). Another test that uses this builtin (test_core_enumval) is conditionally skipped if the compiler is too old. In that spirit, this patch inhibits part of populate_cgroup_info(), which needs this CO-RE builtin. The selftests build again on clang-11. The affected test (the profiler test) doesn't pass on clang-11 because it's missing https://reviews.llvm.org/D85570, but at least the test suite as a whole compiles. The test's expected failure is already called out in the README. Signed-off-by: Andrei Matei <andreimatei1@gmail.com> --- tools/testing/selftests/bpf/progs/profiler.inc.h | 3 +++ 1 file changed, 3 insertions(+)