Re: [PATCH v5 4/4] selftests/bpf: Adjust wasted entries threshold for ARM64 BRBE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test skip threshold for `total_entries` was not adjusted for the increased ARM64 overhead, leading to guaranteed test failures on hardware with 16 branch entries.
- [Medium] The ARM64 threshold for `wasted_entries` provides zero margin for environmental variations, making the test extremely brittle.
--

commit 08ff160f53fde2f401cb063e6cbc84b6bc9a9970
Author: Puranjay Mohan <puranjay@kernel.org>

selftests/bpf: Adjust wasted entries threshold for ARM64 BRBE

The commit adjusts the wasted entries threshold in the get_branch_snapshot test
for ARM64. It raises the threshold to accommodate the ~13 wasted entries
generated by the ARM64 BPF trampoline compared to ~7 on x86.

> diff --git a/tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c b/tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c
> index 0394a1156d99d..8d1a3480767fe 100644
> --- a/tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c
> +++ b/tools/testing/selftests/bpf/prog_tests/get_branch_snapshot.c
> @@ -116,13 +116,18 @@ void serial_test_get_branch_snapshot(void)
>  
>  	ASSERT_GT(skel->bss->test1_hits, 6, "find_looptest_in_lbr");

[Severity: Medium]
Since this patch increases the expected wasted entries to ~13 on ARM64, does the
skip threshold also need to be updated?

Right above this line, the test skips if total_entries is too small:

    if (skel->bss->total_entries < 16) {
        /* too few entries for the hit/waste test */
        test__skip();
        goto cleanup;
    }

On an ARM64 system with a 16-entry BRBE buffer, the skip condition evaluates
to false, but with 13 wasted entries, only 3 remain for hits. This would cause
the ASSERT_GT check to deterministically fail rather than gracefully skipping.

Could the threshold be adjusted to account for the additional ARM64 overhead
(e.g., needing at least 13 + 7 = 20 entries)?

> -	/* Given we stop LBR in software, we will waste a few entries.
> +	/* Given we stop LBR/BRBE in software, we will waste a few entries.
>  	 * But we should try to waste as few as possible entries. We are at
> -	 * about 7 on x86_64 systems.
> -	 * Add a check for < 10 so that we get heads-up when something
> -	 * changes and wastes too many entries.
> +	 * about 7 on x86_64 and about 13 on arm64 systems (the arm64 BPF
> +	 * trampoline generates more branches than x86_64).
> +	 * Add a check so that we get heads-up when something changes and
> +	 * wastes too many entries.
>  	 */
> +#if defined(__aarch64__)
> +	ASSERT_LT(skel->bss->wasted_entries, 14, "check_wasted_entries");

[Severity: Medium]
Does this threshold provide enough margin for variations in kernel configuration?

The commit message notes there are about 13 wasted entries. Using < 14 means
the test will fail if exactly 14 entries are wasted, leaving no margin for
error. 

Minor variations like KASAN, PREEMPT_RCU, or different compiler code
generation could add a single branch to __bpf_prog_enter_recur and cause this
test to become flaky.

For comparison, the x86 threshold is < 10 for an expected 7 entries, giving
a 2-3 branch margin. Could the ARM64 threshold be relaxed slightly to prevent
spurious failures?

> +#else
>  	ASSERT_LT(skel->bss->wasted_entries, 10, "check_wasted_entries");
> +#endif
>  
>  cleanup:
>  	get_branch_snapshot__destroy(skel);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616155716.2631508-1-puranjay@kernel.org?part=4