Re: [PATCH v2 4/4] memcg: multi objcg charge support

[Muchun Song]

> On May 22, 2026, at 09:19, Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:
> 
> Commit 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg
> per-node type") split a memcg's single obj_cgroup into one per NUMA
> node so that reparenting LRU folios can take per-node lru locks. As a
> side effect, the per-CPU obj_stock_pcp -- which caches exactly one
> cached_objcg -- thrashes on workloads where threads of the same memcg
> run on different NUMA nodes. The kernel test robot reported a 67.7%
> regression on stress-ng.switch.ops_per_sec from this pattern.
> 
> Mirror the multi-slot pattern already used by memcg_stock_pcp: turn
> nr_bytes and cached_objcg into NR_OBJ_STOCK-element arrays, scan all
> slots on consume/refill/account, prefer empty slots when inserting,
> and evict a random slot only when full. With multiple slots a CPU can
> hold the per-node objcg variants of one memcg plus a few siblings
> without ever forcing a drain.
> 
> A single int8_t index records which slot the cached slab stats belong
> to; the stats are flushed on slot or pgdat change. With NR_OBJ_STOCK
> = 5 the layout (verified with pahole) is:
> 
>  offset 0  : lock(1) + index(1) + node_id(2) + slab stats(4) = 8B
>  offset 8  : nr_bytes[5]                                     = 10B
>  offset 18 : padding                                         = 6B
>  offset 24 : cached[5]                                       = 40B
>  offset 64 : (line 2) work_struct + flags (cold)
> 
> so consume_obj_stock, refill_obj_stock and the slab account path each
> touch exactly one 64-byte cache line on non-debug 64-bit builds.
> 
> Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
> Closes: https://lore.kernel.org/oe-lkp/202605121641.b6a60cb0-lkp@xxxxxxxxx
> Fixes: 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")
> Signed-off-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> Tested-by: kernel test robot <oliver.sang@xxxxxxxxx>
> ---
> 
> Changes since v1:
> - Use round robin for drain
> 
> mm/memcontrol.c | 188 ++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 136 insertions(+), 52 deletions(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 78c02451312b..ba17633b0bd0 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -150,14 +150,14 @@ static void obj_cgroup_release(struct percpu_ref *ref)
> 	* However, it can be PAGE_SIZE or (x * PAGE_SIZE).
> 	*
> 	* The following sequence can lead to it:
> - 	* 1) CPU0: objcg == stock->cached_objcg
> + 	* 1) CPU0: objcg cached in one of stock->cached[i]
> 	* 2) CPU1: we do a small allocation (e.g. 92 bytes),
> 	*          PAGE_SIZE bytes are charged
> 	* 3) CPU1: a process from another memcg is allocating something,
> 	*          the stock if flushed,
> 	*          objcg->nr_charged_bytes = PAGE_SIZE - 92
> 	* 5) CPU0: we do release this object,
           ^
           4

Since you're already modifying the comments in this section,
would you mind fixing the numbering as well? I noticed that the
sequence was wrong a while back :)

> - 	*          92 bytes are added to stock->nr_bytes
> + 	*          92 bytes are added to stock->nr_bytes[i]
> 	* 6) CPU0: stock is flushed,
           ^
           5

Thanks,
Muchun

> 	*          92 bytes are added to objcg->nr_charged_bytes