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

[Joshua Hahn]

On Wed, 20 May 2026 18:05:23 -0700 Shakeel Butt <shakeel.butt@xxxxxxxxx> wrote:

> On Wed, May 20, 2026 at 06:35:30PM +0900, Harry Yoo wrote:
> > 
> > 
> > On 5/20/26 2:31 PM, Shakeel Butt 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>
> > > ---
> > > @@ -3350,19 +3405,45 @@ static void __refill_obj_stock(struct obj_cgroup *objcg,
> > >   		goto out;
> > >   	}
> > > -	stock_nr_bytes = stock->nr_bytes;
> > > -	if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */
> > > -		drain_obj_stock(stock);
> > > +	for (i = 0; i < NR_OBJ_STOCK; ++i) {
> > > +		struct obj_cgroup *cached = READ_ONCE(stock->cached[i]);
> > > +
> > > +		if (!cached) {
> > > +			if (empty_slot == -1)
> > > +				empty_slot = i;
> > > +			continue;
> > > +		}
> > > +		if (cached == objcg) {
> > > +			slot = i;
> > > +			break;
> > > +		}
> > > +	}
> > > +
> > > +	if (slot == -1) {
> > > +		slot = empty_slot;
> > > +		if (slot == -1) {
> > > +			slot = get_random_u32_below(NR_OBJ_STOCK);
> > 
> > It would break kmalloc_nolock() because _get_random_bytes() uses a spinlock.
> > perhaps prandom_u32_state() should be sufficient in this case.
> 
> > Is there a reason why it uses random eviction, unlike multi-memcg percpu
> > charge cache?
> 
> Oh I didn't know and actually we are already using get_random_u32_below() in
> refill_stock(). So, it need fixing as well. That would be a separate patch.

Hello Harry and Shakeel!

I just wanted to note that my work to push the memcg stock to page_counter [1]
actually gets rid of the random eviction and also the get_random_u32_below.
So the problem will no longer exist, at least for memcg/page_counter stock.

I was also thinking whether there was any way we can abstract or push down the
objcg stock to a different per_X slot (with finer granularity than per-node)
so we don't do random evictions, but it doesn't seem like there's a natural
point to do this from a quick glance.

The fix for get_random_32_below to use prandom_u32_state seems quite simple
though, so if either of you would like to send that in as a small hotfix
it shouldn't be a big deal to rebase on top of that instead too.

(Sorry for joining the conversation late too. I'll take a closer look at this
entire series tomorrow as well).

Thank you both. Have a great day!
Joshua

[1] https://lore.kernel.org/all/20260410210742.550489-1-joshua.hahnjy@xxxxxxxxx/