| Message ID | 20190115225447.245788-3-sdf@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf,1/3] bpf: don't assume build-id length is always 20 bytes | expand |
> On Jan 15, 2019, at 2:54 PM, Stanislav Fomichev <sdf@google.com> wrote: > > While running test_progs in a loop I found out that I'm sometimes hitting > "Didn't find expected build ID from the map" error. > Looking at stack_map_get_build_id_offset() it seems that it is racy (by > design) and can sometimes return BPF_STACK_BUILD_ID_IP (i.e. can't trylock > current->mm->mmap_sem). > > Let's retry this test a single time. > > Fixes: 13790d1cc72c ("bpf: add selftest for stackmap with build_id in NMI context") > Signed-off-by: Stanislav Fomichev <sdf@google.com> Acked-by: Song Liu <songliubraving@fb.com> > --- > tools/testing/selftests/bpf/test_progs.c | 30 ++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index 126fc624290d..25f0083a9b2e 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -1188,7 +1188,9 @@ static void test_stacktrace_build_id(void) > int i, j; > struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH]; > int build_id_matches = 0; > + int retry = 1; > > +retry: > err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd); > if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno)) > goto out; > @@ -1301,6 +1303,19 @@ static void test_stacktrace_build_id(void) > previous_key = key; > } while (bpf_map_get_next_key(stackmap_fd, &previous_key, &key) == 0); > > + /* stack_map_get_build_id_offset() is racy and sometimes can return > + * BPF_STACK_BUILD_ID_IP instead of BPF_STACK_BUILD_ID_VALID; > + * try it one more time. > + */ > + if (build_id_matches < 1 && retry--) { > + ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE); > + close(pmu_fd); > + bpf_object__close(obj); > + printf("%s:WARN:Didn't find expected build ID from the map, retrying\n", > + __func__); > + goto retry; > + } > + > if (CHECK(build_id_matches < 1, "build id match", > "Didn't find expected build ID from the map\n")) > goto disable_pmu; > @@ -1341,7 +1356,9 @@ static void test_stacktrace_build_id_nmi(void) > int i, j; > struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH]; > int build_id_matches = 0; > + int retry = 1; > > +retry: > err = bpf_prog_load(file, BPF_PROG_TYPE_PERF_EVENT, &obj, &prog_fd); > if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno)) > return; > @@ -1436,6 +1453,19 @@ static void test_stacktrace_build_id_nmi(void) > previous_key = key; > } while (bpf_map_get_next_key(stackmap_fd, &previous_key, &key) == 0); > > + /* stack_map_get_build_id_offset() is racy and sometimes can return > + * BPF_STACK_BUILD_ID_IP instead of BPF_STACK_BUILD_ID_VALID; > + * try it one more time. > + */ > + if (build_id_matches < 1 && retry--) { > + ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE); > + close(pmu_fd); > + bpf_object__close(obj); > + printf("%s:WARN:Didn't find expected build ID from the map, retrying\n", > + __func__); > + goto retry; > + } > + > if (CHECK(build_id_matches < 1, "build id match", > "Didn't find expected build ID from the map\n")) > goto disable_pmu; > -- > 2.20.1.97.g81188d93c3-goog >
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 126fc624290d..25f0083a9b2e 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -1188,7 +1188,9 @@ static void test_stacktrace_build_id(void) int i, j; struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH]; int build_id_matches = 0; + int retry = 1; +retry: err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd); if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno)) goto out; @@ -1301,6 +1303,19 @@ static void test_stacktrace_build_id(void) previous_key = key; } while (bpf_map_get_next_key(stackmap_fd, &previous_key, &key) == 0); + /* stack_map_get_build_id_offset() is racy and sometimes can return + * BPF_STACK_BUILD_ID_IP instead of BPF_STACK_BUILD_ID_VALID; + * try it one more time. + */ + if (build_id_matches < 1 && retry--) { + ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE); + close(pmu_fd); + bpf_object__close(obj); + printf("%s:WARN:Didn't find expected build ID from the map, retrying\n", + __func__); + goto retry; + } + if (CHECK(build_id_matches < 1, "build id match", "Didn't find expected build ID from the map\n")) goto disable_pmu; @@ -1341,7 +1356,9 @@ static void test_stacktrace_build_id_nmi(void) int i, j; struct bpf_stack_build_id id_offs[PERF_MAX_STACK_DEPTH]; int build_id_matches = 0; + int retry = 1; +retry: err = bpf_prog_load(file, BPF_PROG_TYPE_PERF_EVENT, &obj, &prog_fd); if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno)) return; @@ -1436,6 +1453,19 @@ static void test_stacktrace_build_id_nmi(void) previous_key = key; } while (bpf_map_get_next_key(stackmap_fd, &previous_key, &key) == 0); + /* stack_map_get_build_id_offset() is racy and sometimes can return + * BPF_STACK_BUILD_ID_IP instead of BPF_STACK_BUILD_ID_VALID; + * try it one more time. + */ + if (build_id_matches < 1 && retry--) { + ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE); + close(pmu_fd); + bpf_object__close(obj); + printf("%s:WARN:Didn't find expected build ID from the map, retrying\n", + __func__); + goto retry; + } + if (CHECK(build_id_matches < 1, "build id match", "Didn't find expected build ID from the map\n")) goto disable_pmu;
While running test_progs in a loop I found out that I'm sometimes hitting "Didn't find expected build ID from the map" error. Looking at stack_map_get_build_id_offset() it seems that it is racy (by design) and can sometimes return BPF_STACK_BUILD_ID_IP (i.e. can't trylock current->mm->mmap_sem). Let's retry this test a single time. Fixes: 13790d1cc72c ("bpf: add selftest for stackmap with build_id in NMI context") Signed-off-by: Stanislav Fomichev <sdf@google.com> --- tools/testing/selftests/bpf/test_progs.c | 30 ++++++++++++++++++++++++ 1 file changed, 30 insertions(+)