Re: [RFC PATCH v24 00/10] Hierarchical Percpu Counters for RSS

["David Hildenbrand (Arm)" <david@kernel.org>]

On 11/5/26 21:36, Mathieu Desnoyers wrote:
> Hi David,

Hi!

> 
> Here is the hierarchical per-cpu counters series for the RSS counters,
> based on v7.4-rc1.  It is an RFC because of the questions at the end,
> and because of the significant changes since v22 [1], notably the
> per-cpu counter size (long -> byte), the configuration heuristics, and
> the lazy activation introduced by patches 6 and 7.  v24 fixes the bugs
> found by the Sashiko review of v23 [2], including a NULL pointer
> dereference in patches 3 to 5 on sparc64.

I think we discussed other possible applications (IIRC Michal expressed concern
if a single user is sufficient motivation for these additional LOC). Did
anything result from that discussion that is relevant to v24?


> 
> The problem
> -----------
> 
> Since commit f1a7941243c1 ("mm: convert mm's rss stats into
> percpu_counter"), the RSS counters are struct percpu_counter with a
> batch size of max(32, 2 * nr_cpus): their approximation error grows with
> O(nr_cpus^2), up to 1.1GB per counter at 384 logical CPUs.

Okay, so that's a per-MM problem, not a per-system problem. On a large system
with many CPUs I'd have said "likel we don't care" :)

> /proc/PID/status, /proc/PID/statm and the OOM killer moved to precise
> sums (commits 82241a83cd15 and 5898aa8f9a0b), but the other readers
> still use the approximation: the RSS high watermark (VmHWM, the
> ru_maxrss reported as "Maximum resident set size" by /usr/bin/time -v,
> taskstats), /proc/PID/stat (ps, top, monitoring agents), the taskstats
> RSS-time integral (coremem, sampled at each tick), NUMA balancing and
> the mark_victim tracepoint.  On the test machine below, the
> /proc/PID/stat RSS of a test process was off by 17MB while
> single-threaded, and by up to 134MB with 64 threads.

What's the resulting impact of that (temporary) imprecision?

> 
> Precise sums cannot fix the high watermark or the RSS-time integral:
> update_hiwater_rss() samples the RSS on every unmap path, and the

As a side-note, I absolutely hate update_hiwater_rss() on the unmap path.

> integral at each tick, while a precise sum reads every possible CPU,
> about 4us per counter at 384 logical CPUs.  A smaller struct
> percpu_counter batch takes the counter spinlock on every carry: at batch
> 32, an update of the counters of a shared mm costs 4.4 to 69us with one
> thread per CPU, compared to 26ns to 1.2us with this series.
> 
> The series
> ----------
> 
> Patches 1 to 5 replace struct percpu_counter for the RSS counters with
> hierarchical per-cpu counters: per-cpu bytes carry into a tree of atomic
> counters shaped at boot from nr_cpu_ids, with a fixed batch of 32, an
> error bound in O(n*log(n)) and no global lock.  

How does this interact with CPU hotplug?

> Each level carries only
> once its residual reaches the batch in either direction, so updates
> going back and forth around a boundary do not climb the tree.  The
> comparison functions have no user in this series; a follow-up will use
> them for a faster OOM killer task selection.
> 
> Patches 6 and 7 keep the RSS counters of single-threaded processes exact
> and without per-cpu memory: until a second thread shares the mm, the
> task running on the mm adds to the bias of each counter without atomic
> instructions, and other tasks add to its atomic bias, both read in O(1).
> Creating the second thread allocates the per-cpu counters and tree
> items, and activates the tree.  struct percpu_counter cannot do this as
> is: its count is an s64 under a raw spinlock, which the remote updates
> (reclaim, khugepaged, remote GUP) would then take.

What was the particular reason to special-case single-threaded apps? Do we
expect them in practice to really be that RSS heavy+dynamic such that it would
matter?

> 
> Patches 8 to 10 are the tools to tune the tree shape on other hardware:
> the percpu_counter_tree.arity= and rss_stat_batch= boot parameters, and
> a KUnit benchmark.  The default shapes were measured on one machine,
> with up to 384 logical CPUs, so results from other hardware are welcome.

Hm, so that means one might have to fine tune these toggles to get the best
performance? That's a bit unfortunate.

> 
> Results
> -------
> 
> 2-socket AMD EPYC 9654, 192 cores, 384 logical CPUs with SMT (the CPU
> counts in this letter are logical CPUs, as in nr_cpu_ids), tree 8,8,8,
> batch 32, whole series; details in patches 3 and 7.  The bare-metal
> results were measured on v7.3: the rebase onto v7.4-rc1 had no conflict,
> and the hpcc update costs of the series as sent, measured in a pinned
> 384-vCPU guest, match the bare-metal ones within 2%.
> 
>   Accuracy per counter: bound, and largest |approximate - precise| seen
>   by get_mm_counter() while a process hops across all CPUs doing
>   mmap/munmap of 1 to 16 pages (384-vCPU KVM guest pinned to the host):
> 
>                                         hpcc   upstream
>     bound                            158.5MB      1.1GB
>     measured, single-threaded      0 (exact)      841MB
>     measured, with a second thread      96MB      845MB
> 
>   Update cost, ns per update per thread, one thread per CPU, bare metal
>   (mm: the 4 counters of one mm; private: one counter per thread; batch
>   32: local change of the benchmark, bound 46.5MB):
> 
>                                               upstream   upstream
>                                        hpcc  batch 768   batch 32
>     mm, +1                               26        288       4370
>     mm, random +-32                     135        385      69171
>     mm, random +-512                   1174      20229      69186
>     private, random +-32                 31         20          -
> 
>   /proc/PID/statm read with 1000 to 16000 processes, bare metal:
>   5.3-6.5us vs 16.5-19.0us for single-threaded processes, on par for
>   multi-threaded ones (14.4-16.0us vs 14.5-17.1us, guest).  fork + exit
>   of a single-threaded child: 116us vs 139us.  will-it-scale
>   page_fault1, 2 and 3: no regression (-0.6% to +5.1%, one boot each,
>   the upstream one on the previous day).
> 
>   Memory per mm, counting the slab objects of mm_struct and of the tree
>   items, and the per-cpu memory of the RSS counters, in bytes (guest,
>   v7.4-rc1):
> 
>                   single-threaded   threaded   upstream
>     16 CPUs                  1820       1948       2076
>     384 CPUs                 1928       8145       8072
>     1024 CPUs                2185      17204      18569
> 
>   The tree items are allocated with the per-cpu counters when the second
>   thread is created, and the mm_struct slab object keeps its upstream
>   size.
> 
> Behavior changes
> ----------------
> 
> - The allocation of the per-cpu counters and tree items moves from
>   fork() to the second thread (clone() with CLONE_VM, without
>   CLONE_VFORK), which can then fail with -ENOMEM.
> - mm_struct gains rss_owner, the task running on the mm, compared with
>   current and never dereferenced, and rss_stat_mutex, which serializes
>   the activations of the tree, as mm_cid.mutex serializes the mode
>   changes of mm_cid at fork and exit.
> - The precise sums (/proc/PID/status, /proc/PID/statm, the OOM killer)
>   take no lock: under concurrent updates of a multi-threaded process,
>   they can miss a carry on its way up the tree, where upstream counts
>   each concurrent update either whole or not at all.  They are exact for
>   a process at rest.
> - The rss_stat tracepoint reports the approximation (patch 5): exact for
>   single-threaded processes, within the bound otherwise.
> - Like struct percpu_counter, the tree is sized at boot and not
>   reshaped on hotplug; its shape and bound depend on nr_cpu_ids, while
>   the default upstream batch follows the online CPUs.
> 
> Caveats
> -------
> 
> - From 33 to 39 CPUs, up to 1.37 times less accurate than upstream.
> - The tree is shaped for the number of CPUs rounded up to a power of 2:
>   just above a power of 2, the RSS counters of a threaded process, with
>   rss_owner and the mutex, use up to 1.50 times the memory of upstream
>   (129 CPUs, 1.28 times at 65 and 1.38 times at 257), compared to at
>   most 0.90 times at a power of 2, and 1.02 times at 384 logical CPUs.
> - Private counters with random +-32 updates: 31ns vs 20ns, the price of
>   a 7 times tighter bound.
> - A multi-threaded process which ran on few CPUs can hold more error
>   than upstream: 2047 pages per CPU path vs 767 at 384 logical CPUs.  A
>   follow-up could index the counters by concurrency ID (mm_cid) instead
>   of CPU, so the accuracy bound and the precise sums would scale with
>   the concurrency the process uses.
> - Before activation, concurrent remote updates of a single-threaded
>   process contend on one atomic per counter.
> - The second thread allocates per-cpu memory, which the per-cpu
>   allocator serializes, as it does upstream at every fork: with 384
>   threads at once, init + destroy of 4 counters takes 874us per
>   operation (1004us upstream, 6.0us alone).  Tree item slab objects
>   owning their per-cpu counters, with slab destructors, could avoid it.
> - The accuracy was measured in a guest, with a debug patch and a test
>   program which are not part of the series.  The bound is analytic and
>   tested in both directions.

[...]

> 
> Questions
> ---------
> 
> 1. Is mm->rss_owner acceptable, or would you prefer current->mm == mm
>    (without PF_KTHREAD) and an "activated" flag?

I'm afraid I'm the wrong person to ask that question. I'd have to look into the
details. If it's just a simple member, we likely don't care about the addition
memory footprint (if that's your main concern).

> 2. Is moving the -ENOMEM from fork() to the second thread acceptable?

I'd assume that's not a problem.

> 3. Would you rather take the single-threaded mode first, on its own?

Again, the question is how much we care in practice, I guess.

> 4. Should the rss_stat tracepoint report the approximation, stay
>    precise, or report both?

No strong optinion on that. Do we know who uses these and for which purpose?

> 5. Should the tuning tools be merged, built only with the benchmark
>    config, or dropped later?  Who could run them on non-AMD, 4-socket,
>    arm64 or POWER machines?

Is there some way to have it be an auto-tuning instead of requiring this tooling?

> 6. Is tools/testing/hpcc the right home for the topology test?
Good question, I'd assume so.

-- 
Cheers,

David