Re: [RFC PATCH v23 04/10] tools/testing/hpcc: add userspace test of hierarchical per-CPU counters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tools/testing/hpcc: counter leak in test_saturated() and test_random_walk()
--

commit 503d613a781c32158eec55568b3137245d93fe15
Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

tools/testing/hpcc: add userspace test of hierarchical per-CPU counters

This commit adds a single-threaded userspace test harness under
tools/testing/hpcc that builds lib/percpu_counter_tree.c with a simulated
topology to verify counter saturation, random walks, accuracy range
contracts, and comparison functions across all supported CPU counts and
batch sizes.

> diff --git a/tools/testing/hpcc/main.c b/tools/testing/hpcc/main.c
> new file mode 100644
> index 0000000000000..b707754aab0a7
> --- /dev/null
> +++ b/tools/testing/hpcc/main.c
> @@ -0,0 +1,413 @@

[ ... ]

> +static void test_saturated(unsigned long batch_size, long inc)
> +{
> +	struct test_counter c, probe, exact;
> +	unsigned long accuracy;
> +	long approx, delta;
> +
> +	if (test_counter_init(&c, batch_size) || test_counter_init(&probe, batch_size) ||
> +	    test_counter_init(&exact, batch_size)) {
> +		CHECK(0);
> +		return;
> +	}

[ ... ]

> +static void test_random_walk(unsigned long batch_size, unsigned long nr_steps)
> +{
> +	struct test_counter c[2];
> +	unsigned long step;
> +
> +	if (test_counter_init(&c[0], batch_size) || test_counter_init(&c[1], batch_size)) {
> +		CHECK(0);
> +		return;
> +	}

[Severity: Medium]
Can these error paths in test_saturated() and test_random_walk() leak the
already-initialized counters when a subsequent test_counter_init() call
fails?

Each test_counter_init() call allocates per-CPU level0 storage and tree
items via percpu_counter_tree_init(). In test_saturated(), if
test_counter_init(&c, batch_size) succeeds and test_counter_init(&probe,
batch_size) or test_counter_init(&exact, batch_size) fails, the function
records CHECK(0) and returns to run_case() without calling
test_counter_destroy() on c (or probe). Similarly, in test_random_walk(),
if test_counter_init(&c[0], batch_size) succeeds and
test_counter_init(&c[1], batch_size) fails, c[0] is not destroyed before
returning.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102190708.1287100-1-mathieu.desnoyers@efficios.com?part=4