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

[Namhyung Kim <namhyung@kernel.org>]

On Wed, Oct 07, 2026 at 05:39:22AM +0000, sashiko-bot@kernel.org wrote:
> 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.

Probably it's ok to go with unsigned long.

Thanks,
Namhyung

> 
> > +			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