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

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

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