Re: [PATCH v5 4/8] lib/string_kunit: add performance benchmark for strlen()

From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Feng Jiang <jiangfeng@kylinos.cn>
Cc: pjw@kernel.org, palmer@dabbelt.com, aou@eecs.berkeley.edu,
	alex@ghiti.fr, akpm@linux-foundation.org, kees@kernel.org,
	andy@kernel.org, ebiggers@kernel.org, martin.petersen@oracle.com,
	sohil.mehta@intel.com, charlie@rivosinc.com,
	conor.dooley@microchip.com, samuel.holland@sifive.com,
	linus.walleij@linaro.org, nathan@kernel.org,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH v5 4/8] lib/string_kunit: add performance benchmark for strlen()
Date: Tue, 27 Jan 2026 11:50:50 +0200	[thread overview]
Message-ID: <aXiKelsJF-SiqnVS@smile.fi.intel.com> (raw)
In-Reply-To: <dcdb82e8-691a-4cc1-88d5-755039ed0313@kylinos.cn>

On Tue, Jan 27, 2026 at 05:33:10PM +0800, Feng Jiang wrote:
> On 2026/1/27 16:57, Andy Shevchenko wrote:
> > On Tue, Jan 27, 2026 at 09:25:54AM +0800, Feng Jiang wrote:

...

> >> +#define STRING_BENCH_BUF(test, buf_name, buf_size, func, ...)		\
> >> +do {									\
> >> +	size_t buf_size, _bn_i, _bn_iters, _bn_size = 0;		\
> >> +	u64 _bn_t, _bn_mbps = 0, _bn_lat = 0;				\
> >> +	char *buf_name, *_bn_buf;					\
> >> +									\
> >> +	_bn_buf = alloc_max_bench_buffer(test, bench_lens,		\
> >> +			ARRAY_SIZE(bench_lens), &_bn_size);		\
> >> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, _bn_buf);			\
> >> +									\
> >> +	fill_random_string(_bn_buf, _bn_size);				\
> >> +									\
> >> +	for (_bn_i = 0; _bn_i < ARRAY_SIZE(bench_lens); _bn_i++) {	\
> >> +		buf_size = bench_lens[_bn_i];				\
> >> +		buf_name = _bn_buf + _bn_size - buf_size - 1;		\
> >> +		_bn_iters = STRING_BENCH_WORKLOAD / max(buf_size, 1U);	\
> >> +									\
> >> +		_bn_t = STRING_BENCH(_bn_iters, func, ##__VA_ARGS__);	\
> > 
> >> +									\
> > 
> > Remove unneeded blank line.
> 
> Will fix.
> 
> >> +		if (_bn_t > 0) {					\
> >> +			_bn_mbps = (u64)(buf_size) * _bn_iters		\
> > 
> > Why buf_size in the parentheses here and not anywhere else (above)?
> 
> It was a bit inconsistent. I'll remove the unneeded parentheses for buf_size.
> 
> > I assume it's just an external temporary variable? But why do we need to have
> > it in the parameters to the macro?
> 
> This is necessary because buf_size often needs to be passed as an argument
> to the function under test. For instance, when benchmarking strnlen, the
> caller must pass the current length as an argument:
> STRING_BENCH_BUF(test, buf, len, strnlen, buf, len);

Okay, and why is it needed in this macro? It get overridden immediately in
the loop. Assuming that the array size of bench lengths is not zero, this
has no visible use. Can you elaborate?

> >> +					* (NSEC_PER_SEC / MEGA);	\
> > > Leave '*' on the previous line.
> 
> Will fix.
> 
> >> +			_bn_mbps = div64_u64(_bn_mbps, _bn_t);		\
> >> +			_bn_lat = div64_u64(_bn_t, _bn_iters);		\
> >> +		}							\
> >> +		kunit_info(test, "len=%zu: %llu MB/s (%llu ns/call)\n",	\
> >> +				buf_size, _bn_mbps, _bn_lat);		\
> >> +	}								\
> >> +} while (0)

-- 
With Best Regards,
Andy Shevchenko

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv