* [PATCH bpf-next 2/2] selftests/bpf: Bail out quickly from failing consumer test
2024-09-24 11:07 [PATCH bpf-next 1/2] selftests/bpf: Fix uprobe consumer test Jiri Olsa
@ 2024-09-24 11:07 ` Jiri Olsa
2024-09-24 17:11 ` [PATCH bpf-next 1/2] selftests/bpf: Fix uprobe " Ihor Solodrai
2024-09-25 10:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Jiri Olsa @ 2024-09-24 11:07 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Ihor Solodrai
Let's bail out from consumer test after we hit first fail,
so we don't pollute the log with many instances with possibly
the same error.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
.../selftests/bpf/prog_tests/uprobe_multi_test.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
index c1ac813ff9ba..2c39902b8a09 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -836,10 +836,10 @@ uprobe_consumer_test(struct uprobe_multi_consumers *skel,
return 0;
}
-static void consumer_test(struct uprobe_multi_consumers *skel,
- unsigned long before, unsigned long after)
+static int consumer_test(struct uprobe_multi_consumers *skel,
+ unsigned long before, unsigned long after)
{
- int err, idx;
+ int err, idx, ret = -1;
printf("consumer_test before %lu after %lu\n", before, after);
@@ -881,13 +881,17 @@ static void consumer_test(struct uprobe_multi_consumers *skel,
fmt = "idx 2/3: uretprobe";
}
- ASSERT_EQ(skel->bss->uprobe_result[idx], val, fmt);
+ if (!ASSERT_EQ(skel->bss->uprobe_result[idx], val, fmt))
+ goto cleanup;
skel->bss->uprobe_result[idx] = 0;
}
+ ret = 0;
+
cleanup:
for (idx = 0; idx < 4; idx++)
uprobe_detach(skel, idx);
+ return ret;
}
static void test_consumers(void)
@@ -939,9 +943,11 @@ static void test_consumers(void)
for (before = 0; before < 16; before++) {
for (after = 0; after < 16; after++)
- consumer_test(skel, before, after);
+ if (consumer_test(skel, before, after))
+ goto out;
}
+out:
uprobe_multi_consumers__destroy(skel);
}
--
2.46.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH bpf-next 1/2] selftests/bpf: Fix uprobe consumer test
2024-09-24 11:07 [PATCH bpf-next 1/2] selftests/bpf: Fix uprobe consumer test Jiri Olsa
2024-09-24 11:07 ` [PATCH bpf-next 2/2] selftests/bpf: Bail out quickly from failing " Jiri Olsa
@ 2024-09-24 17:11 ` Ihor Solodrai
2024-09-25 10:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Ihor Solodrai @ 2024-09-24 17:11 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, bpf,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo
On Tuesday, September 24th, 2024 at 4:07 AM, Jiri Olsa <jolsa@kernel.org> wrote:
>
>
> With newly merged code the uprobe behaviour is slightly different
> and affects uprobe consumer test.
>
> We no longer need to check if the uprobe object is still preserved
> after removing last uretprobe, because it stays as long as there's
> pending/installed uretprobe instance.
>
> This allows to run uretprobe consumers registered 'after' uprobe was
> hit even if previous uretprobe got unregistered before being hit.
>
> The uprobe object will be now removed after the last uprobe ref is
> released and in such case it's held by ri->uprobe (return instance)
>
> which is released after the uretprobe is hit.
>
> Reported-by: Ihor Solodrai ihor.solodrai@pm.me
>
> Closes: https://lore.kernel.org/bpf/w6U8Z9fdhjnkSp2UaFaV1fGqJXvfLEtDKEUyGDkwmoruDJ_AgF_c0FFhrkeKW18OqiP-05s9yDKiT6X-Ns-avN_ABf0dcUkXqbSJN1TQSXo=@pm.me/
> Signed-off-by: Jiri Olsa jolsa@kernel.org
>
> ---
> .../testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> index 844f6fc8487b..c1ac813ff9ba 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> @@ -869,21 +869,14 @@ static void consumer_test(struct uprobe_multi_consumers skel,
> fmt = "prog 0/1: uprobe";
> } else {
> /
> - * uprobe return is tricky ;-)
> - *
> * to trigger uretprobe consumer, the uretprobe needs to be installed,
> * which means one of the 'return' uprobes was alive when probe was hit:
> *
> * idxs: 2/3 uprobe return in 'installed' mask
> - *
> - * in addition if 'after' state removes everything that was installed in
> - * 'before' state, then uprobe kernel object goes away and return uprobe
> - * is not installed and we won't hit it even if it's in 'after' state.
> /
> unsigned long had_uretprobes = before & 0b1100; / is uretprobe installed /
> - unsigned long probe_preserved = before & after; / did uprobe go away */
>
> - if (had_uretprobes && probe_preserved && test_bit(idx, after))
> + if (had_uretprobes && test_bit(idx, after))
> val++;
> fmt = "idx 2/3: uretprobe";
> }
> --
> 2.46.0
Hi Jiri,
I tested this change on top of bpf-next/master, and
selftests/bpf/vmtest.sh passes.
Thank you for the fix!
Tested-by: Ihor Solodrai <ihor.solodrai@pm.me>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH bpf-next 1/2] selftests/bpf: Fix uprobe consumer test
2024-09-24 11:07 [PATCH bpf-next 1/2] selftests/bpf: Fix uprobe consumer test Jiri Olsa
2024-09-24 11:07 ` [PATCH bpf-next 2/2] selftests/bpf: Bail out quickly from failing " Jiri Olsa
2024-09-24 17:11 ` [PATCH bpf-next 1/2] selftests/bpf: Fix uprobe " Ihor Solodrai
@ 2024-09-25 10:00 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-09-25 10:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: ast, daniel, andrii, ihor.solodrai, bpf, kafai, songliubraving,
yhs, john.fastabend, kpsingh, sdf, haoluo
Hello:
This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Tue, 24 Sep 2024 13:07:30 +0200 you wrote:
> With newly merged code the uprobe behaviour is slightly different
> and affects uprobe consumer test.
>
> We no longer need to check if the uprobe object is still preserved
> after removing last uretprobe, because it stays as long as there's
> pending/installed uretprobe instance.
>
> [...]
Here is the summary with links:
- [bpf-next,1/2] selftests/bpf: Fix uprobe consumer test
https://git.kernel.org/bpf/bpf-next/c/510c654dfa3b
- [bpf-next,2/2] selftests/bpf: Bail out quickly from failing consumer test
https://git.kernel.org/bpf/bpf-next/c/9eac650cd120
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread