Re: [RFC PATCH v25 06/14] lib: percpu_counter_tree: add lazy tree activation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] lib: percpu_counter_tree_kunit: lazy_race_fn() serializes
  remote updates
--

commit 5130476e081ef221464be8ba2bb48a1905d08dcd
Author: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

lib: percpu_counter_tree: add lazy tree activation

This commit adds lazy tree initialization and activation for hierarchical
per-CPU counters, along with KUnit and userspace tests.

> diff --git a/lib/tests/percpu_counter_tree_kunit.c b/lib/tests/percpu_counter_tree_kunit.c
> index 5774278ea1403..12bd5a4bbb459 100644
> --- a/lib/tests/percpu_counter_tree_kunit.c
> +++ b/lib/tests/percpu_counter_tree_kunit.c
> @@ -796,6 +798,225 @@ static void hpcc_test_saturated_compare(struct kunit *test)

[ ... ]

> +static void lazy_race_fn(struct work_struct *work)
> +{
> +	struct lazy_race_work *w = container_of(work, struct lazy_race_work, work);
> +	struct lazy_race_shared *sh = w->sh;
> +	long sum = 0;
> +	int i;
> +
> +	for (i = 0; i < LAZY_RACE_NR_INC; i++) {
> +		long inc = (long)(get_random_u32() % 65) - 32;
> +
> +		if (i == LAZY_RACE_NR_FIRST) {
> +			/* Fully ordered: the first updates are visible before the pause. */
> +			atomic_long_add_return(sum, &sh->first_sum);
> +			atomic_inc_return(&sh->paused);
> +			/* Pairs with the test's release of go. */
> +			while (!smp_load_acquire(&sh->go))
> +				cond_resched();
> +			if (!w->owner) {
> +				mutex_lock(&sh->activate_lock);
> +				if (percpu_counter_tree_activate_many(sh->c, 1, GFP_KERNEL) ||
> +				    !percpu_counter_tree_active(sh->c))
> +					atomic_inc(&sh->activate_errors);
> +				mutex_unlock(&sh->activate_lock);
> +			}
> +		}
> +		if (w->owner)
> +			percpu_counter_tree_add_owner(sh->c, inc);
> +		else
> +			percpu_counter_tree_add(sh->c, inc);

[Severity: Medium]
Does acquiring sh->activate_lock in every non-owner worker right after
sh->go becomes true prevent percpu_counter_tree_add() from ever racing
with percpu_counter_tree_activate_many()?

Because both the main test thread and all non-owner workers serialize on
sh->activate_lock immediately upon release, the first caller to take the
mutex finishes activating and publishing the tree while the remaining
non-owner workers wait on the mutex.  None of the non-owner workers can
reach percpu_counter_tree_add() until activation is already complete, so
only the owner worker's percpu_counter_tree_add_owner() calls actually
race with tree publication.

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