| Message ID | 20211006185619.364369-6-fallentree@fb.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | selftests/bpf: Add parallelism to test_progs | expand |
On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote: > > From: Yucong Sun <sunyucong@gmail.com> > > This patch moved a helper function to test_progs and make all tests > setting sampling frequency use it to read current perf_max_sample_freq, > this will avoid triggering EINVAL error. > > Signed-off-by: Yucong Sun <sunyucong@gmail.com> > --- > .../selftests/bpf/prog_tests/bpf_cookie.c | 2 +- > .../selftests/bpf/prog_tests/perf_branches.c | 4 ++-- > .../selftests/bpf/prog_tests/perf_link.c | 2 +- > .../bpf/prog_tests/stacktrace_build_id_nmi.c | 19 ++----------------- > tools/testing/selftests/bpf/test_progs.c | 15 +++++++++++++++ > tools/testing/selftests/bpf/test_progs.h | 1 + > 6 files changed, 22 insertions(+), 21 deletions(-) > We have trace_helper.c, seems like it would be better to have it there? I haven't applied this patch yet. [...] > @@ -48,6 +31,8 @@ void test_stacktrace_build_id_nmi(void) > if (CHECK(err, "skel_load", "skeleton load failed: %d\n", err)) > goto cleanup; > > + attr.sample_freq = read_perf_max_sample_freq(); > + > pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, > 0 /* cpu 0 */, -1 /* group id */, > 0 /* flags */); > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index 2ac922f8aa2c..66825313414b 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -1500,3 +1500,18 @@ int main(int argc, char **argv) > > return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS; > } > + > +__u64 read_perf_max_sample_freq(void) > +{ > + __u64 sample_freq = 1000; /* fallback to 1000 on error */ previous default was 5000, message below still claims 5000, what's the reason for changing it? > + FILE *f; > + __u32 duration = 0; > + > + f = fopen("/proc/sys/kernel/perf_event_max_sample_rate", "r"); > + if (f == NULL) > + return sample_freq; > + CHECK(fscanf(f, "%llu", &sample_freq) != 1, "Get max sample rate", > + "return default value: 5000,err %d\n", -errno); > + fclose(f); > + return sample_freq; > +} > diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h > index b239dc9fcef0..d5ca0d36cc96 100644 > --- a/tools/testing/selftests/bpf/test_progs.h > +++ b/tools/testing/selftests/bpf/test_progs.h > @@ -327,6 +327,7 @@ int extract_build_id(char *build_id, size_t size); > int kern_sync_rcu(void); > int trigger_module_test_read(int read_sz); > int trigger_module_test_write(int write_sz); > +__u64 read_perf_max_sample_freq(void); > > #ifdef __x86_64__ > #define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep" > -- > 2.30.2 >
On Fri, Oct 8, 2021 at 3:27 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Oct 6, 2021 at 11:56 AM Yucong Sun <fallentree@fb.com> wrote: > > > > From: Yucong Sun <sunyucong@gmail.com> > > > > This patch moved a helper function to test_progs and make all tests > > setting sampling frequency use it to read current perf_max_sample_freq, > > this will avoid triggering EINVAL error. > > > > Signed-off-by: Yucong Sun <sunyucong@gmail.com> > > --- > > .../selftests/bpf/prog_tests/bpf_cookie.c | 2 +- > > .../selftests/bpf/prog_tests/perf_branches.c | 4 ++-- > > .../selftests/bpf/prog_tests/perf_link.c | 2 +- > > .../bpf/prog_tests/stacktrace_build_id_nmi.c | 19 ++----------------- > > tools/testing/selftests/bpf/test_progs.c | 15 +++++++++++++++ > > tools/testing/selftests/bpf/test_progs.h | 1 + > > 6 files changed, 22 insertions(+), 21 deletions(-) > > > > We have trace_helper.c, seems like it would be better to have it > there? I haven't applied this patch yet. I did look at that file, but the content was not really related, so didn't go with it, of course we can :-D > > [...] > > > @@ -48,6 +31,8 @@ void test_stacktrace_build_id_nmi(void) > > if (CHECK(err, "skel_load", "skeleton load failed: %d\n", err)) > > goto cleanup; > > > > + attr.sample_freq = read_perf_max_sample_freq(); > > + > > pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, > > 0 /* cpu 0 */, -1 /* group id */, > > 0 /* flags */); > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > > index 2ac922f8aa2c..66825313414b 100644 > > --- a/tools/testing/selftests/bpf/test_progs.c > > +++ b/tools/testing/selftests/bpf/test_progs.c > > @@ -1500,3 +1500,18 @@ int main(int argc, char **argv) > > > > return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS; > > } > > + > > +__u64 read_perf_max_sample_freq(void) > > +{ > > + __u64 sample_freq = 1000; /* fallback to 1000 on error */ > > previous default was 5000, message below still claims 5000, what's the > The reason for changing it? This is from my observation that on my machine it frequently dip down to 3000, the test doesn't really rely on 5000 either, they were fine with 1000 even. > > > > > + FILE *f; > > + __u32 duration = 0; > > + > > + f = fopen("/proc/sys/kernel/perf_event_max_sample_rate", "r"); > > + if (f == NULL) > > + return sample_freq; > > + CHECK(fscanf(f, "%llu", &sample_freq) != 1, "Get max sample rate", > > + "return default value: 5000,err %d\n", -errno); > > + fclose(f); > > + return sample_freq; > > +} > > diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h > > index b239dc9fcef0..d5ca0d36cc96 100644 > > --- a/tools/testing/selftests/bpf/test_progs.h > > +++ b/tools/testing/selftests/bpf/test_progs.h > > @@ -327,6 +327,7 @@ int extract_build_id(char *build_id, size_t size); > > int kern_sync_rcu(void); > > int trigger_module_test_read(int read_sz); > > int trigger_module_test_write(int write_sz); > > +__u64 read_perf_max_sample_freq(void); > > > > #ifdef __x86_64__ > > #define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep" > > -- > > 2.30.2 > >
the patch can still be found here : https://www.spinics.net/lists/bpf/msg47375.html
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c index 5eea3c3a40fe..19c9f7b53cfa 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c @@ -193,7 +193,7 @@ static void pe_subtest(struct test_bpf_cookie *skel) attr.type = PERF_TYPE_SOFTWARE; attr.config = PERF_COUNT_SW_CPU_CLOCK; attr.freq = 1; - attr.sample_freq = 4000; + attr.sample_freq = read_perf_max_sample_freq(); pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC); if (!ASSERT_GE(pfd, 0, "perf_fd")) goto cleanup; diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c index 12c4f45cee1a..6b2e3dced619 100644 --- a/tools/testing/selftests/bpf/prog_tests/perf_branches.c +++ b/tools/testing/selftests/bpf/prog_tests/perf_branches.c @@ -110,7 +110,7 @@ static void test_perf_branches_hw(void) attr.type = PERF_TYPE_HARDWARE; attr.config = PERF_COUNT_HW_CPU_CYCLES; attr.freq = 1; - attr.sample_freq = 4000; + attr.sample_freq = read_perf_max_sample_freq(); attr.sample_type = PERF_SAMPLE_BRANCH_STACK; attr.branch_sample_type = PERF_SAMPLE_BRANCH_USER | PERF_SAMPLE_BRANCH_ANY; pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC); @@ -151,7 +151,7 @@ static void test_perf_branches_no_hw(void) attr.type = PERF_TYPE_SOFTWARE; attr.config = PERF_COUNT_SW_CPU_CLOCK; attr.freq = 1; - attr.sample_freq = 4000; + attr.sample_freq = read_perf_max_sample_freq(); pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC); if (CHECK(pfd < 0, "perf_event_open", "err %d\n", pfd)) return; diff --git a/tools/testing/selftests/bpf/prog_tests/perf_link.c b/tools/testing/selftests/bpf/prog_tests/perf_link.c index b1abd0c46607..74e5bd5f1c19 100644 --- a/tools/testing/selftests/bpf/prog_tests/perf_link.c +++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c @@ -38,7 +38,7 @@ void test_perf_link(void) attr.type = PERF_TYPE_SOFTWARE; attr.config = PERF_COUNT_SW_CPU_CLOCK; attr.freq = 1; - attr.sample_freq = 4000; + attr.sample_freq = read_perf_max_sample_freq(); pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC); if (!ASSERT_GE(pfd, 0, "perf_fd")) goto cleanup; diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c index 0a91d8d9954b..150536f01442 100644 --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c @@ -2,21 +2,6 @@ #include <test_progs.h> #include "test_stacktrace_build_id.skel.h" -static __u64 read_perf_max_sample_freq(void) -{ - __u64 sample_freq = 5000; /* fallback to 5000 on error */ - FILE *f; - __u32 duration = 0; - - f = fopen("/proc/sys/kernel/perf_event_max_sample_rate", "r"); - if (f == NULL) - return sample_freq; - CHECK(fscanf(f, "%llu", &sample_freq) != 1, "Get max sample rate", - "return default value: 5000,err %d\n", -errno); - fclose(f); - return sample_freq; -} - void test_stacktrace_build_id_nmi(void) { int control_map_fd, stackid_hmap_fd, stackmap_fd; @@ -34,8 +19,6 @@ void test_stacktrace_build_id_nmi(void) int build_id_matches = 0; int retry = 1; - attr.sample_freq = read_perf_max_sample_freq(); - retry: skel = test_stacktrace_build_id__open(); if (CHECK(!skel, "skel_open", "skeleton open failed\n")) @@ -48,6 +31,8 @@ void test_stacktrace_build_id_nmi(void) if (CHECK(err, "skel_load", "skeleton load failed: %d\n", err)) goto cleanup; + attr.sample_freq = read_perf_max_sample_freq(); + pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, 0 /* cpu 0 */, -1 /* group id */, 0 /* flags */); diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 2ac922f8aa2c..66825313414b 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -1500,3 +1500,18 @@ int main(int argc, char **argv) return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS; } + +__u64 read_perf_max_sample_freq(void) +{ + __u64 sample_freq = 1000; /* fallback to 1000 on error */ + FILE *f; + __u32 duration = 0; + + f = fopen("/proc/sys/kernel/perf_event_max_sample_rate", "r"); + if (f == NULL) + return sample_freq; + CHECK(fscanf(f, "%llu", &sample_freq) != 1, "Get max sample rate", + "return default value: 5000,err %d\n", -errno); + fclose(f); + return sample_freq; +} diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h index b239dc9fcef0..d5ca0d36cc96 100644 --- a/tools/testing/selftests/bpf/test_progs.h +++ b/tools/testing/selftests/bpf/test_progs.h @@ -327,6 +327,7 @@ int extract_build_id(char *build_id, size_t size); int kern_sync_rcu(void); int trigger_module_test_read(int read_sz); int trigger_module_test_write(int write_sz); +__u64 read_perf_max_sample_freq(void); #ifdef __x86_64__ #define SYS_NANOSLEEP_KPROBE_NAME "__x64_sys_nanosleep"