* [PATCH bpf-next] selftests/bpf: Add missing bpf_iter_vma_offset__destroy call
@ 2022-10-02 15:11 Jiri Olsa
2022-10-04 0:12 ` Martin KaFai Lau
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2022-10-02 15:11 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Kui-Feng Lee, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo
Adding missing bpf_iter_vma_offset__destroy call to
test_task_vma_offset_common function and related goto jumps.
Fixes: b3e1331eb925 ("selftests/bpf: Test parameterized task BPF iterators.")
Cc: Kui-Feng Lee <kuifeng@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 3369c5ec3a17..462fe92e0736 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -1515,11 +1515,11 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool
link = bpf_program__attach_iter(skel->progs.get_vma_offset, opts);
if (!ASSERT_OK_PTR(link, "attach_iter"))
- return;
+ goto exit_skel;
iter_fd = bpf_iter_create(bpf_link__fd(link));
if (!ASSERT_GT(iter_fd, 0, "create_iter"))
- goto exit;
+ goto exit_link;
while ((len = read(iter_fd, buf, sizeof(buf))) > 0)
;
@@ -1534,8 +1534,10 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool
close(iter_fd);
-exit:
+exit_link:
bpf_link__destroy(link);
+exit_skel:
+ bpf_iter_vma_offset__destroy(skel);
}
static void test_task_vma_offset(void)
--
2.37.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf-next] selftests/bpf: Add missing bpf_iter_vma_offset__destroy call 2022-10-02 15:11 [PATCH bpf-next] selftests/bpf: Add missing bpf_iter_vma_offset__destroy call Jiri Olsa @ 2022-10-04 0:12 ` Martin KaFai Lau 2022-10-04 6:38 ` Jiri Olsa 0 siblings, 1 reply; 6+ messages in thread From: Martin KaFai Lau @ 2022-10-04 0:12 UTC (permalink / raw) To: Jiri Olsa, Kui-Feng Lee Cc: bpf, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko On 10/2/22 8:11 AM, Jiri Olsa wrote: > Adding missing bpf_iter_vma_offset__destroy call to > test_task_vma_offset_common function and related goto jumps. > > Fixes: b3e1331eb925 ("selftests/bpf: Test parameterized task BPF iterators.") > Cc: Kui-Feng Lee <kuifeng@fb.com> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > index 3369c5ec3a17..462fe92e0736 100644 > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > @@ -1515,11 +1515,11 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > link = bpf_program__attach_iter(skel->progs.get_vma_offset, opts); Thanks for the fix. A nit. Instead of adding a new goto label. How about doing skel->links.get_vma_offset = bpf_program_attach_iter(...) and bpf_iter_vma_offset__destroy(skel) will take care of the link destroy. The earlier test_task_vma_common() is doing that also. Kui-Feng, please also take a look. > if (!ASSERT_OK_PTR(link, "attach_iter")) > - return; > + goto exit_skel; > > iter_fd = bpf_iter_create(bpf_link__fd(link)); > if (!ASSERT_GT(iter_fd, 0, "create_iter")) > - goto exit; > + goto exit_link; > > while ((len = read(iter_fd, buf, sizeof(buf))) > 0) > ; > @@ -1534,8 +1534,10 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > close(iter_fd); > > -exit: > +exit_link: > bpf_link__destroy(link); > +exit_skel: > + bpf_iter_vma_offset__destroy(skel); > } > > static void test_task_vma_offset(void) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Add missing bpf_iter_vma_offset__destroy call 2022-10-04 0:12 ` Martin KaFai Lau @ 2022-10-04 6:38 ` Jiri Olsa 2022-10-04 12:16 ` Jiri Olsa 0 siblings, 1 reply; 6+ messages in thread From: Jiri Olsa @ 2022-10-04 6:38 UTC (permalink / raw) To: Martin KaFai Lau Cc: Kui-Feng Lee, bpf, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko On Mon, Oct 03, 2022 at 05:12:44PM -0700, Martin KaFai Lau wrote: > On 10/2/22 8:11 AM, Jiri Olsa wrote: > > Adding missing bpf_iter_vma_offset__destroy call to > > test_task_vma_offset_common function and related goto jumps. > > > > Fixes: b3e1331eb925 ("selftests/bpf: Test parameterized task BPF iterators.") > > Cc: Kui-Feng Lee <kuifeng@fb.com> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > index 3369c5ec3a17..462fe92e0736 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > @@ -1515,11 +1515,11 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > link = bpf_program__attach_iter(skel->progs.get_vma_offset, opts); > > Thanks for the fix. > > A nit. Instead of adding a new goto label. How about doing > > skel->links.get_vma_offset = bpf_program_attach_iter(...) > > and bpf_iter_vma_offset__destroy(skel) will take care of the link destroy. > The earlier test_task_vma_common() is doing that also. right, I forgot destroy would do that.. it'll be simpler change thanks, jirka > > Kui-Feng, please also take a look. > > > if (!ASSERT_OK_PTR(link, "attach_iter")) > > - return; > > + goto exit_skel; > > iter_fd = bpf_iter_create(bpf_link__fd(link)); > > if (!ASSERT_GT(iter_fd, 0, "create_iter")) > > - goto exit; > > + goto exit_link; > > while ((len = read(iter_fd, buf, sizeof(buf))) > 0) > > ; > > @@ -1534,8 +1534,10 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > close(iter_fd); > > -exit: > > +exit_link: > > bpf_link__destroy(link); > > +exit_skel: > > + bpf_iter_vma_offset__destroy(skel); > > } > > static void test_task_vma_offset(void) > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Add missing bpf_iter_vma_offset__destroy call 2022-10-04 6:38 ` Jiri Olsa @ 2022-10-04 12:16 ` Jiri Olsa 2022-10-05 22:58 ` Andrii Nakryiko 0 siblings, 1 reply; 6+ messages in thread From: Jiri Olsa @ 2022-10-04 12:16 UTC (permalink / raw) To: Jiri Olsa Cc: Martin KaFai Lau, Kui-Feng Lee, bpf, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko On Tue, Oct 04, 2022 at 08:38:23AM +0200, Jiri Olsa wrote: > On Mon, Oct 03, 2022 at 05:12:44PM -0700, Martin KaFai Lau wrote: > > On 10/2/22 8:11 AM, Jiri Olsa wrote: > > > Adding missing bpf_iter_vma_offset__destroy call to > > > test_task_vma_offset_common function and related goto jumps. > > > > > > Fixes: b3e1331eb925 ("selftests/bpf: Test parameterized task BPF iterators.") > > > Cc: Kui-Feng Lee <kuifeng@fb.com> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 8 +++++--- > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > index 3369c5ec3a17..462fe92e0736 100644 > > > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > @@ -1515,11 +1515,11 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > > link = bpf_program__attach_iter(skel->progs.get_vma_offset, opts); > > > > Thanks for the fix. > > > > A nit. Instead of adding a new goto label. How about doing > > > > skel->links.get_vma_offset = bpf_program_attach_iter(...) > > > > and bpf_iter_vma_offset__destroy(skel) will take care of the link destroy. > > The earlier test_task_vma_common() is doing that also. > > right, I forgot destroy would do that.. it'll be simpler change ugh actually no ;-) it's outside (of skeleton) link, so it won't get closed in bpf_iter_vma_offset__destroy the earlier test_task_vma_common does not create such link jirka > > thanks, > jirka > > > > > Kui-Feng, please also take a look. > > > > > if (!ASSERT_OK_PTR(link, "attach_iter")) > > > - return; > > > + goto exit_skel; > > > iter_fd = bpf_iter_create(bpf_link__fd(link)); > > > if (!ASSERT_GT(iter_fd, 0, "create_iter")) > > > - goto exit; > > > + goto exit_link; > > > while ((len = read(iter_fd, buf, sizeof(buf))) > 0) > > > ; > > > @@ -1534,8 +1534,10 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > > close(iter_fd); > > > -exit: > > > +exit_link: > > > bpf_link__destroy(link); > > > +exit_skel: > > > + bpf_iter_vma_offset__destroy(skel); > > > } > > > static void test_task_vma_offset(void) > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Add missing bpf_iter_vma_offset__destroy call 2022-10-04 12:16 ` Jiri Olsa @ 2022-10-05 22:58 ` Andrii Nakryiko 2022-10-06 7:03 ` Jiri Olsa 0 siblings, 1 reply; 6+ messages in thread From: Andrii Nakryiko @ 2022-10-05 22:58 UTC (permalink / raw) To: Jiri Olsa Cc: Martin KaFai Lau, Kui-Feng Lee, bpf, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko On Tue, Oct 4, 2022 at 5:16 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Tue, Oct 04, 2022 at 08:38:23AM +0200, Jiri Olsa wrote: > > On Mon, Oct 03, 2022 at 05:12:44PM -0700, Martin KaFai Lau wrote: > > > On 10/2/22 8:11 AM, Jiri Olsa wrote: > > > > Adding missing bpf_iter_vma_offset__destroy call to > > > > test_task_vma_offset_common function and related goto jumps. > > > > > > > > Fixes: b3e1331eb925 ("selftests/bpf: Test parameterized task BPF iterators.") > > > > Cc: Kui-Feng Lee <kuifeng@fb.com> > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > --- > > > > tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 8 +++++--- > > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > > index 3369c5ec3a17..462fe92e0736 100644 > > > > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > > @@ -1515,11 +1515,11 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > > > link = bpf_program__attach_iter(skel->progs.get_vma_offset, opts); > > > > > > Thanks for the fix. > > > > > > A nit. Instead of adding a new goto label. How about doing > > > > > > skel->links.get_vma_offset = bpf_program_attach_iter(...) > > > > > > and bpf_iter_vma_offset__destroy(skel) will take care of the link destroy. > > > The earlier test_task_vma_common() is doing that also. > > > > right, I forgot destroy would do that.. it'll be simpler change > > ugh actually no ;-) it's outside (of skeleton) link, > so it won't get closed in bpf_iter_vma_offset__destroy Martin's point was that if you assign it to skel->links.get_vma_offset it will be closed by skeleton's destroy method. So let's do that? > > the earlier test_task_vma_common does not create such link > > jirka > > > > > thanks, > > jirka > > > > > > > > Kui-Feng, please also take a look. > > > > > > > if (!ASSERT_OK_PTR(link, "attach_iter")) > > > > - return; > > > > + goto exit_skel; > > > > iter_fd = bpf_iter_create(bpf_link__fd(link)); > > > > if (!ASSERT_GT(iter_fd, 0, "create_iter")) > > > > - goto exit; > > > > + goto exit_link; > > > > while ((len = read(iter_fd, buf, sizeof(buf))) > 0) > > > > ; > > > > @@ -1534,8 +1534,10 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > > > close(iter_fd); > > > > -exit: > > > > +exit_link: > > > > bpf_link__destroy(link); > > > > +exit_skel: > > > > + bpf_iter_vma_offset__destroy(skel); > > > > } > > > > static void test_task_vma_offset(void) > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Add missing bpf_iter_vma_offset__destroy call 2022-10-05 22:58 ` Andrii Nakryiko @ 2022-10-06 7:03 ` Jiri Olsa 0 siblings, 0 replies; 6+ messages in thread From: Jiri Olsa @ 2022-10-06 7:03 UTC (permalink / raw) To: Andrii Nakryiko Cc: Jiri Olsa, Martin KaFai Lau, Kui-Feng Lee, bpf, Song Liu, Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko On Wed, Oct 05, 2022 at 03:58:45PM -0700, Andrii Nakryiko wrote: > On Tue, Oct 4, 2022 at 5:16 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Tue, Oct 04, 2022 at 08:38:23AM +0200, Jiri Olsa wrote: > > > On Mon, Oct 03, 2022 at 05:12:44PM -0700, Martin KaFai Lau wrote: > > > > On 10/2/22 8:11 AM, Jiri Olsa wrote: > > > > > Adding missing bpf_iter_vma_offset__destroy call to > > > > > test_task_vma_offset_common function and related goto jumps. > > > > > > > > > > Fixes: b3e1331eb925 ("selftests/bpf: Test parameterized task BPF iterators.") > > > > > Cc: Kui-Feng Lee <kuifeng@fb.com> > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > > --- > > > > > tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 8 +++++--- > > > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > > > index 3369c5ec3a17..462fe92e0736 100644 > > > > > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > > > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > > > > > @@ -1515,11 +1515,11 @@ static void test_task_vma_offset_common(struct bpf_iter_attach_opts *opts, bool > > > > > link = bpf_program__attach_iter(skel->progs.get_vma_offset, opts); > > > > > > > > Thanks for the fix. > > > > > > > > A nit. Instead of adding a new goto label. How about doing > > > > > > > > skel->links.get_vma_offset = bpf_program_attach_iter(...) > > > > > > > > and bpf_iter_vma_offset__destroy(skel) will take care of the link destroy. > > > > The earlier test_task_vma_common() is doing that also. > > > > > > right, I forgot destroy would do that.. it'll be simpler change > > > > ugh actually no ;-) it's outside (of skeleton) link, > > so it won't get closed in bpf_iter_vma_offset__destroy > > Martin's point was that if you assign it to skel->links.get_vma_offset > it will be closed by skeleton's destroy method. So let's do that? ah ok, will send new version thanks, jirka ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-06 7:04 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-02 15:11 [PATCH bpf-next] selftests/bpf: Add missing bpf_iter_vma_offset__destroy call Jiri Olsa 2022-10-04 0:12 ` Martin KaFai Lau 2022-10-04 6:38 ` Jiri Olsa 2022-10-04 12:16 ` Jiri Olsa 2022-10-05 22:58 ` Andrii Nakryiko 2022-10-06 7:03 ` Jiri Olsa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox