Re: [PATCH v2 4/4] memcg: multi objcg charge support
From: Shakeel Butt
Date: Fri May 22 2026 - 12:54:13 EST
On Fri, May 22, 2026 at 02:33:36PM +0800, Muchun Song wrote:
>
>
> > 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 :)
Haha I didn't even notice. If I send a new version, I will fix this otherwise I
will ask Andrew to fix inplace.