Re: [PATCH v2] perf bench: Add atomic operations benchmark

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] perf: 64-bit atomic builtins on u64 cause link errors on 32-bit
- [Low] perf: Missing explicit inclusion of <stdint.h> for UINT64_MAX
--

commit 1f5903d48b4529edce94275c5756879218402332
Author: Changbin Du <changbin.du@intel.com>

perf bench: Add atomic operations benchmark

Add a new atomic collection to perf bench for benchmarking atomic
read-modify-write operations with multi-threaded contention testing.

> diff --git a/tools/perf/bench/atomic.c b/tools/perf/bench/atomic.c
> new file mode 100644
> index 0000000000000..08497910b563f
> --- /dev/null
> +++ b/tools/perf/bench/atomic.c
[ ... ]
> +static void *worker(void *arg)
> +{
> +	struct worker_stats *ws = (struct worker_stats *)arg;
> +	struct timespec tstart, tend;
> +	u64 i;
> +
> +	pthread_barrier_wait(&start_barrier);
> +
> +	clock_gettime(CLOCK_MONOTONIC, &tstart);
> +	switch (bench_op) {
> +	case OP_CAS:
> +		for (i = 0; i < iterations; i++) {
> +			u64 old_val, new_val;
> +
> +			old_val = __atomic_load_n(&shared_counter, __ATOMIC_RELAXED);

[Severity: High]
Does the use of 64-bit atomic builtins on a u64 variable in worker() in
tools/perf/bench/atomic.c break compilation on 32-bit architectures?

Without 32-bit fallback definitions or omitting 64-bit atomics entirely,
calls like __atomic_load_n, __atomic_compare_exchange_n, and
__atomic_fetch_add on the shared_counter might generate unresolved
__atomic_*_8 library calls on 32-bit systems.

> +			do {
> +				new_val = old_val + 1;
> +			} while (!__atomic_compare_exchange_n(&shared_counter,
> +							      &old_val, new_val,
> +							      false,
> +							      __ATOMIC_RELAXED,
> +							      __ATOMIC_RELAXED));
> +		}
> +		break;
> +	case OP_INC:
> +		for (i = 0; i < iterations; i++)
> +			__atomic_fetch_add(&shared_counter, 1, __ATOMIC_RELAXED);
> +		break;
> +	default:
> +		break;
> +	}
[ ... ]
> +static void print_default_format(struct worker_stats *wstats,
> +				 struct stats *time_stats)
> +{
[ ... ]
> +	/*
> +	 * Per-thread statistics from the last measured repeat show
> +	 * fairness / imbalance in CPU scheduling.
> +	 */
> +	if (threads > 1) {
> +		u64 min_ns = UINT64_MAX, max_ns = 0, total_ns = 0;

[Severity: Low]
Does the usage of UINT64_MAX in print_default_format() in
tools/perf/bench/atomic.c require the explicit inclusion of <stdint.h>?

This might violate strict musl compatibility header guidelines, which require
explicitly including <stdint.h> for this constant.

> +		double min_thru, max_thru, avg_thru;
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007053027.1738402-1-changbin.du@gmail.com?part=1