memcg: cache obj_stock by memcg, not by objcg pointer

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, but the per-CPU obj_stock_pcp still keys cached_objcg by pointer. Cross-NUMA workloads now see a drain on every refill and a miss on every consume that targets a sibling per-node objcg of the same memcg, producing the 67.7% stress-ng switch-mq regression reported by LKP.

stock->nr_bytes are fungible across per-node objcgs of one memcg:
drain_obj_stock() and obj_cgroup_uncharge_pages() both account via obj_cgroup_memcg(). Treat the cache as keyed by memcg in both __consume_obj_stock() and __refill_obj_stock() so siblings share the reserve -- eliminating the drain on free and keeping the alloc fast path in consume.

Though kernel test robot reported the regression but it was not easy to reproduce locally. Qi implemented [1] a specialized reproducer to show the corner case which cause the regression and then Qi tested the patch and reported that the corner case is eliminated after the patch.

Reported-by kernel test robot <[email protected]>

Closes https://lore.kernel.org/oe-lkp/[email protected] open here

Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")

Link https://lore.kernel.org/[email protected]/ [1] open here

Signed-off-by Shakeel Butt <[email protected]>

Debugged-by Qi Zheng <[email protected]>

Tested-by Qi Zheng <[email protected]>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Fungible sharing of stock reserve bytes between per-node objcgs breaks the obj_cgroup_release invariant, causing permanent kmem charge leaks and WARN_ON_ONCE.
- [High] Missing update to __account_obj_stock() bypasses vmstat batching for sibling objcgs, causing severe lock and atomic contention.
- [High] Calling obj_cgroup_memcg() without required RCU or cgroup locks triggers lockdep splats on fast paths.
--

commit e77fe5b6af93b69e06c1ccd13f83e26b59234483
Author: Shakeel Butt <[email protected]>

memcg: cache obj_stock by memcg, not by objcg pointer

This commit modifies the per-CPU object stock caching to share the reserve among per-node sibling obj_cgroups of the same memcg, aiming to reduce drains and refills on cross-NUMA workloads.

Link https://lore.kernel.org/oe-lkp/[email protected] open here

Link https://lore.kernel.org/[email protected]/ open here

Quoted 6 lines

mm/memcontrol.c +0 -0

 index d978e18b9b2d0..66448f4285316 100644
 --- a/mm/memcontrol.c
 +++ b/mm/memcontrol.c

mm/memcontrol.c

Quoted 10 lines

mm/memcontrol.c +0 -0

 @@ -3210,7 +3210,11 @@ static bool __consume_obj_stock(struct obj_cgroup *objcg,
 struct obj_stock_pcp *stock,
 unsigned int nr_bytes)
 {
 - if (objcg == READ_ONCE(stock->cached_objcg) &&
 + struct obj_cgroup *cached = READ_ONCE(stock->cached_objcg);
 +
 + /* Cache is keyed by memcg; sibling per-node objcgs share the reserve. */
 + if ((cached == objcg ||
 + (cached && obj_cgroup_memcg(cached) == obj_cgroup_memcg(objcg))) &&

Does calling obj_cgroup_memcg() here trigger lockdep warnings on slab fast paths?

Looking at obj_cgroup_memcg(), it expects rcu_read_lock_held() or lockdep_is_held(&cgroup_mutex) to be true.

Since this function is called from slab fast paths where only the local CPU stock lock is held, the RCU read lock may not be held.

Could this check directly compare READ_ONCE(cached->memcg) and READ_ONCE(objcg->memcg) to avoid the lockdep assertion?

Quoted 6 lines

stock->nr_bytes >= nr_bytes) {
stock->nr_bytes -= nr_bytes;
return true;

Will sharing the reserve between per-node sibling objcgs without updating stock->cached_objcg break the page multiple invariant in obj_cgroup_release()?

If an allocation for objcg_B consumes bytes originally funded by objcg_A, and the stock is later drained, those borrowed bytes are flushed into objcg_A->nr_charged_bytes.

When obj_cgroup_release() is invoked, nr_charged_bytes will not be an exact multiple of PAGE_SIZE. Will this trigger WARN_ON_ONCE(nr_bytes & (PAGE_SIZE - 1)) and truncate the remainder, permanently leaking the page charge from the memcg?

[ ... ]

Quoted 6 lines

mm/memcontrol.c +0 -0

@@ -3318,6 +3322,7 @@ static void __refill_obj_stock(struct obj_cgroup *objcg,
 unsigned int nr_bytes,
 bool allow_uncharge)
 {
 

[ ... ]

Quoted 8 lines

- if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */
+ cached = READ_ONCE(stock->cached_objcg);
+ /* Same memcg: bytes are fungible, no drain needed. */
+ if (cached != objcg &&
+ (!cached || obj_cgroup_memcg(cached) != obj_cgroup_memcg(objcg))) { drain_obj_stock(stock);

Since the stock->cached_objcg pointer is not updated when a sibling is already cached, does this bypass the vmstat batching in __account_obj_stock()?

If __account_obj_stock() still requires strict pointer equality (READ_ONCE(stock->cached_objcg) == objcg), alternating cross-node allocations will continually fail that match.

Will this force a fallback to direct mod_objcg_mlstate() updates on every object allocation and free, causing severe cacheline contention?

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Quoted 8 lines

Will sharing the reserve between per-node sibling objcgs without updating stock->cached_objcg break the page multiple invariant in obj_cgroup_release()?

If an allocation for objcg_B consumes bytes originally funded by objcg_A, and the stock is later drained, those borrowed bytes are flushed into objcg_A->nr_charged_bytes.

Quoted 4 lines

When obj_cgroup_release() is invoked, nr_charged_bytes will not be an exact multiple of PAGE_SIZE. Will this trigger WARN_ON_ONCE(nr_bytes & (PAGE_SIZE - 1)) and truncate the remainder, permanently leaking the page charge from the memcg?

This is actually a very good point and need more thought.