Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree

[Mathieu Desnoyers]

On 2026-03-14 17:45, Andrew Morton wrote:
> On Fri, 13 Mar 2026 17:54:24 +0000 David Carlier <devnexen@xxxxxxxxx> wrote:

Thanks Andrew for CCing me on this.

> > The compare_delta() helper takes (delta, accuracy_neg, accuracy_pos),
> > but every call site passes (delta, accuracy_pos, accuracy_neg) — the
> > last two arguments are consistently swapped.
> >
> > The documented invariant (include/linux/percpu_counter_tree.h) is:
> >
> >    (precise_sum - under) <= approx_sum <= (precise_sum + over)
> >
> > Which means precise_sum is in [approx_sum - over, approx_sum + under].

Yes, as also documented in the comment above
percpu_counter_tree_approximate_min_max_range().

> > For a positive delta (v - approx_sum >= 0), accuracy_pos must be
> > "under" (the maximum amount precise_sum can exceed approx_sum).
> > For a negative delta, accuracy_neg must be "over".

Right, I mistakenly swapped the logic there. I did not consider that we
have "approximated value" as input and wish to compare precise ranges.

> > Since under > over
> > always (batch_size * M vs (batch_size - 1) * M), swapping them causes
> > false definitive results: the functions return 1 ("v > counter") when
> > the correct answer is 0 (indeterminate).
> >
> > This affects all comparison functions:
> > - percpu_counter_tree_approximate_compare_value()
> > - percpu_counter_tree_approximate_compare()
> > - percpu_counter_tree_precise_compare_value()
> > - percpu_counter_tree_precise_compare()
> >
> > The precise variants are also affected because their approximate
> > fast-path can short-circuit with a wrong result, skipping the precise
> > sum computation.

Yes.

> > Fix by swapping the parameter order in compare_delta() itself, since
> > all call sites are consistently swapped.

That seems like a simple fix for that problem indeed. Did you run the
kunit tests on the fixed code ? Any thought on how to extend the kunit
test to cover this ?

> This affects mm-unstable's "lib: introduce hierarchical per-cpu
> counters", so let's cc Mathieu.

Thanks!

Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>

> > --- a/lib/percpu_counter_tree.c
> > +++ b/lib/percpu_counter_tree.c
> > @@ -458,7 +458,7 @@ long percpu_counter_tree_precise_sum(struct percpu_counter_tree *counter)
> >   EXPORT_SYMBOL_GPL(percpu_counter_tree_precise_sum);
> >   
> >   static
> > -int compare_delta(long delta, unsigned long accuracy_neg, unsigned long accuracy_pos)
> > +int compare_delta(long delta, unsigned long accuracy_pos, unsigned long accuracy_neg)
> >   {
> >   	if (delta >= 0) {
> >   		if (delta <= accuracy_pos)


--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com