On Tue, Jan 20, 2026 at 02:58:48PM +0800, Feng Jiang wrote:
> Introduce a benchmarking framework to the string_kunit test suite to
> measure the execution efficiency of string functions.
> 
> The implementation is inspired by crc_benchmark(), measuring throughput
> (MB/s) and latency (ns/call) across a range of string lengths. It
> includes a warm-up phase, disables preemption during measurement, and
> uses a fixed seed for reproducible results.
> 
> This allows for comparing different implementations (e.g., generic C vs.
> architecture-optimized assembly) within the KUnit environment.
> 
> Initially, provide benchmarks for strlen().

...

> +#define STRING_BENCH_SEED    888
> +#define STRING_BENCH_WORKLOAD        1000000UL

Can also be (1 * MEGA) from units.h.

...

> +static const size_t bench_lens[] = {
> +     0, 1, 7, 8, 16, 31, 64, 127, 512, 1024, 3173, 4096

Leave trailing comma.

> +};

...

> +static void *alloc_max_bench_buffer(struct kunit *test,
> +             const size_t *lens, size_t count, size_t *buf_len)
> +{
> +     void *buf;
> +     size_t i, max_len = 0;
> +
> +     for (i = 0; i < count; i++) {
> +             if (max_len < lens[i])
> +                     max_len = lens[i];
> +     }
> +
> +     /* Add space for NUL terminator */
> +     max_len += 1;

> +     buf = kunit_kzalloc(test, max_len, GFP_KERNEL);
> +     if (buf && buf_len)
> +             *buf_len = max_len;
> +
> +     return buf;

        if (!buf)
                return NULL;

        *buf_len ...
        return buf;

> +}

...

> +static void fill_random_string(char *buf, size_t len)
> +{
> +     size_t i;
> +     struct rnd_state state;

Reversed xmas tree ordering?

> +     if (!buf || !len)
> +             return;
> +
> +     /* Use a fixed seed to ensure deterministic benchmark results */
> +     prandom_seed_state(&state, 888);
> +     prandom_bytes_state(&state, buf, len);
> +
> +     /* Replace null bytes to avoid early string termination */
> +     for (i = 0; i < len; i++) {
> +             if (buf[i] == '\0')
> +                     buf[i] = 0x01;
> +     }
> +
> +     buf[len - 1] = '\0';
> +}

...

> +#define STRING_BENCH(iters, func, ...)                                       
> \

Is this same / similar code to crc_benchmark()? Perhaps we need to have KUnit
provided macro / environment to perform such tests... Have you talked to KUnit
people about all this?

> +({                                                                   \
> +     u64 __bn_t;                                                     \
> +     size_t __bn_i;                                                  \
> +     size_t __bn_iters = (iters);                                    \
> +     size_t __bn_warm_iters = max_t(size_t, __bn_iters / 10, 50U);   \

Try to avoid max_t() as much as possible. Wouldn't max() suffice?

> +     /* Volatile function pointer prevents dead code elimination */  \
> +     typeof(func) (* volatile __func) = (func);                      \
> +                                                                     \
> +     for (__bn_i = 0; __bn_i < __bn_warm_iters; __bn_i++)            \
> +             (void)__func(__VA_ARGS__);                              \
> +                                                                     \
> +     preempt_disable();                                              \
> +     __bn_t = ktime_get_ns();                                        \
> +     for (__bn_i = 0; __bn_i < __bn_iters; __bn_i++)                 \
> +             (void)__func(__VA_ARGS__);                              \
> +     __bn_t = ktime_get_ns() - __bn_t;                               \
> +     preempt_enable();                                               \
> +     __bn_t;                                                         \
> +})
> +
> +/**
> + * STRING_BENCH_BUF() - Benchmark harness for single-buffer functions.
> + * @test: KUnit context.
> + * @buf_name: Local char * variable name to be defined.
> + * @buf_size: Local size_t variable name to be defined.
> + * @func: Function to benchmark.
> + * @...: Extra arguments for @func.
> + *
> + * Prepares a randomized, null-terminated buffer and iterates through lengths
> + * in bench_lens, defining @buf_name and @buf_size in each loop.
> + */
> +#define STRING_BENCH_BUF(test, buf_name, buf_size, func, ...)                
> \
> +do {                                                                 \
> +     char *buf_name, *_bn_buf;                                       \
> +     size_t buf_size, _bn_i, _bn_iters, _bn_size = 0;                \
> +     u64 _bn_t, _bn_mbps = 0, _bn_lat = 0;                           \
> +                                                                     \
> +     if (!IS_ENABLED(CONFIG_STRING_KUNIT_BENCH))                     \
> +             kunit_skip(test, "not enabled");                        \
> +                                                                     \
> +     _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);                          \

> +     _bn_buf[_bn_size - 1] = '\0';                                   \

You have already this there in the function, no?

> +     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_t(size_t, buf_size, 1U);            \

max()

> +             _bn_t = STRING_BENCH(_bn_iters, func, ##__VA_ARGS__);   \
> +                                                                     \
> +             if (_bn_t > 0) {                                        \
> +                     _bn_mbps = (u64)(buf_size) * _bn_iters * 1000;  \
> +                     _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



Reply via email to