[PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN

[PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN

[Qiliang Yuan]

enable_stats attaches its program to sys_enter, so every syscall of the
system runs it. Once the program is detached, the test compares its
run_cnt to the number of runs the program counted itself.

Detaching doesn't wait for the runs already started on other CPUs, and
each of them increments count before run_cnt. With other tests making
syscalls in parallel, the test reads run_cnt one run behind count now
and then:

  test_enable_stats:FAIL:check_run_cnt_valid invalid run_cnt stats

Run the program with BPF_PROG_TEST_RUN instead of attaching it, so that
nothing else runs it and both counts are final when the test reads
them. As a single run can be shorter than the resolution of the clock,
run it until its run time shows, unless its runs aren't counted at all.

Fixes: 31a9f7fe9337 ("bpf: Add selftest for BPF_ENABLE_STATS")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
V1 -> V2:
- Run the program until its run time shows, as a single run can be
  shorter than the resolution of the clock (Sashiko)

v1: https://lore.kernel.org/r/20261008-selftests-bpf-enable-stats-inflight-v1-1-0cb92ee2c6d3@gmail.com
---
 .../selftests/bpf/prog_tests/enable_stats.c        | 26 +++++++++++++---------
 .../selftests/bpf/progs/test_enable_stats.c        |  2 +-
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/enable_stats.c b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
index 75f85d0fe74a4..691a82a4d6d20 100644
--- a/tools/testing/selftests/bpf/prog_tests/enable_stats.c
+++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
@@ -9,6 +9,7 @@ void test_enable_stats(void)
 	struct bpf_prog_info info;
 	__u32 info_len = sizeof(info);
 	int duration = 0;
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
 
 	skel = test_enable_stats__open_and_load();
 	if (CHECK(!skel, "skel_open_and_load", "skeleton open/load failed\n"))
@@ -20,18 +21,23 @@ void test_enable_stats(void)
 		return;
 	}
 
-	err = test_enable_stats__attach(skel);
-	if (CHECK(err, "attach_raw_tp", "err %d\n", err))
-		goto cleanup;
-
-	test_enable_stats__detach(skel);
-
 	prog_fd = bpf_program__fd(skel->progs.test_enable_stats);
-	memset(&info, 0, info_len);
-	err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
-	if (CHECK(err, "get_prog_info",
-		  "failed to get bpf_prog_info for fd %d\n", prog_fd))
+
+	/*
+	 * A single run can be too short for the clock to tell. Run the program
+	 * until its run time shows, unless its runs aren't counted at all.
+	 */
+	do {
+		err = bpf_prog_test_run_opts(prog_fd, &topts);
+		if (err)
+			break;
+
+		memset(&info, 0, info_len);
+		err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
+	} while (!err && info.run_cnt && !info.run_time_ns);
+	if (CHECK(err, "run_and_get_prog_info", "err %d\n", err))
 		goto cleanup;
+
 	if (CHECK(info.run_time_ns == 0, "check_stats_enabled",
 		  "failed to enable run_time_ns stats\n"))
 		goto cleanup;
diff --git a/tools/testing/selftests/bpf/progs/test_enable_stats.c b/tools/testing/selftests/bpf/progs/test_enable_stats.c
index 1705097d01d7b..e28d674fc77ac 100644
--- a/tools/testing/selftests/bpf/progs/test_enable_stats.c
+++ b/tools/testing/selftests/bpf/progs/test_enable_stats.c
@@ -10,7 +10,7 @@ char _license[] SEC("license") = "GPL";
 
 __u64 count = 0;
 
-SEC("raw_tracepoint/sys_enter")
+SEC("raw_tp")
 int test_enable_stats(void *ctx)
 {
 	__sync_fetch_and_add(&count, 1);

---
base-commit: e1d84a37cba984388988d2f1ddc84561413f0db2
change-id: 20261008-selftests-bpf-enable-stats-inflight-edfb36a280f3

Best regards,
-- 
Qiliang Yuan <odys.yuan@gmail.com>

Re: [PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: selftests: use of deprecated CHECK macro instead of ASSERT_*
--

commit 2195ab7af2e709db01f39a1419b8eb0dd3b3c741
Author: Qiliang Yuan <odys.yuan@gmail.com>

selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN
    
This commit changes the test_enable_stats test to run its program with
BPF_PROG_TEST_RUN rather than attaching it to sys_enter. This ensures that
counts are final and avoids race conditions with parallel tests.

Link: https://lore.kernel.org/r/20261008-selftests-bpf-enable-stats-inflight-v1-1-0cb92ee2c6d3@gmail.com

> diff --git a/tools/testing/selftests/bpf/prog_tests/enable_stats.c b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> index 75f85d0fe74a4..691a82a4d6d20 100644
> --- a/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
[ ... ]
> @@ -20,18 +21,23 @@ void test_enable_stats(void)
[ ... ]
> +	do {
> +		err = bpf_prog_test_run_opts(prog_fd, &topts);
> +		if (err)
> +			break;
> +
> +		memset(&info, 0, info_len);
> +		err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
> +	} while (!err && info.run_cnt && !info.run_time_ns);
> +	if (CHECK(err, "run_and_get_prog_info", "err %d\n", err))

[Severity: Low]
Does this newly added error path use the deprecated CHECK() macro? The BPF
subsystem selftest guidelines prefer using modern assertions like ASSERT_OK()
for new code instead of CHECK().

>  		goto cleanup;

[Severity: Low]
This problem wasn't introduced by this patch, but while updating the
test_enable_stats() logic, would it be worth migrating the existing
CHECK(info.run_cnt != skel->bss->count, ...) to a modern assertion like
ASSERT_NEQ() to help reduce technical debt?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-selftests-bpf-enable-stats-inflight-v2-1-691ee379e511@gmail.com?part=1

Re: [PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN

[Alexei Starovoitov]

On Fri, Oct 09, 2026 at 10:04 AM Qiliang Yuan <odys.yuan@gmail.com> wrote:
> them. As a single run can be shorter than the resolution of the clock,
> run it until its run time shows, unless its runs aren't counted at all.

Did v1 fail anywhere?
It passed BPF CI on x86, arm64 and s390.

> +	do {
> +		err = bpf_prog_test_run_opts(prog_fd, &topts);
> +		if (err)
> +			break;
> +
> +		memset(&info, 0, info_len);
> +		err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
> +	} while (!err && info.run_cnt && !info.run_time_ns);

If the kernel counts the runs, but not the time, this loop never ends.
That's the bug this test is for.
check_stats_enabled below can fire only when run_cnt is zero.
The watchdog of test_progs won't help.
It does pthread_kill(SIGSEGV) once. crash_handler() prints
the backtrace and returns. Both bpf() calls don't block,
so there is no EINTR and the loop keeps going.
The test has to fail instead of hang.

Run the prog 1000 times in a for loop and keep get_info as it was in v1.
Then no need for the comment, and check_run_cnt_valid compares
1000 with 1000 instead of 1 with 1.

Also pls don't respin in 12 hours only because a bot said something. Wait for people to reply.

pw-bot: cr