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

[David CARLIER]

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@xxxxxxxxxxxx> 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