Re: [PATCH v1 08/12] perf tests: Fix flakiness in BPF counters test on hybrid systems

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The script incorrectly uses `nproc` to pick a random CPU ID, which causes `taskset` to fail unconditionally on systems with restricted cpusets.
- [Medium] The percentage calculation using `expr $first_num \* 3 / 20` is vulnerable to a 32-bit signed integer overflow, causing false test failures on minimalist environments.
--

commit 3c1a23e10cc0581a3aebcf4bbcfca207d9638689
Author: Ian Rogers <irogers@google.com>
Subject: perf tests: Fix flakiness in BPF counters test on hybrid systems

This commit fixes flakiness in the BPF counters test by switching to a
deterministic `awk` workload pinned to a random CPU and restricted to
user-space instructions. It also tightens the comparison tolerance.

> diff --git a/tools/perf/tests/shell/stat_bpf_counters.sh b/tools/perf/tests/shell/stat_bpf_counters.sh
> index 35463358b273c..2f01608c95e37 100755
> --- a/tools/perf/tests/shell/stat_bpf_counters.sh
> +++ b/tools/perf/tests/shell/stat_bpf_counters.sh
> @@ -4,21 +4,25 @@
>  
>  set -e
>  
> -workload="perf test -w sqrtloop"
> +CPU=0
> +if command -v shuf >/dev/null 2>&1; then
> +	CPU=$(shuf -i 0-$(($(nproc) - 1)) -n 1 2>/dev/null || echo 0)
> +fi
> +workload=(taskset -c "$CPU" awk 'BEGIN { for (i=0; i<10000000; i++) sum+=i }')

[Severity: Medium]
On systems where the process is restricted to a subset of CPUs (e.g., CPUs 4
and 5 via cpusets), `nproc` will return 2. Will this select a CPU ID of 0 or 1,
causing `taskset -c "$CPU"` to fail unconditionally with an 'Invalid argument'
error?

If taskset fails immediately, it seems `perf stat` will only measure the brief
execution of the failed taskset command instead of the intended workload.

>  
> -# check whether $2 is within +/- 20% of $1
> +# check whether $2 is within +/- 15% of $1
>  compare_number()
>  {
>  	first_num=$1
>  	second_num=$2
>  
> -	# upper bound is first_num * 120%
> -	upper=$(expr $first_num + $first_num / 5 )
> -	# lower bound is first_num * 80%
> -	lower=$(expr $first_num - $first_num / 5 )
> +	# upper bound is first_num * 115%
> +	upper=$(expr $first_num + $first_num \* 3 / 20 )
> +	# lower bound is first_num * 85%
> +	lower=$(expr $first_num - $first_num \* 3 / 20 )

[Severity: Medium]
Will this percentage calculation cause an integer overflow on systems with
32-bit `expr` implementations (such as some BusyBox configurations)?

The newly introduced `awk` workload runs for 10 million iterations, executing
roughly 1.5 to 3 billion instructions. Since multiplication is performed before
division, `$first_num \* 3` will produce a value over 4.5 billion, overflowing
the 32-bit signed `INT_MAX` (2,147,483,647) and resulting in a negative bound.

Prior to this patch, division occurred first (`first_num / 5`), avoiding this
intermediate overflow.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616012744.4049193-1-irogers@google.com?part=8