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

[Changbin Du <changbin.du@gmail.com>]

On Wed, Oct 07, 2026 at 02:36:05PM -0700, Namhyung Kim wrote:
> 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.
> 
Yes, just fixed it by using the native word size for the shared counter — 
unsigned long instead of u64.

> 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

-- 
Cheers,
Changbin Du