Re: [PATCH v21 0/6] Hierarchical Percpu Counters for RSS

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

On 2026-09-03 13:18, Shakeel Butt wrote:
> On Tue, Sep 01, 2026 at 02:28:45PM -0400, Mathieu Desnoyers wrote:
>> Hi Andrew,
>>
>> Here is the hierarchical percpu counters for RSS accounting series
>> rebased on top of v7.3-rc1. It includes small bootup fixes which were
>> needed to fix bootup sequence on specific architectures, and a rename of
>> the test config option to include "KUNIT_".
>>
>> This aims at replacing the prior version of the series you had
>> in mm.
>>
>> As a reminder, the goal here is to provide more precise RSS counters
>> through /proc. A future step would be to also use these counters to
>> speed up the OOM killer task selection when OOM is triggered by
>> memcg. In whole-machine OOM situations, latency introduced by the
>> OOM killer task selection may not be too much of an issue, but for
>> memcg, where cgroups may routinely depend on the OOM killer, the
>> acceptable latency tradeoff is quite different.
>>
> 
> Hi Mathieu, can you please comment on the Sashiko's findings [1]?
> Worth addressing or not?
> 
> [1] https://sashiko.dev/#/patchset/20260901182857.26690-1-mathieu.desnoyers%40efficios.com
> 

I've looked at the report. There are 4 classes of things happening
there. FWIW I've also used Claude to counter-check the report.

Summary: no code change appears to be needed for classes 1 and 2.
I should do test and module teardown fixes, as well as overflow cleanup
for a v22.

* Class 1: the supposed "over vs under" swapped accuracy bounds. In that
case it appears to be the LLM which is confused. This is because the range
of approximation as a function of the precise sum is expressed in the
opposite direction than the range of precise sum as a function of the
approximation:

+ * The range of the approximation as a function of the precise sum is expressed as:
+ *
+ *   approx_sum >= precise_sum - approx_accuracy_range.under
+ *   approx_sum <= precise_sum + approx_accuracy_range.over
+ *
+ * Therefore, the range of the precise sum as a function of the approximation is expressed as:
+ *
+ *   precise_sum <= approx_sum + approx_accuracy_range.under
+ *   precise_sum >= approx_sum - approx_accuracy_range.over

We could possibly extend this documentation elsewhere to make sure that a
LLM with limited context don't get confused, but the code is OK there.

Sashiko wrote that the positive margin "should be over_a + under_b" and then
claimed the code passes under_a + over_b. The code passes a->over + b->under.
Exactly what Sashiko itself derived. Its reasoning agreed with the code; it
misread which argument was which.

* Class 2:

> +void percpu_counter_tree_add(struct percpu_counter_tree *counter, long inc)
> +{
> +    unsigned long bit_mask = counter->level0_bit_mask, orig, res;
> +
> +    res = this_cpu_add_return(*counter->level0, inc);
> +    orig = res - inc;
> +    inc = percpu_counter_tree_carry(orig, res, inc, bit_mask);
> +    if (likely(!inc))
> +        return;
> +    percpu_counter_tree_add_slowpath(counter, inc);
> +}

Sashiko:
Does this code risk an unbounded approximation error if preemption occurs
after this_cpu_add_return() but before percpu_counter_tree_add_slowpath()?
The level 0 counter reflects the addition immediately, but if the task is
preempted before propagating the carry to the tree, that carry is held back
indefinitely. Multiple preempted tasks could cause the approximate sum to
deviate from the precise sum by an amount far exceeding the topological
accuracy bounds. Could we disable preemption across this entire sequence?

My take on this:

I think we need a clearer _documentation_ statement about what the approximation
upper bounds guarantee. It guarantees upper bounds on the approximation error
_when the counters are quiescent_, never when they are actively being concurrently
updated. So if a thread is preempted _while propagating the carry_, it's not
quiescent. I don't think we need any code change there. And disabling preemption
would not "fix" anything there, it would merely make the race smaller.


* Class 3:

Test cases boilerplate, module teardown and cpu hotplug issues: Good catches,
I'll fix them.

* Class 4: Overflows

In order to be strictly C compliant, I should do the difference in unsigned
(where overflow/underflow is OK), and cast the result as signed to compare
with 0 with a inequality. It won't change things in practice, but it's a good
call.

Thoughts ?

Thanks,

Mathieu



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