* [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
@ 2026-03-13 17:54 David Carlier
2026-03-14 15:30 ` Josh Law
2026-03-14 21:45 ` Andrew Morton
0 siblings, 2 replies; 15+ messages in thread
From: David Carlier @ 2026-03-13 17:54 UTC (permalink / raw)
To: Andrew Morton, Josh Law, Dennis Zhou; +Cc: linux-kernel, David Carlier
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].
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". 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.
Fix by swapping the parameter order in compare_delta() itself, since
all call sites are consistently swapped.
Signed-off-by: David Carlier <devnexen@gmail.com>
---
lib/percpu_counter_tree.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/percpu_counter_tree.c b/lib/percpu_counter_tree.c
index 5c8fc2dcdc16..beb1144e6450 100644
--- 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)
--
2.51.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-13 17:54 [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree David Carlier
@ 2026-03-14 15:30 ` Josh Law
2026-03-14 21:45 ` Andrew Morton
1 sibling, 0 replies; 15+ messages in thread
From: Josh Law @ 2026-03-14 15:30 UTC (permalink / raw)
To: David Carlier, Andrew Morton, Dennis Zhou; +Cc: linux-kernel
On 13 March 2026 17:54:24 GMT, David Carlier <devnexen@gmail.com> wrote:
>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].
>
>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". 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.
>
>Fix by swapping the parameter order in compare_delta() itself, since
>all call sites are consistently swapped.
>
>Signed-off-by: David Carlier <devnexen@gmail.com>
>---
> lib/percpu_counter_tree.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/percpu_counter_tree.c b/lib/percpu_counter_tree.c
>index 5c8fc2dcdc16..beb1144e6450 100644
>--- 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)
Good patch.
Reviewed-by: Josh Law <objecting@objecting.org>
V/R
Josh Law
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-13 17:54 [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree David Carlier
2026-03-14 15:30 ` Josh Law
@ 2026-03-14 21:45 ` Andrew Morton
2026-03-15 22:00 ` Mathieu Desnoyers
1 sibling, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2026-03-14 21:45 UTC (permalink / raw)
To: David Carlier; +Cc: Josh Law, Dennis Zhou, linux-kernel, Mathieu Desnoyers
On Fri, 13 Mar 2026 17:54:24 +0000 David Carlier <devnexen@gmail.com> wrote:
> 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].
>
> 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". 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.
>
> Fix by swapping the parameter order in compare_delta() itself, since
> all call sites are consistently swapped.
This affects mm-unstable's "lib: introduce hierarchical per-cpu
counters", so let's cc Mathieu.
> --- 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)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-14 21:45 ` Andrew Morton
@ 2026-03-15 22:00 ` Mathieu Desnoyers
2026-03-15 22:47 ` David CARLIER
0 siblings, 1 reply; 15+ messages in thread
From: Mathieu Desnoyers @ 2026-03-15 22:00 UTC (permalink / raw)
To: Andrew Morton, David Carlier; +Cc: Josh Law, Dennis Zhou, linux-kernel
On 2026-03-14 17:45, Andrew Morton wrote:
> On Fri, 13 Mar 2026 17:54:24 +0000 David Carlier <devnexen@gmail.com> 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@efficios.com>
>
>> --- 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-15 22:00 ` Mathieu Desnoyers
@ 2026-03-15 22:47 ` David CARLIER
2026-03-15 23:16 ` Mathieu Desnoyers
0 siblings, 1 reply; 15+ messages in thread
From: David CARLIER @ 2026-03-15 22:47 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
On Sun, 15 Mar 2026 at 22:00, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> On 2026-03-14 17:45, Andrew Morton wrote:
> > On Fri, 13 Mar 2026 17:54:24 +0000 David Carlier <devnexen@gmail.com> 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 ?
Yes.
Any thought on how to extend the kunit
> test to cover this ?
Can something like this be applicable ?
static void hpcc_test_compare_value_boundaries(struct kunit *test)
{
struct percpu_counter_tree pct;
struct percpu_counter_tree_level_item *counter_items;
unsigned long under = 0, over = 0;
int ret;
counter_items = kzalloc(percpu_counter_tree_items_size(), GFP_KERNEL);
KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
ret = percpu_counter_tree_init(&pct, counter_items, 32, GFP_KERNEL);
KUNIT_ASSERT_EQ(test, ret, 0);
percpu_counter_tree_set(&pct, 0);
percpu_counter_tree_approximate_accuracy_range(&pct, &under, &over);
/* Skip if under == over (no gap to test) */
if (under == over)
goto out;
/* Positive boundary: v = under should be indeterminate (precise
could be up to +under) */
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct, (long)under));
/* Positive boundary: v = over + 1 should be indeterminate */
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct,
(long)(over + 1)));
/* Positive boundary: v = under + 1 should be definitively greater */
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_approximate_compare_value(&pct,
(long)(under + 1)));
/* Negative boundary: -(over + 1) should be definitively less */
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare_value(&pct,
-(long)(over + 1)));
/* Negative boundary: -under should be indeterminate */
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct, -(long)under));
/* Negative boundary: -(under + 1) should be definitively less */
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare_value(&pct,
-(long)(under + 1)));
out:
percpu_counter_tree_destroy(&pct);
kfree(counter_items);
}
>
> >
> > This affects mm-unstable's "lib: introduce hierarchical per-cpu
> > counters", so let's cc Mathieu.
>
> Thanks!
>
> Reviewed-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>
> >
> >> --- 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-15 22:47 ` David CARLIER
@ 2026-03-15 23:16 ` Mathieu Desnoyers
2026-03-16 0:05 ` David CARLIER
0 siblings, 1 reply; 15+ messages in thread
From: Mathieu Desnoyers @ 2026-03-15 23:16 UTC (permalink / raw)
To: David CARLIER; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
On 2026-03-15 18:47, David CARLIER wrote:
> On Sun, 15 Mar 2026 at 22:00, Mathieu Desnoyers
[...]
> Any thought on how to extend the kunit
>> test to cover this ?
>
> Can something like this be applicable ?
>
> static void hpcc_test_compare_value_boundaries(struct kunit *test)
> {
> struct percpu_counter_tree pct;
> struct percpu_counter_tree_level_item *counter_items;
> unsigned long under = 0, over = 0;
> int ret;
>
> counter_items = kzalloc(percpu_counter_tree_items_size(), GFP_KERNEL);
> KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
> ret = percpu_counter_tree_init(&pct, counter_items, 32, GFP_KERNEL);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> percpu_counter_tree_set(&pct, 0);
> percpu_counter_tree_approximate_accuracy_range(&pct, &under, &over);
> > /* Skip if under == over (no gap to test) */
> if (under == over)
> goto out;
If we ever have over == under, then I think we could keep running this test,
but skip the "expect indeterminate" test items which depend on having
asymmetric ranges.
>
> /* Positive boundary: v = under should be indeterminate (precise
> could be up to +under) */
> KUNIT_EXPECT_EQ(test, 0,
> percpu_counter_tree_approximate_compare_value(&pct, (long)under));
>
> /* Positive boundary: v = over + 1 should be indeterminate */
> KUNIT_EXPECT_EQ(test, 0,
> percpu_counter_tree_approximate_compare_value(&pct,
> (long)(over + 1)));
This assumes that under > over, right ? In that case, I think we may
want to grab "under" and "over" values and copy them into a "a" and
"b" variables where a > b.
This way, the test does not cast on stone the library implementation
details: the fact that the "under" range is slightly larger than "over"
due to complement 2 +/- ranges.
Thanks!
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-15 23:16 ` Mathieu Desnoyers
@ 2026-03-16 0:05 ` David CARLIER
2026-03-16 0:41 ` Mathieu Desnoyers
0 siblings, 1 reply; 15+ messages in thread
From: David CARLIER @ 2026-03-16 0:05 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
Ok what about this revised version ?
static void hpcc_test_compare_value_boundaries(struct kunit *test)
{
struct percpu_counter_tree pct;
struct percpu_counter_tree_level_item *counter_items;
unsigned long under = 0, over = 0;
int ret;
counter_items = kzalloc(percpu_counter_tree_items_size(), GFP_KERNEL);
KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
ret = percpu_counter_tree_init(&pct, counter_items, 32, GFP_KERNEL);
KUNIT_ASSERT_EQ(test, ret, 0);
percpu_counter_tree_set(&pct, 0);
percpu_counter_tree_approximate_accuracy_range(&pct, &under, &over);
/*
* With approx_sum = precise_sum = 0, from the accuracy invariant:
* approx_sum - over <= precise_sum <= approx_sum + under
* Positive deltas use 'under' as tolerance, negative use 'over'.
*/
/* At boundary: indeterminate */
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct,
(long)under));
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct,
-(long)over));
/* Beyond boundary: definitive */
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_approximate_compare_value(&pct,
(long)(under + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare_value(&pct,
-(long)(over + 1)));
/*
* When ranges are asymmetric, test values in the gap between the
* smaller and larger range to catch swapped accuracy parameters.
* Determine which range is larger at runtime to avoid assuming a
* specific relationship between under and over.
*/
if (under != over) {
unsigned long a = max(under, over);
unsigned long b = min(under, over);
/*
* b + 1 is beyond the smaller range but within the larger.
* The side with the larger tolerance must return indeterminate,
* the side with the smaller tolerance must return definitive.
*/
if (a == under) {
/* Positive side has larger tolerance */
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(
&pct, (long)(b + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare_value(
&pct, -(long)(b + 1)));
} else {
/* Negative side has larger tolerance */
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_approximate_compare_value(
&pct, (long)(b + 1)));
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(
&pct, -(long)(b + 1)));
}
}
percpu_counter_tree_destroy(&pct);
kfree(counter_items);
}
Mathieu Desnoyers
23:16 (43 minutes ago)
to me, Andrew, Josh, Dennis, linux-kernel
On 2026-03-15 18:47, David CARLIER wrote:
> On Sun, 15 Mar 2026 at 22:00, Mathieu Desnoyers
[...]
> Any thought on how to extend the kunit
>> test to cover this ?
>
> Can something like this be applicable ?
>
> static void hpcc_test_compare_value_boundaries(struct kunit *test)
> {
> struct percpu_counter_tree pct;
> struct percpu_counter_tree_level_item *counter_items;
> unsigned long under = 0, over = 0;
> int ret;
>
> counter_items = kzalloc(percpu_counter_tree_items_size(), GFP_KERNEL);
> KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
> ret = percpu_counter_tree_init(&pct, counter_items, 32, GFP_KERNEL);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> percpu_counter_tree_set(&pct, 0);
> percpu_counter_tree_approximate_accuracy_range(&pct, &under, &over);
> > /* Skip if under == over (no gap to test) */
> if (under == over)
> goto out;
If we ever have over == under, then I think we could keep running this test,
but skip the "expect indeterminate" test items which depend on having
asymmetric ranges.
>
> /* Positive boundary: v = under should be indeterminate (precise
> could be up to +under) */
> KUNIT_EXPECT_EQ(test, 0,
> percpu_counter_tree_approximate_compare_value(&pct, (long)under));
>
> /* Positive boundary: v = over + 1 should be indeterminate */
> KUNIT_EXPECT_EQ(test, 0,
> percpu_counter_tree_approximate_compare_value(&pct,
> (long)(over + 1)));
This assumes that under > over, right ? In that case, I think we may
want to grab "under" and "over" values and copy them into a "a" and
"b" variables where a > b.
This way, the test does not cast on stone the library implementation
details: the fact that the "under" range is slightly larger than "over"
due to complement 2 +/- ranges.
Thanks!
On Sun, 15 Mar 2026 at 23:16, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
goto out;
>
> If we ever have over == under, then I think we could keep running this test,
> but skip the "expect indeterminate" test items which depend on having
> asymmetric ranges.
>
> >
> > /* Positive boundary: v = under should be indeterminate (precise
> > could be up to +under) */
> > KUNIT_EXPECT_EQ(test, 0,
> > percpu_counter_tree_approximate_compare_value(&pct, (long)under));
> >
> > /* Positive boundary: v = over + 1 should be indeterminate */
> > KUNIT_EXPECT_EQ(test, 0,
> > percpu_counter_tree_approximate_compare_value(&pct,
> > (long)(over + 1)));
>
> This assumes that under > over, right ? In that case, I think we may
> want to grab "under" and "over" values and copy them into a "a" and
> "b" variables where a > b.
>
> This way, the test does not cast on stone the library implementation
> details: the fact that the "under" range is slightly larger than "over"
> due to complement 2 +/- ranges.
>
> Thanks!
>
> Mathieu
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-16 0:05 ` David CARLIER
@ 2026-03-16 0:41 ` Mathieu Desnoyers
2026-03-16 4:28 ` David CARLIER
0 siblings, 1 reply; 15+ messages in thread
From: Mathieu Desnoyers @ 2026-03-16 0:41 UTC (permalink / raw)
To: David CARLIER; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
On 2026-03-15 20:05, David CARLIER wrote:
> Ok what about this revised version ?
>
> static void hpcc_test_compare_value_boundaries(struct kunit *test)
> {
> struct percpu_counter_tree pct;
> struct percpu_counter_tree_level_item *counter_items;
> unsigned long under = 0, over = 0;
> int ret;
>
> counter_items = kzalloc(percpu_counter_tree_items_size(), GFP_KERNEL);
> KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
> ret = percpu_counter_tree_init(&pct, counter_items, 32, GFP_KERNEL);
> KUNIT_ASSERT_EQ(test, ret, 0);
>
> percpu_counter_tree_set(&pct, 0);
> percpu_counter_tree_approximate_accuracy_range(&pct, &under, &over);
>
> /*
> * With approx_sum = precise_sum = 0, from the accuracy invariant:
> * approx_sum - over <= precise_sum <= approx_sum + under
> * Positive deltas use 'under' as tolerance, negative use 'over'.
> */
>
> /* At boundary: indeterminate */
> KUNIT_EXPECT_EQ(test, 0,
> percpu_counter_tree_approximate_compare_value(&pct,
> (long)under));
> KUNIT_EXPECT_EQ(test, 0,
> percpu_counter_tree_approximate_compare_value(&pct,
> -(long)over));
>
> /* Beyond boundary: definitive */
> KUNIT_EXPECT_EQ(test, 1,
> percpu_counter_tree_approximate_compare_value(&pct,
> (long)(under + 1)));
> KUNIT_EXPECT_EQ(test, -1,
> percpu_counter_tree_approximate_compare_value(&pct,
> -(long)(over + 1)));
>
> /*
> * When ranges are asymmetric, test values in the gap between the
> * smaller and larger range to catch swapped accuracy parameters.
> * Determine which range is larger at runtime to avoid assuming a
> * specific relationship between under and over.
> */
> if (under != over) {
> unsigned long a = max(under, over);
> unsigned long b = min(under, over);
Looking at the resulting code, I wonder if the a/b variables are
useful after all.
>
> /*
> * b + 1 is beyond the smaller range but within the larger.
> * The side with the larger tolerance must return indeterminate,
> * the side with the smaller tolerance must return definitive.
> */
> if (a == under) {
This can become if (under > over) {
> /* Positive side has larger tolerance */
> KUNIT_EXPECT_EQ(test, 0,
> percpu_counter_tree_approximate_compare_value(
> &pct, (long)(b + 1)));
"b" becomes "under"
> KUNIT_EXPECT_EQ(test, -1,
> percpu_counter_tree_approximate_compare_value(
> &pct, -(long)(b + 1)));
same.
> } else {
> /* Negative side has larger tolerance */
> KUNIT_EXPECT_EQ(test, 1,
> percpu_counter_tree_approximate_compare_value(
> &pct, (long)(b + 1)));
"b" becomes "over".
> KUNIT_EXPECT_EQ(test, 0,
> percpu_counter_tree_approximate_compare_value(
> &pct, -(long)(b + 1)));
same.
Thanks,
Mathieu
> }
> }
>
> percpu_counter_tree_destroy(&pct);
> kfree(counter_items);
> }
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-16 0:41 ` Mathieu Desnoyers
@ 2026-03-16 4:28 ` David CARLIER
2026-03-16 13:06 ` Mathieu Desnoyers
0 siblings, 1 reply; 15+ messages in thread
From: David CARLIER @ 2026-03-16 4:28 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
You're right. Here's the simplified version:
static void hpcc_test_compare_value_boundaries(struct kunit *test)
{
struct percpu_counter_tree pct;
struct percpu_counter_tree_level_item *counter_items;
unsigned long under = 0, over = 0;
int ret;
counter_items = kzalloc(percpu_counter_tree_items_size(), GFP_KERNEL);
KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
ret = percpu_counter_tree_init(&pct, counter_items, 32, GFP_KERNEL);
KUNIT_ASSERT_EQ(test, ret, 0);
percpu_counter_tree_set(&pct, 0);
percpu_counter_tree_approximate_accuracy_range(&pct, &under, &over);
/*
* With approx_sum = precise_sum = 0, from the accuracy invariant:
* approx_sum - over <= precise_sum <= approx_sum + under
* Positive deltas use 'under' as tolerance, negative use 'over'.
*/
/* At boundary: indeterminate */
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct,
(long)under));
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct,
-(long)over));
/* Beyond boundary: definitive */
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_approximate_compare_value(&pct,
(long)(under + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare_value(&pct,
-(long)(over + 1)));
/*
* When ranges are asymmetric, test a value just past the smaller
* range but within the larger. The side with the larger tolerance
* must return indeterminate, the other must return definitive.
* This catches any swap of the accuracy parameters.
*/
if (under != over) {
if (under > over) {
/* Positive side has larger tolerance */
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(
&pct, (long)(over + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare_value(
&pct, -(long)(over + 1)));
} else {
/* Negative side has larger tolerance */
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_approximate_compare_value(
&pct, (long)(under + 1)));
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(
&pct, -(long)(under + 1)));
}
}
percpu_counter_tree_destroy(&pct);
kfree(counter_items);
}
Thanks,
David
On Mon, 16 Mar 2026 at 00:41, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> On 2026-03-15 20:05, David CARLIER wrote:
> > Ok what about this revised version ?
> >
> > static void hpcc_test_compare_value_boundaries(struct kunit *test)
> > {
> > struct percpu_counter_tree pct;
> > struct percpu_counter_tree_level_item *counter_items;
> > unsigned long under = 0, over = 0;
> > int ret;
> >
> > counter_items = kzalloc(percpu_counter_tree_items_size(), GFP_KERNEL);
> > KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
> > ret = percpu_counter_tree_init(&pct, counter_items, 32, GFP_KERNEL);
> > KUNIT_ASSERT_EQ(test, ret, 0);
> >
> > percpu_counter_tree_set(&pct, 0);
> > percpu_counter_tree_approximate_accuracy_range(&pct, &under, &over);
> >
> > /*
> > * With approx_sum = precise_sum = 0, from the accuracy invariant:
> > * approx_sum - over <= precise_sum <= approx_sum + under
> > * Positive deltas use 'under' as tolerance, negative use 'over'.
> > */
> >
> > /* At boundary: indeterminate */
> > KUNIT_EXPECT_EQ(test, 0,
> > percpu_counter_tree_approximate_compare_value(&pct,
> > (long)under));
> > KUNIT_EXPECT_EQ(test, 0,
> > percpu_counter_tree_approximate_compare_value(&pct,
> > -(long)over));
> >
> > /* Beyond boundary: definitive */
> > KUNIT_EXPECT_EQ(test, 1,
> > percpu_counter_tree_approximate_compare_value(&pct,
> > (long)(under + 1)));
> > KUNIT_EXPECT_EQ(test, -1,
> > percpu_counter_tree_approximate_compare_value(&pct,
> > -(long)(over + 1)));
> >
> > /*
> > * When ranges are asymmetric, test values in the gap between the
> > * smaller and larger range to catch swapped accuracy parameters.
> > * Determine which range is larger at runtime to avoid assuming a
> > * specific relationship between under and over.
> > */
> > if (under != over) {
> > unsigned long a = max(under, over);
> > unsigned long b = min(under, over);
>
> Looking at the resulting code, I wonder if the a/b variables are
> useful after all.
>
> >
> > /*
> > * b + 1 is beyond the smaller range but within the larger.
> > * The side with the larger tolerance must return indeterminate,
> > * the side with the smaller tolerance must return definitive.
> > */
> > if (a == under) {
>
> This can become if (under > over) {
>
> > /* Positive side has larger tolerance */
> > KUNIT_EXPECT_EQ(test, 0,
> > percpu_counter_tree_approximate_compare_value(
> > &pct, (long)(b + 1)));
>
> "b" becomes "under"
>
> > KUNIT_EXPECT_EQ(test, -1,
> > percpu_counter_tree_approximate_compare_value(
> > &pct, -(long)(b + 1)));
>
> same.
>
> > } else {
> > /* Negative side has larger tolerance */
> > KUNIT_EXPECT_EQ(test, 1,
> > percpu_counter_tree_approximate_compare_value(
> > &pct, (long)(b + 1)));
>
> "b" becomes "over".
>
> > KUNIT_EXPECT_EQ(test, 0,
> > percpu_counter_tree_approximate_compare_value(
> > &pct, -(long)(b + 1)));
>
> same.
>
> Thanks,
>
> Mathieu
>
> > }
> > }
> >
> > percpu_counter_tree_destroy(&pct);
> > kfree(counter_items);
> > }
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-16 4:28 ` David CARLIER
@ 2026-03-16 13:06 ` Mathieu Desnoyers
2026-03-16 13:41 ` David CARLIER
0 siblings, 1 reply; 15+ messages in thread
From: Mathieu Desnoyers @ 2026-03-16 13:06 UTC (permalink / raw)
To: David CARLIER; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
On 2026-03-16 00:28, David CARLIER wrote:
> You're right. Here's the simplified version:
>
Much better. So this covers percpu_counter_tree_approximate_compare_value.
Can we also add coverage for the following affected APIs ?
- percpu_counter_tree_precise_compare_value
- percpu_counter_tree_precise_compare
- percpu_counter_tree_approximate_compare
When comparing between two sets of counters, we end up summing the
the accuracy ranges of the two counters. So in case we ever have an
issue there, it would be good to add coverage of the range limit cases
in the same way.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-16 13:06 ` Mathieu Desnoyers
@ 2026-03-16 13:41 ` David CARLIER
2026-03-16 13:53 ` Mathieu Desnoyers
0 siblings, 1 reply; 15+ messages in thread
From: David CARLIER @ 2026-03-16 13:41 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
Sure. For precise_compare_value, the boundary tests extend
naturally — the function must return the exact result regardless
of which code path (fast approximate or precise fallback) is taken.
For the two-counter APIs, I noticed that the combined accuracy
is always symmetric: over_a + under_b = under_a + over_b =
(bs_a + bs_b - 1) * multiplier, so the asymmetric gap test
doesn't apply there. But boundary tests at the combined accuracy
limit still provide useful coverage.
Here's the extended compare_value test, plus a new test for
two-counter comparisons:
static void hpcc_test_compare_value_boundaries(struct kunit *test)
{
struct percpu_counter_tree pct;
struct percpu_counter_tree_level_item *counter_items;
unsigned long under = 0, over = 0;
int ret;
counter_items = kzalloc(percpu_counter_tree_items_size(), GFP_KERNEL);
KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
ret = percpu_counter_tree_init(&pct, counter_items, 32, GFP_KERNEL);
KUNIT_ASSERT_EQ(test, ret, 0);
percpu_counter_tree_set(&pct, 0);
percpu_counter_tree_approximate_accuracy_range(&pct, &under, &over);
/*
* With approx_sum = precise_sum = 0, from the accuracy invariant:
* approx_sum - over <= precise_sum <= approx_sum + under
* Positive deltas use 'under' as tolerance, negative use 'over'.
*/
/* --- percpu_counter_tree_approximate_compare_value --- */
/* At boundary: indeterminate */
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct,
(long)under));
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(&pct,
-(long)over));
/* Beyond boundary: definitive */
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_approximate_compare_value(&pct,
(long)(under + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare_value(&pct,
-(long)(over + 1)));
/* Asymmetric gap: catches swapped accuracy parameters */
if (under != over) {
if (under > over) {
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(
&pct, (long)(over + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare_value(
&pct, -(long)(over + 1)));
} else {
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_approximate_compare_value(
&pct, (long)(under + 1)));
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare_value(
&pct, -(long)(under + 1)));
}
}
/* --- percpu_counter_tree_precise_compare_value --- */
/*
* With approx_sum = precise_sum = 0, the precise comparison
* must return the exact result. At boundary values the
* approximate fast-path returns indeterminate, exercising
* the precise sum fallback.
*/
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_precise_compare_value(&pct, 0));
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_precise_compare_value(&pct,
(long)under));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_precise_compare_value(&pct,
-(long)over));
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_precise_compare_value(&pct,
(long)(under + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_precise_compare_value(&pct,
-(long)(over + 1)));
if (under != over) {
if (under > over) {
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_precise_compare_value(
&pct, (long)(over + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_precise_compare_value(
&pct, -(long)(over + 1)));
} else {
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_precise_compare_value(
&pct, (long)(under + 1)));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_precise_compare_value(
&pct, -(long)(under + 1)));
}
}
percpu_counter_tree_destroy(&pct);
kfree(counter_items);
}
static void hpcc_test_compare_counter_boundaries(struct kunit *test)
{
struct percpu_counter_tree pct[2];
struct percpu_counter_tree_level_item *counter_items;
unsigned long under = 0, over = 0;
unsigned long combined;
int ret;
counter_items = kzalloc(percpu_counter_tree_items_size() * 2,
GFP_KERNEL);
KUNIT_ASSERT_PTR_NE(test, counter_items, NULL);
ret = percpu_counter_tree_init_many(pct, counter_items, 2, 32,
GFP_KERNEL);
KUNIT_ASSERT_EQ(test, ret, 0);
percpu_counter_tree_approximate_accuracy_range(&pct[0],
&under, &over);
/*
* Both counters have the same configuration. The combined
* accuracy for two-counter comparison is:
* accuracy_pos = over + under
* accuracy_neg = under + over
* These are always symmetric, so test the common boundary.
*/
combined = under + over;
/* --- percpu_counter_tree_approximate_compare --- */
/* At boundary: indeterminate */
percpu_counter_tree_set(&pct[0], (long)combined);
percpu_counter_tree_set(&pct[1], 0);
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare(&pct[0], &pct[1]));
percpu_counter_tree_set(&pct[0], 0);
percpu_counter_tree_set(&pct[1], (long)combined);
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare(&pct[0], &pct[1]));
/* Beyond boundary: definitive */
percpu_counter_tree_set(&pct[0], (long)(combined + 1));
percpu_counter_tree_set(&pct[1], 0);
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_approximate_compare(&pct[0], &pct[1]));
percpu_counter_tree_set(&pct[0], 0);
percpu_counter_tree_set(&pct[1], (long)(combined + 1));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_approximate_compare(&pct[0], &pct[1]));
/* --- percpu_counter_tree_precise_compare --- */
/* At boundary: precise gives exact result */
percpu_counter_tree_set(&pct[0], (long)combined);
percpu_counter_tree_set(&pct[1], 0);
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_precise_compare(&pct[0], &pct[1]));
percpu_counter_tree_set(&pct[0], 0);
percpu_counter_tree_set(&pct[1], (long)combined);
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_precise_compare(&pct[0], &pct[1]));
/* Beyond boundary: definitive */
percpu_counter_tree_set(&pct[0], (long)(combined + 1));
percpu_counter_tree_set(&pct[1], 0);
KUNIT_EXPECT_EQ(test, 1,
percpu_counter_tree_precise_compare(&pct[0], &pct[1]));
percpu_counter_tree_set(&pct[0], 0);
percpu_counter_tree_set(&pct[1], (long)(combined + 1));
KUNIT_EXPECT_EQ(test, -1,
percpu_counter_tree_precise_compare(&pct[0], &pct[1]));
/* Equal counters */
percpu_counter_tree_set(&pct[0], 42);
percpu_counter_tree_set(&pct[1], 42);
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_approximate_compare(&pct[0], &pct[1]));
KUNIT_EXPECT_EQ(test, 0,
percpu_counter_tree_precise_compare(&pct[0], &pct[1]));
percpu_counter_tree_destroy_many(pct, 2);
kfree(counter_items);
}
Cheers
On Mon, 16 Mar 2026 at 13:06, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> On 2026-03-16 00:28, David CARLIER wrote:
> > You're right. Here's the simplified version:
> >
> Much better. So this covers percpu_counter_tree_approximate_compare_value.
>
> Can we also add coverage for the following affected APIs ?
>
> - percpu_counter_tree_precise_compare_value
> - percpu_counter_tree_precise_compare
> - percpu_counter_tree_approximate_compare
>
> When comparing between two sets of counters, we end up summing the
> the accuracy ranges of the two counters. So in case we ever have an
> issue there, it would be good to add coverage of the range limit cases
> in the same way.
>
> Thanks,
>
> Mathieu
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-16 13:41 ` David CARLIER
@ 2026-03-16 13:53 ` Mathieu Desnoyers
2026-03-16 14:15 ` David CARLIER
2026-03-16 14:15 ` David CARLIER
0 siblings, 2 replies; 15+ messages in thread
From: Mathieu Desnoyers @ 2026-03-16 13:53 UTC (permalink / raw)
To: David CARLIER; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
On 2026-03-16 09:41, David CARLIER wrote:
> Sure. For precise_compare_value, the boundary tests extend
> naturally — the function must return the exact result regardless
> of which code path (fast approximate or precise fallback) is taken.
>
> For the two-counter APIs, I noticed that the combined accuracy
> is always symmetric: over_a + under_b = under_a + over_b =
> (bs_a + bs_b - 1) * multiplier, so the asymmetric gap test
> doesn't apply there. But boundary tests at the combined accuracy
> limit still provide useful coverage.
In order to do a relevant test for the two-counter APIs, we'll need
to initialize each of the two counters with different batch sizes.
This will ensure that the limits are asymmetric, and therefore allow
more precise testing of the limit conditions.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-16 13:53 ` Mathieu Desnoyers
@ 2026-03-16 14:15 ` David CARLIER
2026-03-16 14:15 ` David CARLIER
1 sibling, 0 replies; 15+ messages in thread
From: David CARLIER @ 2026-03-16 14:15 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
On Mon, 16 Mar 2026 at 13:53, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> On 2026-03-16 09:41, David CARLIER wrote:
> > Sure. For precise_compare_value, the boundary tests extend
> > naturally — the function must return the exact result regardless
> > of which code path (fast approximate or precise fallback) is taken.
> >
> > For the two-counter APIs, I noticed that the combined accuracy
> > is always symmetric: over_a + under_b = under_a + over_b =
> > (bs_a + bs_b - 1) * multiplier, so the asymmetric gap test
> > doesn't apply there. But boundary tests at the combined accuracy
> > limit still provide useful coverage.
>
> In order to do a relevant test for the two-counter APIs, we'll need
> to initialize each of the two counters with different batch sizes.
> This will ensure that the limits are asymmetric, and therefore allow
> more precise testing of the limit conditions.
>
> Thanks,
>
> Mathieu
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-16 13:53 ` Mathieu Desnoyers
2026-03-16 14:15 ` David CARLIER
@ 2026-03-16 14:15 ` David CARLIER
2026-03-16 14:23 ` Mathieu Desnoyers
1 sibling, 1 reply; 15+ messages in thread
From: David CARLIER @ 2026-03-16 14:15 UTC (permalink / raw)
To: Mathieu Desnoyers; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
On Mon, 16 Mar 2026 at 13:53, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
>
> On 2026-03-16 09:41, David CARLIER wrote:
> > Sure. For precise_compare_value, the boundary tests extend
> > naturally — the function must return the exact result regardless
> > of which code path (fast approximate or precise fallback) is taken.
> >
> > For the two-counter APIs, I noticed that the combined accuracy
> > is always symmetric: over_a + under_b = under_a + over_b =
> > (bs_a + bs_b - 1) * multiplier, so the asymmetric gap test
> > doesn't apply there. But boundary tests at the combined accuracy
> > limit still provide useful coverage.
>
> In order to do a relevant test for the two-counter APIs, we'll need
> to initialize each of the two counters with different batch sizes.
> This will ensure that the limits are asymmetric, and therefore allow
> more precise testing of the limit conditions.
>
> Thanks,
>
> Mathieu
I looked into this but since accuracy_multiplier is a single global
constant, the combined accuracy ends up symmetric regardless of
batch sizes. For the two-counter comparison:
accuracy_pos = over_a + under_b
accuracy_neg = under_a + over_b
Since under = batch_size * M and over = (batch_size - 1) * M,
both sides always simplify to the same value. So the asymmetric
gap test only applies to the single-value comparison APIs.
Should I keep the two-counter test with symmetric boundary
coverage, or did you have something else in mind?
Thanks
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree
2026-03-16 14:15 ` David CARLIER
@ 2026-03-16 14:23 ` Mathieu Desnoyers
0 siblings, 0 replies; 15+ messages in thread
From: Mathieu Desnoyers @ 2026-03-16 14:23 UTC (permalink / raw)
To: David CARLIER; +Cc: Andrew Morton, Josh Law, Dennis Zhou, linux-kernel
On 2026-03-16 10:15, David CARLIER wrote:
> On Mon, 16 Mar 2026 at 13:53, Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com> wrote:
>>
>> On 2026-03-16 09:41, David CARLIER wrote:
>>> Sure. For precise_compare_value, the boundary tests extend
>>> naturally — the function must return the exact result regardless
>>> of which code path (fast approximate or precise fallback) is taken.
>>>
>>> For the two-counter APIs, I noticed that the combined accuracy
>>> is always symmetric: over_a + under_b = under_a + over_b =
>>> (bs_a + bs_b - 1) * multiplier, so the asymmetric gap test
>>> doesn't apply there. But boundary tests at the combined accuracy
>>> limit still provide useful coverage.
>>
>> In order to do a relevant test for the two-counter APIs, we'll need
>> to initialize each of the two counters with different batch sizes.
>> This will ensure that the limits are asymmetric, and therefore allow
>> more precise testing of the limit conditions.
>>
>> Thanks,
>>
>> Mathieu
>
> I looked into this but since accuracy_multiplier is a single global
> constant, the combined accuracy ends up symmetric regardless of
> batch sizes. For the two-counter comparison:
>
> accuracy_pos = over_a + under_b
> accuracy_neg = under_a + over_b
>
> Since under = batch_size * M and over = (batch_size - 1) * M,
> both sides always simplify to the same value. So the asymmetric
> gap test only applies to the single-value comparison APIs.
>
> Should I keep the two-counter test with symmetric boundary
> coverage, or did you have something else in mind?
No, you're right, after simplification they become equivalent.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-03-16 14:23 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-13 17:54 [PATCH] lib: fix compare_delta parameter order in percpu_counter_tree David Carlier
2026-03-14 15:30 ` Josh Law
2026-03-14 21:45 ` Andrew Morton
2026-03-15 22:00 ` Mathieu Desnoyers
2026-03-15 22:47 ` David CARLIER
2026-03-15 23:16 ` Mathieu Desnoyers
2026-03-16 0:05 ` David CARLIER
2026-03-16 0:41 ` Mathieu Desnoyers
2026-03-16 4:28 ` David CARLIER
2026-03-16 13:06 ` Mathieu Desnoyers
2026-03-16 13:41 ` David CARLIER
2026-03-16 13:53 ` Mathieu Desnoyers
2026-03-16 14:15 ` David CARLIER
2026-03-16 14:15 ` David CARLIER
2026-03-16 14:23 ` Mathieu Desnoyers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox