| Message ID | 20220301033907.902728-1-mykolal@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | [v2,bpf-next] Improve BPF test stability (related to perf events and scheduling) | expand |
On 2/28/22 7:39 PM, Mykola Lysenko wrote: > In send_signal, replace sleep with dummy cpu intensive computation > to increase probability of child process being scheduled. Add few > more asserts. > > In find_vma, reduce sample_freq as higher values may be rejected in > some qemu setups, remove usleep and increase length of cpu intensive > computation. > > In bpf_cookie, perf_link and perf_branches, reduce sample_freq as > higher values may be rejected in some qemu setups > > Signed-off-by: Mykola Lysenko <mykolal@fb.com> LGTM with a few nits below. Acked-by: Yonghong Song <yhs@fb.com> > --- > .../selftests/bpf/prog_tests/bpf_cookie.c | 2 +- > .../testing/selftests/bpf/prog_tests/find_vma.c | 13 ++++++++++--- > .../selftests/bpf/prog_tests/perf_branches.c | 4 ++-- > .../selftests/bpf/prog_tests/perf_link.c | 2 +- > .../selftests/bpf/prog_tests/send_signal.c | 17 ++++++++++------- > .../selftests/bpf/progs/test_send_signal_kern.c | 2 +- > 6 files changed, 25 insertions(+), 15 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c > index cd10df6cd0fc..0612e79a9281 100644 > --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c > @@ -199,7 +199,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 = 1000; > 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/find_vma.c b/tools/testing/selftests/bpf/prog_tests/find_vma.c > index b74b3c0c555a..a0b68381cd79 100644 > --- a/tools/testing/selftests/bpf/prog_tests/find_vma.c > +++ b/tools/testing/selftests/bpf/prog_tests/find_vma.c > @@ -30,12 +30,20 @@ static int open_pe(void) > attr.type = PERF_TYPE_HARDWARE; > attr.config = PERF_COUNT_HW_CPU_CYCLES; > attr.freq = 1; > - attr.sample_freq = 4000; > + attr.sample_freq = 1000; > pfd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, PERF_FLAG_FD_CLOEXEC); > > return pfd >= 0 ? pfd : -errno; > } > > +static bool find_vma_pe_condition(struct find_vma *skel) > +{ > + return skel->bss->found_vm_exec != 1 || In test_and_reset_skel(), we have following codes for reset/default values: skel->bss->found_vm_exec = 0; skel->data->find_addr_ret = -1; skel->data->find_zero_ret = -1; skel->bss->d_iname[0] = 0; I think we should stick to them, so it would be good to change skel->bss->found_vm_exec != 1 to skel->bss->found_vm_exec == 0 > + skel->data->find_addr_ret == -1 || > + skel->data->find_zero_ret != 0 || Change skel->data->find_zero_ret != 0 to skel->data->find_zero_ret == -1 The bpf program may set skel->data->find_zero_ret to -ENOENT (-2) or -EBUSY (-16) in which case we should stop the iteration. > + skel->bss->d_iname[0] == 0; > +} > + > static void test_find_vma_pe(struct find_vma *skel) > { > struct bpf_link *link = NULL; > @@ -57,7 +65,7 @@ static void test_find_vma_pe(struct find_vma *skel) > if (!ASSERT_OK_PTR(link, "attach_perf_event")) > goto cleanup; > > - for (i = 0; i < 1000000; ++i) > + for (i = 0; i < 1000000000 && find_vma_pe_condition(skel); ++i) > ++j; > > test_and_reset_skel(skel, -EBUSY /* in nmi, irq_work is busy */); > @@ -108,7 +116,6 @@ void serial_test_find_vma(void) > skel->bss->addr = (__u64)(uintptr_t)test_find_vma_pe; > > test_find_vma_pe(skel); > - usleep(100000); /* allow the irq_work to finish */ > test_find_vma_kprobe(skel); > > find_vma__destroy(skel); > diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c > index 12c4f45cee1a..bc24f83339d6 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 = 1000; > 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 = 1000; > 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 ede07344f264..224eba6fef2e 100644 > --- a/tools/testing/selftests/bpf/prog_tests/perf_link.c > +++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c > @@ -39,7 +39,7 @@ void serial_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 = 1000; > 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/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c > index 776916b61c40..b1b574c7016a 100644 > --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c > +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c > @@ -4,11 +4,11 @@ > #include <sys/resource.h> > #include "test_send_signal_kern.skel.h" > > -int sigusr1_received = 0; > +static int sigusr1_received; > > static void sigusr1_handler(int signum) > { > - sigusr1_received++; > + sigusr1_received = 1; > } > > static void test_send_signal_common(struct perf_event_attr *attr, > @@ -40,9 +40,10 @@ static void test_send_signal_common(struct perf_event_attr *attr, > > if (pid == 0) { > int old_prio; > + volatile int volatile_variable = 0; I think it is okay to use variable 'j' to be consistent with other similar codes in selftests. > > /* install signal handler and notify parent */ > - signal(SIGUSR1, sigusr1_handler); > + ASSERT_NEQ(signal(SIGUSR1, sigusr1_handler), SIG_ERR, "signal"); > > close(pipe_c2p[0]); /* close read */ > close(pipe_p2c[1]); /* close write */ > @@ -63,9 +64,11 @@ static void test_send_signal_common(struct perf_event_attr *attr, > ASSERT_EQ(read(pipe_p2c[0], buf, 1), 1, "pipe_read"); > > /* wait a little for signal handler */ > - sleep(1); > + for (int i = 0; i < 100000000 && !sigusr1_received; i++) > + volatile_variable /= i + 1; > > buf[0] = sigusr1_received ? '2' : '0'; > + ASSERT_EQ(sigusr1_received, 1, "sigusr1_received"); > ASSERT_EQ(write(pipe_c2p[1], buf, 1), 1, "pipe_write"); > > /* wait for parent notification and exit */ [...]
> On Mar 1, 2022, at 9:23 PM, Yonghong Song <yhs@fb.com> wrote: > > > > On 2/28/22 7:39 PM, Mykola Lysenko wrote: >> In send_signal, replace sleep with dummy cpu intensive computation >> to increase probability of child process being scheduled. Add few >> more asserts. >> In find_vma, reduce sample_freq as higher values may be rejected in >> some qemu setups, remove usleep and increase length of cpu intensive >> computation. >> In bpf_cookie, perf_link and perf_branches, reduce sample_freq as >> higher values may be rejected in some qemu setups >> Signed-off-by: Mykola Lysenko <mykolal@fb.com> > > LGTM with a few nits below. > > Acked-by: Yonghong Song <yhs@fb.com> Thanks for the review! > >> --- >> .../selftests/bpf/prog_tests/bpf_cookie.c | 2 +- >> .../testing/selftests/bpf/prog_tests/find_vma.c | 13 ++++++++++--- >> .../selftests/bpf/prog_tests/perf_branches.c | 4 ++-- >> .../selftests/bpf/prog_tests/perf_link.c | 2 +- >> .../selftests/bpf/prog_tests/send_signal.c | 17 ++++++++++------- >> .../selftests/bpf/progs/test_send_signal_kern.c | 2 +- >> 6 files changed, 25 insertions(+), 15 deletions(-) >> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c >> index cd10df6cd0fc..0612e79a9281 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c >> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c >> @@ -199,7 +199,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 = 1000; >> 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/find_vma.c b/tools/testing/selftests/bpf/prog_tests/find_vma.c >> index b74b3c0c555a..a0b68381cd79 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/find_vma.c >> +++ b/tools/testing/selftests/bpf/prog_tests/find_vma.c >> @@ -30,12 +30,20 @@ static int open_pe(void) >> attr.type = PERF_TYPE_HARDWARE; >> attr.config = PERF_COUNT_HW_CPU_CYCLES; >> attr.freq = 1; >> - attr.sample_freq = 4000; >> + attr.sample_freq = 1000; >> pfd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, PERF_FLAG_FD_CLOEXEC); >> return pfd >= 0 ? pfd : -errno; >> } >> +static bool find_vma_pe_condition(struct find_vma *skel) >> +{ >> + return skel->bss->found_vm_exec != 1 || > > In test_and_reset_skel(), we have following codes for reset/default values: > skel->bss->found_vm_exec = 0; > skel->data->find_addr_ret = -1; > skel->data->find_zero_ret = -1; > skel->bss->d_iname[0] = 0; > > I think we should stick to them, so it would be good > to change > skel->bss->found_vm_exec != 1 > to > skel->bss->found_vm_exec == 0 > >> + skel->data->find_addr_ret == -1 || >> + skel->data->find_zero_ret != 0 || > > Change > skel->data->find_zero_ret != 0 > to > skel->data->find_zero_ret == -1 > > The bpf program may set skel->data->find_zero_ret to > -ENOENT (-2) or -EBUSY (-16) in which case we should > stop the iteration. Debugged this test a bit more, and it seems we should continue iterating when -16 is returned, as it converges to 0 eventually and test passes. Will you be ok to add check that find_zero_ret == -1 or == -16? > >> + skel->bss->d_iname[0] == 0; >> +} >> + >> static void test_find_vma_pe(struct find_vma *skel) >> { >> struct bpf_link *link = NULL; >> @@ -57,7 +65,7 @@ static void test_find_vma_pe(struct find_vma *skel) >> if (!ASSERT_OK_PTR(link, "attach_perf_event")) >> goto cleanup; >> - for (i = 0; i < 1000000; ++i) >> + for (i = 0; i < 1000000000 && find_vma_pe_condition(skel); ++i) >> ++j; >> test_and_reset_skel(skel, -EBUSY /* in nmi, irq_work is busy */); >> @@ -108,7 +116,6 @@ void serial_test_find_vma(void) >> skel->bss->addr = (__u64)(uintptr_t)test_find_vma_pe; >> test_find_vma_pe(skel); >> - usleep(100000); /* allow the irq_work to finish */ >> test_find_vma_kprobe(skel); >> find_vma__destroy(skel); >> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c >> index 12c4f45cee1a..bc24f83339d6 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 = 1000; >> 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 = 1000; >> 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 ede07344f264..224eba6fef2e 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/perf_link.c >> +++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c >> @@ -39,7 +39,7 @@ void serial_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 = 1000; >> 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/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c >> index 776916b61c40..b1b574c7016a 100644 >> --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c >> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c >> @@ -4,11 +4,11 @@ >> #include <sys/resource.h> >> #include "test_send_signal_kern.skel.h" >> -int sigusr1_received = 0; >> +static int sigusr1_received; >> static void sigusr1_handler(int signum) >> { >> - sigusr1_received++; >> + sigusr1_received = 1; >> } >> static void test_send_signal_common(struct perf_event_attr *attr, >> @@ -40,9 +40,10 @@ static void test_send_signal_common(struct perf_event_attr *attr, >> if (pid == 0) { >> int old_prio; >> + volatile int volatile_variable = 0; > > I think it is okay to use variable 'j' to be consistent with other > similar codes in selftests. Sounds good > >> /* install signal handler and notify parent */ >> - signal(SIGUSR1, sigusr1_handler); >> + ASSERT_NEQ(signal(SIGUSR1, sigusr1_handler), SIG_ERR, "signal"); >> close(pipe_c2p[0]); /* close read */ >> close(pipe_p2c[1]); /* close write */ >> @@ -63,9 +64,11 @@ static void test_send_signal_common(struct perf_event_attr *attr, >> ASSERT_EQ(read(pipe_p2c[0], buf, 1), 1, "pipe_read"); >> /* wait a little for signal handler */ >> - sleep(1); >> + for (int i = 0; i < 100000000 && !sigusr1_received; i++) >> + volatile_variable /= i + 1; >> buf[0] = sigusr1_received ? '2' : '0'; >> + ASSERT_EQ(sigusr1_received, 1, "sigusr1_received"); >> ASSERT_EQ(write(pipe_c2p[1], buf, 1), 1, "pipe_write"); >> /* wait for parent notification and exit */ > [...]
> On Mar 2, 2022, at 1:03 PM, Mykola Lysenko <mykolal@fb.com> wrote: > > > >> On Mar 1, 2022, at 9:23 PM, Yonghong Song <yhs@fb.com> wrote: >> >> >> >> On 2/28/22 7:39 PM, Mykola Lysenko wrote: >>> In send_signal, replace sleep with dummy cpu intensive computation >>> to increase probability of child process being scheduled. Add few >>> more asserts. >>> In find_vma, reduce sample_freq as higher values may be rejected in >>> some qemu setups, remove usleep and increase length of cpu intensive >>> computation. >>> In bpf_cookie, perf_link and perf_branches, reduce sample_freq as >>> higher values may be rejected in some qemu setups >>> Signed-off-by: Mykola Lysenko <mykolal@fb.com> >> >> LGTM with a few nits below. >> >> Acked-by: Yonghong Song <yhs@fb.com> > > Thanks for the review! > >> >>> --- >>> .../selftests/bpf/prog_tests/bpf_cookie.c | 2 +- >>> .../testing/selftests/bpf/prog_tests/find_vma.c | 13 ++++++++++--- >>> .../selftests/bpf/prog_tests/perf_branches.c | 4 ++-- >>> .../selftests/bpf/prog_tests/perf_link.c | 2 +- >>> .../selftests/bpf/prog_tests/send_signal.c | 17 ++++++++++------- >>> .../selftests/bpf/progs/test_send_signal_kern.c | 2 +- >>> 6 files changed, 25 insertions(+), 15 deletions(-) >>> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c >>> index cd10df6cd0fc..0612e79a9281 100644 >>> --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c >>> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c >>> @@ -199,7 +199,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 = 1000; >>> 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/find_vma.c b/tools/testing/selftests/bpf/prog_tests/find_vma.c >>> index b74b3c0c555a..a0b68381cd79 100644 >>> --- a/tools/testing/selftests/bpf/prog_tests/find_vma.c >>> +++ b/tools/testing/selftests/bpf/prog_tests/find_vma.c >>> @@ -30,12 +30,20 @@ static int open_pe(void) >>> attr.type = PERF_TYPE_HARDWARE; >>> attr.config = PERF_COUNT_HW_CPU_CYCLES; >>> attr.freq = 1; >>> - attr.sample_freq = 4000; >>> + attr.sample_freq = 1000; >>> pfd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, PERF_FLAG_FD_CLOEXEC); >>> return pfd >= 0 ? pfd : -errno; >>> } >>> +static bool find_vma_pe_condition(struct find_vma *skel) >>> +{ >>> + return skel->bss->found_vm_exec != 1 || >> >> In test_and_reset_skel(), we have following codes for reset/default values: >> skel->bss->found_vm_exec = 0; >> skel->data->find_addr_ret = -1; >> skel->data->find_zero_ret = -1; >> skel->bss->d_iname[0] = 0; >> >> I think we should stick to them, so it would be good >> to change >> skel->bss->found_vm_exec != 1 >> to >> skel->bss->found_vm_exec == 0 >> >>> + skel->data->find_addr_ret == -1 || >>> + skel->data->find_zero_ret != 0 || >> >> Change >> skel->data->find_zero_ret != 0 >> to >> skel->data->find_zero_ret == -1 >> >> The bpf program may set skel->data->find_zero_ret to >> -ENOENT (-2) or -EBUSY (-16) in which case we should >> stop the iteration. > > Debugged this test a bit more, and it seems we should continue iterating when -16 is returned, as it converges to 0 eventually and test passes. > > Will you be ok to add check that find_zero_ret == -1 or == -16? Correction, I read your comment incorrectly. Will do the change as you asked. > >> >>> + skel->bss->d_iname[0] == 0; >>> +} >>> + >>> static void test_find_vma_pe(struct find_vma *skel) >>> { >>> struct bpf_link *link = NULL; >>> @@ -57,7 +65,7 @@ static void test_find_vma_pe(struct find_vma *skel) >>> if (!ASSERT_OK_PTR(link, "attach_perf_event")) >>> goto cleanup; >>> - for (i = 0; i < 1000000; ++i) >>> + for (i = 0; i < 1000000000 && find_vma_pe_condition(skel); ++i) >>> ++j; >>> test_and_reset_skel(skel, -EBUSY /* in nmi, irq_work is busy */); >>> @@ -108,7 +116,6 @@ void serial_test_find_vma(void) >>> skel->bss->addr = (__u64)(uintptr_t)test_find_vma_pe; >>> test_find_vma_pe(skel); >>> - usleep(100000); /* allow the irq_work to finish */ >>> test_find_vma_kprobe(skel); >>> find_vma__destroy(skel); >>> diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c >>> index 12c4f45cee1a..bc24f83339d6 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 = 1000; >>> 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 = 1000; >>> 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 ede07344f264..224eba6fef2e 100644 >>> --- a/tools/testing/selftests/bpf/prog_tests/perf_link.c >>> +++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c >>> @@ -39,7 +39,7 @@ void serial_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 = 1000; >>> 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/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c >>> index 776916b61c40..b1b574c7016a 100644 >>> --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c >>> +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c >>> @@ -4,11 +4,11 @@ >>> #include <sys/resource.h> >>> #include "test_send_signal_kern.skel.h" >>> -int sigusr1_received = 0; >>> +static int sigusr1_received; >>> static void sigusr1_handler(int signum) >>> { >>> - sigusr1_received++; >>> + sigusr1_received = 1; >>> } >>> static void test_send_signal_common(struct perf_event_attr *attr, >>> @@ -40,9 +40,10 @@ static void test_send_signal_common(struct perf_event_attr *attr, >>> if (pid == 0) { >>> int old_prio; >>> + volatile int volatile_variable = 0; >> >> I think it is okay to use variable 'j' to be consistent with other >> similar codes in selftests. > > Sounds good > >> >>> /* install signal handler and notify parent */ >>> - signal(SIGUSR1, sigusr1_handler); >>> + ASSERT_NEQ(signal(SIGUSR1, sigusr1_handler), SIG_ERR, "signal"); >>> close(pipe_c2p[0]); /* close read */ >>> close(pipe_p2c[1]); /* close write */ >>> @@ -63,9 +64,11 @@ static void test_send_signal_common(struct perf_event_attr *attr, >>> ASSERT_EQ(read(pipe_p2c[0], buf, 1), 1, "pipe_read"); >>> /* wait a little for signal handler */ >>> - sleep(1); >>> + for (int i = 0; i < 100000000 && !sigusr1_received; i++) >>> + volatile_variable /= i + 1; >>> buf[0] = sigusr1_received ? '2' : '0'; >>> + ASSERT_EQ(sigusr1_received, 1, "sigusr1_received"); >>> ASSERT_EQ(write(pipe_c2p[1], buf, 1), 1, "pipe_write"); >>> /* wait for parent notification and exit */ >> [...]
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c index cd10df6cd0fc..0612e79a9281 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_cookie.c @@ -199,7 +199,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 = 1000; 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/find_vma.c b/tools/testing/selftests/bpf/prog_tests/find_vma.c index b74b3c0c555a..a0b68381cd79 100644 --- a/tools/testing/selftests/bpf/prog_tests/find_vma.c +++ b/tools/testing/selftests/bpf/prog_tests/find_vma.c @@ -30,12 +30,20 @@ static int open_pe(void) attr.type = PERF_TYPE_HARDWARE; attr.config = PERF_COUNT_HW_CPU_CYCLES; attr.freq = 1; - attr.sample_freq = 4000; + attr.sample_freq = 1000; pfd = syscall(__NR_perf_event_open, &attr, 0, -1, -1, PERF_FLAG_FD_CLOEXEC); return pfd >= 0 ? pfd : -errno; } +static bool find_vma_pe_condition(struct find_vma *skel) +{ + return skel->bss->found_vm_exec != 1 || + skel->data->find_addr_ret == -1 || + skel->data->find_zero_ret != 0 || + skel->bss->d_iname[0] == 0; +} + static void test_find_vma_pe(struct find_vma *skel) { struct bpf_link *link = NULL; @@ -57,7 +65,7 @@ static void test_find_vma_pe(struct find_vma *skel) if (!ASSERT_OK_PTR(link, "attach_perf_event")) goto cleanup; - for (i = 0; i < 1000000; ++i) + for (i = 0; i < 1000000000 && find_vma_pe_condition(skel); ++i) ++j; test_and_reset_skel(skel, -EBUSY /* in nmi, irq_work is busy */); @@ -108,7 +116,6 @@ void serial_test_find_vma(void) skel->bss->addr = (__u64)(uintptr_t)test_find_vma_pe; test_find_vma_pe(skel); - usleep(100000); /* allow the irq_work to finish */ test_find_vma_kprobe(skel); find_vma__destroy(skel); diff --git a/tools/testing/selftests/bpf/prog_tests/perf_branches.c b/tools/testing/selftests/bpf/prog_tests/perf_branches.c index 12c4f45cee1a..bc24f83339d6 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 = 1000; 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 = 1000; 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 ede07344f264..224eba6fef2e 100644 --- a/tools/testing/selftests/bpf/prog_tests/perf_link.c +++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c @@ -39,7 +39,7 @@ void serial_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 = 1000; 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/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c index 776916b61c40..b1b574c7016a 100644 --- a/tools/testing/selftests/bpf/prog_tests/send_signal.c +++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c @@ -4,11 +4,11 @@ #include <sys/resource.h> #include "test_send_signal_kern.skel.h" -int sigusr1_received = 0; +static int sigusr1_received; static void sigusr1_handler(int signum) { - sigusr1_received++; + sigusr1_received = 1; } static void test_send_signal_common(struct perf_event_attr *attr, @@ -40,9 +40,10 @@ static void test_send_signal_common(struct perf_event_attr *attr, if (pid == 0) { int old_prio; + volatile int volatile_variable = 0; /* install signal handler and notify parent */ - signal(SIGUSR1, sigusr1_handler); + ASSERT_NEQ(signal(SIGUSR1, sigusr1_handler), SIG_ERR, "signal"); close(pipe_c2p[0]); /* close read */ close(pipe_p2c[1]); /* close write */ @@ -63,9 +64,11 @@ static void test_send_signal_common(struct perf_event_attr *attr, ASSERT_EQ(read(pipe_p2c[0], buf, 1), 1, "pipe_read"); /* wait a little for signal handler */ - sleep(1); + for (int i = 0; i < 100000000 && !sigusr1_received; i++) + volatile_variable /= i + 1; buf[0] = sigusr1_received ? '2' : '0'; + ASSERT_EQ(sigusr1_received, 1, "sigusr1_received"); ASSERT_EQ(write(pipe_c2p[1], buf, 1), 1, "pipe_write"); /* wait for parent notification and exit */ @@ -93,7 +96,7 @@ static void test_send_signal_common(struct perf_event_attr *attr, goto destroy_skel; } } else { - pmu_fd = syscall(__NR_perf_event_open, attr, pid, -1, + pmu_fd = syscall(__NR_perf_event_open, attr, pid, -1 /* cpu */, -1 /* group id */, 0 /* flags */); if (!ASSERT_GE(pmu_fd, 0, "perf_event_open")) { err = -1; @@ -110,9 +113,9 @@ static void test_send_signal_common(struct perf_event_attr *attr, ASSERT_EQ(read(pipe_c2p[0], buf, 1), 1, "pipe_read"); /* trigger the bpf send_signal */ - skel->bss->pid = pid; - skel->bss->sig = SIGUSR1; skel->bss->signal_thread = signal_thread; + skel->bss->sig = SIGUSR1; + skel->bss->pid = pid; /* notify child that bpf program can send_signal now */ ASSERT_EQ(write(pipe_p2c[1], buf, 1), 1, "pipe_write"); diff --git a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c index b4233d3efac2..92354cd72044 100644 --- a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c +++ b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c @@ -10,7 +10,7 @@ static __always_inline int bpf_send_signal_test(void *ctx) { int ret; - if (status != 0 || sig == 0 || pid == 0) + if (status != 0 || pid == 0) return 0; if ((bpf_get_current_pid_tgid() >> 32) == pid) {
In send_signal, replace sleep with dummy cpu intensive computation to increase probability of child process being scheduled. Add few more asserts. In find_vma, reduce sample_freq as higher values may be rejected in some qemu setups, remove usleep and increase length of cpu intensive computation. In bpf_cookie, perf_link and perf_branches, reduce sample_freq as higher values may be rejected in some qemu setups Signed-off-by: Mykola Lysenko <mykolal@fb.com> --- .../selftests/bpf/prog_tests/bpf_cookie.c | 2 +- .../testing/selftests/bpf/prog_tests/find_vma.c | 13 ++++++++++--- .../selftests/bpf/prog_tests/perf_branches.c | 4 ++-- .../selftests/bpf/prog_tests/perf_link.c | 2 +- .../selftests/bpf/prog_tests/send_signal.c | 17 ++++++++++------- .../selftests/bpf/progs/test_send_signal_kern.c | 2 +- 6 files changed, 25 insertions(+), 15 deletions(-)