memcg: shrink obj_stock_pcp and cache multiple objcgs
Subsystems (3)
Shakeel Butt
maintainerv2 cover memcg: shrink obj_stock_pcp and cache multiple objcgs
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 a single cached_objcg pointer -- 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.
Commit d0211878ce06 ("memcg: cache obj_stock by memcg, not by objcg pointer") landed as a temporary fix by treating sibling per-node objcgs as equivalent for the cache lookup, intended to be reverted once per-node kmem accounting is introduced. This series takes a more general approach: cache multiple objcgs per CPU using the multi-slot pattern memcg_stock_pcp already uses, so the per-node objcg variants of one memcg can all coexist in the stock without ever forcing a drain. The temporary fix can then be reverted.
To avoid increasing the per-CPU cache footprint, the first three patches shrink the existing single-slot obj_stock_pcp fields. The final patch converts cached_objcg and nr_bytes into NR_OBJ_STOCK=5 slot arrays and reorders the struct so the entire consume/refill/account hot path fits within a single 64-byte cache line on non-debug 64-bit builds (verified with pahole).
Shakeel Butt (4):
memcg: store node_id instead of pglist_data pointer
memcg: uint16_t for nr_bytes in obj_stock_pcp
memcg: int16_t for cached slab stats
memcg: multi objcg charge support
mm/memcontrol.c | 214 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 157 insertions(+), 57 deletions(-)
--
Changes since v1:
http://lore.kernel.org/[email protected]
- Collected review tags (Harry & Muchun)
- Fix comparison operators (Harry)
- Use round robin for drain
2.53.0-Meta
Shakeel Butt
maintainerPATCH v2 1/4 memcg: store node_id instead of pglist_data pointer
The struct obj_stock_pcp stores a pointer to pglist_data for the slab stats cached on the cpu. On 64-bit machines, this costs 8 bytes. The pointer is not strictly required: NODE_DATA() can recover it from the node id. Replace cached_pgdat with int16_t node_id and use NUMA_NO_NODE as the "no stats cached" sentinel.
At the moment all the archs limit MAX_NUMNODES to 1024 so int16_t is plenty; a BUILD_BUG_ON() makes sure we notice if that ever changes.
---
Changes since v1:
- Added tags in the commit message
mm/memcontrol.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
mm/memcontrol.c +19 -7
@@ -2021,7 +2021,7 @@ struct obj_stock_pcp {local_trylock_t lock;unsigned int nr_bytes;struct obj_cgroup *cached_objcg; -struct pglist_data *cached_pgdat; +int16_t node_id;int nr_slab_reclaimable_b;int nr_slab_unreclaimable_b;
@@ -2031,6 +2031,7 @@ struct obj_stock_pcp { static DEFINE_PER_CPU_ALIGNED(struct obj_stock_pcp, obj_stock) = { .lock =INIT_LOCAL_TRYLOCK (lock), + .node_id =NUMA_NO_NODE , };static DEFINE_MUTEX (percpu_charge_mutex);
@@ -3159,6 +3160,13 @@ static void __account_obj_stock(struct obj_cgroup *objcg, {int *bytes; +/* +* Though at the moment MAX_NUMNODES <= 1024 in all archs but let's make +* sure it does not exceed S16_MAX otherwise we need to fix node_id type +* in struct obj_stock_pcp. +*/ +BUILD_BUG_ON (MAX_NUMNODES >=S16_MAX ); +if (!stock ||READ_ONCE (stock->cached_objcg) != objcg) goto direct;
@@ -3166,9 +3174,11 @@ static void __account_obj_stock(struct obj_cgroup *objcg,* Save vmstat data in stock and skip vmstat array update unless * accumulating over a page of vmstat data or when pgdat changes. */ -if (stock->cached_pgdat != pgdat) { +if (stock->node_id ==NUMA_NO_NODE ) { + stock->node_id = pgdat->node_id; + }else if (stock->node_id != pgdat->node_id) {/* Flush the existing cached vmstat data */ -struct pglist_data *oldpg = stock->cached_pgdat; +struct pglist_data *oldpg =NODE_DATA (stock->node_id);if (stock->nr_slab_reclaimable_b) { mod_objcg_mlstate(objcg, oldpg, NR_SLAB_RECLAIMABLE_B,
@@ -3180,7 +3190,7 @@ static void __account_obj_stock(struct obj_cgroup *objcg, stock->nr_slab_unreclaimable_b); stock->nr_slab_unreclaimable_b =0 ; } - stock->cached_pgdat = pgdat; + stock->node_id = pgdat->node_id; } bytes = (idx == NR_SLAB_RECLAIMABLE_B) ? &stock->nr_slab_reclaimable_b
@@ -3276,19 +3286,21 @@ static void drain_obj_stock(struct obj_stock_pcp *stock)* Flush the vmstat data in current stock */ if (stock->nr_slab_reclaimable_b || stock->nr_slab_unreclaimable_b) { +struct pglist_data *oldpg =NODE_DATA (stock->node_id); +if (stock->nr_slab_reclaimable_b) { -mod_objcg_mlstate (old, stock->cached_pgdat, +mod_objcg_mlstate (old, oldpg,NR_SLAB_RECLAIMABLE_B , stock->nr_slab_reclaimable_b); stock->nr_slab_reclaimable_b =0 ; }if (stock->nr_slab_unreclaimable_b) { -mod_objcg_mlstate (old, stock->cached_pgdat, +mod_objcg_mlstate (old, oldpg,NR_SLAB_UNRECLAIMABLE_B , stock->nr_slab_unreclaimable_b); stock->nr_slab_unreclaimable_b =0 ; } - stock->cached_pgdat =NULL ; + stock->node_id =NUMA_NO_NODE ; }WRITE_ONCE (stock->cached_objcg,NULL );
Signature · 1 line
2.53.0-Meta
Shakeel Butt
maintainerPATCH v2 2/4 memcg: uint16_t for nr_bytes in obj_stock_pcp
In reply to Shakeel ButtCurrently struct obj_stock_pcp stores nr_bytes in an 'unsigned int' which is 4 bytes on 64-bit machines. Switch the field to uint16_t to shrink the per-CPU cache.
The kernel supports PAGE_SIZE_4KB, _8KB, _16KB, _32KB, _64KB and _256KB (see HAVE_PAGE_SIZE_* in arch/Kconfig). After the PAGE_SIZE-aligned flush in __refill_obj_stock(), the sub-page remainder fits in uint16_t up through 64KiB pages where PAGE_SIZE - 1 == U16_MAX, but on 256KiB pages PAGE_SIZE - 1 == 0x3FFFF exceeds U16_MAX. The accumulator also needs to stay within uint16_t between page-aligned flushes on 64KiB pages where PAGE_SIZE itself is U16_MAX + 1.
Accumulate the new total in an 'unsigned int' local, then:
1. Flush whenever the accumulator would hit U16_MAX. Together with the existing allow_uncharge flush at PAGE_SIZE, this keeps the uint16_t safe on PAGE_SIZE <= 64KiB.
2. On configs with PAGE_SHIFT > 16 (PAGE_SIZE_256KB on hexagon and powerpc 44x), push any sub-page remainder above U16_MAX into objcg->nr_charged_bytes via atomic_add before storing back, so the store cannot silently truncate. The PAGE_SHIFT > 16 guard folds the branch out at compile time on smaller page sizes.
---
Changes since v1:
- Collected tags
- Rearrange fields of obj_stock_pcp (David Laight)
- Fix comparison operator (Harry)
mm/memcontrol.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-)
mm/memcontrol.c +27 -6
@@ -2019,8 +2019,8 @@ static DEFINE_PER_CPU_ALIGNED(struct memcg_stock_pcp, memcg_stock) = {struct obj_stock_pcp {local_trylock_t lock; -unsigned int nr_bytes;struct obj_cgroup *cached_objcg; +uint16_t nr_bytes;int16_t node_id;int nr_slab_reclaimable_b;int nr_slab_unreclaimable_b;
@@ -3331,6 +3331,7 @@ static void __refill_obj_stock(struct obj_cgroup *objcg, bool allow_uncharge) {unsigned int nr_pages =0 ; +unsigned int stock_nr_bytes;if (!stock) { nr_pages = nr_bytes >>PAGE_SHIFT ;
@@ -3339,21 +3340,41 @@ 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);obj_cgroup_get (objcg); - stock->nr_bytes =atomic_read (&objcg->nr_charged_bytes) + stock_nr_bytes =atomic_read (&objcg->nr_charged_bytes) ?atomic_xchg (&objcg->nr_charged_bytes,0 ) :0 ;WRITE_ONCE (stock->cached_objcg, objcg); allow_uncharge = true;/* Allow uncharge when objcg changes */ } - stock->nr_bytes += nr_bytes; + stock_nr_bytes += nr_bytes; + +/* Since stock->nr_bytes is uint16_t, don't refill >= U16_MAX */ +if ((allow_uncharge && (stock_nr_bytes >PAGE_SIZE )) || + stock_nr_bytes >U16_MAX ) { + nr_pages = stock_nr_bytes >>PAGE_SHIFT ; + stock_nr_bytes &= (PAGE_SIZE -1 ); + +/* +* On configs with PAGE_SHIFT > 16 (PAGE_SIZE_256KB on +* hexagon and powerpc 44x), the sub-page remainder can +* still exceed U16_MAX. Push the excess back to +* objcg->nr_charged_bytes so the store into uint16_t +* cannot silently truncate; folded out at compile time +* on smaller page sizes. +*/ +if (PAGE_SHIFT >16 && stock_nr_bytes >U16_MAX ) { +unsigned int kept = stock_nr_bytes &U16_MAX ; -if (allow_uncharge && (stock->nr_bytes >PAGE_SIZE )) { - nr_pages = stock->nr_bytes >>PAGE_SHIFT ; - stock->nr_bytes &= (PAGE_SIZE -1 ); +atomic_add (stock_nr_bytes - kept, + &objcg->nr_charged_bytes); + stock_nr_bytes = kept; + } } + stock->nr_bytes = stock_nr_bytes; out:if (nr_pages)
Signature · 1 line
2.53.0-Meta
Shakeel Butt
maintainerPATCH v2 3/4 memcg: int16_t for cached slab stats
In reply to Shakeel ButtCurrently struct obj_stock_pcp stores cached slab stats in 'int' which is 4 bytes per counter on 64-bit machines. Switch them to int16_t to shrink the cached metadata.
The existing PAGE_SIZE flush in __account_obj_stock() bounds *bytes at PAGE_SIZE on 4KiB and 16KiB page archs, well within int16_t. On 64KiB pages PAGE_SIZE is well above S16_MAX so that flush never fires, and a sufficiently long run of accumulations would overflow the cache. Add an explicit S16_MAX guard before each add: when the next add would push abs(*bytes) past S16_MAX, fold the cached value into @nr and flush directly via mod_objcg_mlstate() before the accumulation.
---
Changes since v2:
- Collected tags
mm/memcontrol.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
mm/memcontrol.c +13 -3
@@ -2022,8 +2022,8 @@ struct obj_stock_pcp {struct obj_cgroup *cached_objcg;uint16_t nr_bytes;int16_t node_id; -int nr_slab_reclaimable_b; -int nr_slab_unreclaimable_b; +int16_t nr_slab_reclaimable_b; +int16_t nr_slab_unreclaimable_b;struct work_struct work;unsigned long flags;
@@ -3158,7 +3158,7 @@ static void __account_obj_stock(struct obj_cgroup *objcg, struct obj_stock_pcp *stock, int nr, struct pglist_data *pgdat, enum node_stat_item idx) { - int *bytes; + int16_t *bytes;/* * Though at the moment MAX_NUMNODES <= 1024 in all archs but let's make
@@ -3195,6 +3195,16 @@ static void __account_obj_stock(struct obj_cgroup *objcg, bytes = (idx ==NR_SLAB_RECLAIMABLE_B ) ? &stock->nr_slab_reclaimable_b : &stock->nr_slab_unreclaimable_b; +/* +* To avoid overflow or underflow, flush directly if accumulating @nr +* would push the cached value past S16_MAX. +*/ +if (abs (nr + *bytes) >S16_MAX ) { + nr += *bytes; + *bytes =0 ; + goto direct; + } +/* * Even for large object >= PAGE_SIZE, the vmstat data will still be * cached locally at least once before pushing it out.
Signature · 1 line
2.53.0-Meta
Shakeel Butt
maintainerPATCH v2 4/4 memcg: multi objcg charge support
In reply to Shakeel ButtCommit 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.
---
Changes since v1:
- Use round robin for drain
mm/memcontrol.c | 188 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 136 insertions(+), 52 deletions(-)
mm/memcontrol.c +136 -52
@@ -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, -* 92 bytes are added to stock->nr_bytes +* 92 bytes are added to stock->nr_bytes[i] * 6) CPU0: stock is flushed, * 92 bytes are added to objcg->nr_charged_bytes *
@@ -2017,25 +2017,40 @@ static DEFINE_PER_CPU_ALIGNED(struct memcg_stock_pcp, memcg_stock) = { .lock = INIT_LOCAL_TRYLOCK(lock), }; +/* +* NR_OBJ_STOCK is sized so the entire hot path of obj_stock_pcp +* (lock, accounting metadata, nr_bytes[] and cached[]) fits within a +* single 64-byte cache line on non-debug 64-bit builds. With 5 slots: +* lock(1) + index(1) + node_id(2) + slab stats(4) + nr_bytes(10) +* + pad(6) + cached(40) == 64 bytes. +* A CPU can thus consume/refill/account against five different objcgs +* (typically per-node variants of the same memcg) while incurring at +* most one cache miss on the stock. +*/ +#define NR_OBJ_STOCK 5struct obj_stock_pcp {local_trylock_t lock; -struct obj_cgroup *cached_objcg; -uint16_t nr_bytes; +int8_t index;int16_t node_id;int16_t nr_slab_reclaimable_b;int16_t nr_slab_unreclaimable_b; +uint16_t nr_bytes[NR_OBJ_STOCK ]; +struct obj_cgroup *cached[NR_OBJ_STOCK ];struct work_struct work;unsigned long flags; +uint8_t drain_idx; }; static DEFINE_PER_CPU_ALIGNED(struct obj_stock_pcp, obj_stock) = { .lock =INIT_LOCAL_TRYLOCK (lock), + .index =-1 , .node_id =NUMA_NO_NODE , };static DEFINE_MUTEX (percpu_charge_mutex); +static void drain_obj_stock_slot (struct obj_stock_pcp *stock,int i);static void drain_obj_stock (struct obj_stock_pcp *stock);static bool obj_stock_flush_required (struct obj_stock_pcp *stock,struct mem_cgroup *root_memcg);
@@ -3153,12 +3168,13 @@ static void unlock_stock(struct obj_stock_pcp *stock)local_unlock (&obj_stock.lock); } -/* Call after __refill_obj_stock() to ensure stock->cached_objg == objcg */ +/* Call after __refill_obj_stock() so a slot for objcg exists in the stock */ static void __account_obj_stock (struct obj_cgroup *objcg,struct obj_stock_pcp *stock,int nr,struct pglist_data *pgdat,enum node_stat_item idx) {int16_t *bytes; +int i;/* * Though at the moment MAX_NUMNODES <= 1024 in all archs but let's make
@@ -3167,29 +3183,39 @@ static void __account_obj_stock(struct obj_cgroup *objcg,*/ BUILD_BUG_ON (MAX_NUMNODES >=S16_MAX ); -if (!stock ||READ_ONCE (stock->cached_objcg) != objcg) +if (!stock) + goto direct; + +for (i =0 ; i <NR_OBJ_STOCK ; ++i) { +if (READ_ONCE (stock->cached[i]) == objcg) +break ; + } +if (i ==NR_OBJ_STOCK ) goto direct;/* * Save vmstat data in stock and skip vmstat array update unless -* accumulating over a page of vmstat data or when pgdat changes. +* accumulating over a page of vmstat data or when the objcg slot or +* pgdat the stats belong to changes. */ -if (stock->node_id ==NUMA_NO_NODE ) { +if (stock->index <0 ) { + stock->index = i; stock->node_id = pgdat->node_id; - }else if (stock->node_id != pgdat->node_id) { -/* Flush the existing cached vmstat data */ + }else if (stock->index != i || stock->node_id != pgdat->node_id) { +struct obj_cgroup *old =READ_ONCE (stock->cached[stock->index]);struct pglist_data *oldpg =NODE_DATA (stock->node_id);if (stock->nr_slab_reclaimable_b) { -mod_objcg_mlstate (objcg, oldpg,NR_SLAB_RECLAIMABLE_B , +mod_objcg_mlstate (old, oldpg,NR_SLAB_RECLAIMABLE_B , stock->nr_slab_reclaimable_b); stock->nr_slab_reclaimable_b =0 ; }if (stock->nr_slab_unreclaimable_b) { -mod_objcg_mlstate (objcg, oldpg,NR_SLAB_UNRECLAIMABLE_B , +mod_objcg_mlstate (old, oldpg,NR_SLAB_UNRECLAIMABLE_B , stock->nr_slab_unreclaimable_b); stock->nr_slab_unreclaimable_b =0 ; } + stock->index = i; stock->node_id = pgdat->node_id; }
@@ -3230,10 +3256,16 @@ 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) && - stock->nr_bytes >= nr_bytes) { - stock->nr_bytes -= nr_bytes; -return true; + int i; + +for (i =0 ; i <NR_OBJ_STOCK ; ++i) { +if (READ_ONCE (stock->cached[i]) != objcg) +continue ; +if (stock->nr_bytes[i] >= nr_bytes) { + stock->nr_bytes[i] -= nr_bytes; +return true; + } +return false; }return false;
@@ -3254,16 +3286,42 @@ static bool consume_obj_stock(struct obj_cgroup *objcg, unsigned int nr_bytes)return ret; } -static void drain_obj_stock (struct obj_stock_pcp *stock) +/* Flush the cached slab stats (if any) back to their owning objcg/pgdat. */ +static void drain_obj_stock_stats (struct obj_stock_pcp *stock) { -struct obj_cgroup *old =READ_ONCE (stock->cached_objcg); +struct obj_cgroup *old; +struct pglist_data *oldpg; + +if (stock->index <0 ) +return ; + + old =READ_ONCE (stock->cached[stock->index]); + oldpg =NODE_DATA (stock->node_id); + +if (stock->nr_slab_reclaimable_b) { +mod_objcg_mlstate (old, oldpg,NR_SLAB_RECLAIMABLE_B , + stock->nr_slab_reclaimable_b); + stock->nr_slab_reclaimable_b =0 ; + } +if (stock->nr_slab_unreclaimable_b) { +mod_objcg_mlstate (old, oldpg,NR_SLAB_UNRECLAIMABLE_B , + stock->nr_slab_unreclaimable_b); + stock->nr_slab_unreclaimable_b =0 ; + } + stock->index =-1 ; + stock->node_id =NUMA_NO_NODE ; +} + +static void drain_obj_stock_slot (struct obj_stock_pcp *stock,int i) +{ +struct obj_cgroup *old =READ_ONCE (stock->cached[i]);if (!old)return ; -if (stock->nr_bytes) { -unsigned int nr_pages = stock->nr_bytes >>PAGE_SHIFT ; -unsigned int nr_bytes = stock->nr_bytes & (PAGE_SIZE -1 ); +if (stock->nr_bytes[i]) { +unsigned int nr_pages = stock->nr_bytes[i] >>PAGE_SHIFT ; +unsigned int nr_bytes = stock->nr_bytes[i] & (PAGE_SIZE -1 );if (nr_pages) {struct mem_cgroup *memcg;
@@ -3289,46 +3347,43 @@ static void drain_obj_stock(struct obj_stock_pcp *stock)* so it might be changed in the future. */ atomic_add (nr_bytes, &old->nr_charged_bytes); - stock->nr_bytes =0 ; + stock->nr_bytes[i] =0 ; } -/* -* Flush the vmstat data in current stock -*/ -if (stock->nr_slab_reclaimable_b || stock->nr_slab_unreclaimable_b) { -struct pglist_data *oldpg =NODE_DATA (stock->node_id); - -if (stock->nr_slab_reclaimable_b) { -mod_objcg_mlstate (old, oldpg, -NR_SLAB_RECLAIMABLE_B , - stock->nr_slab_reclaimable_b); - stock->nr_slab_reclaimable_b =0 ; - } -if (stock->nr_slab_unreclaimable_b) { -mod_objcg_mlstate (old, oldpg, -NR_SLAB_UNRECLAIMABLE_B , - stock->nr_slab_unreclaimable_b); - stock->nr_slab_unreclaimable_b =0 ; - } - stock->node_id =NUMA_NO_NODE ; - } +/* Flush vmstat data when its owning slot is being drained. */ +if (stock->index == i) +drain_obj_stock_stats (stock); -WRITE_ONCE (stock->cached_objcg,NULL ); +WRITE_ONCE (stock->cached[i],NULL );obj_cgroup_put (old); } +static void drain_obj_stock (struct obj_stock_pcp *stock) +{ +int i; + +for (i =0 ; i <NR_OBJ_STOCK ; ++i) +drain_obj_stock_slot (stock, i); +} +static bool obj_stock_flush_required (struct obj_stock_pcp *stock,struct mem_cgroup *root_memcg) { -struct obj_cgroup *objcg =READ_ONCE (stock->cached_objcg); +struct obj_cgroup *objcg;struct mem_cgroup *memcg;bool flush = false; +int i;rcu_read_lock (); -if (objcg) { +for (i =0 ; i <NR_OBJ_STOCK ; ++i) { + objcg =READ_ONCE (stock->cached[i]); +if (!objcg) +continue ; memcg =obj_cgroup_memcg (objcg); -if (memcg &&mem_cgroup_is_descendant (memcg, root_memcg)) +if (memcg &&mem_cgroup_is_descendant (memcg, root_memcg)) { flush = true; +break ; + } }rcu_read_unlock ();
@@ -3342,6 +3397,7 @@ static void __refill_obj_stock(struct obj_cgroup *objcg, {unsigned int nr_pages =0 ;unsigned int stock_nr_bytes; +int i, slot =-1 , empty_slot =-1 ;if (!stock) { nr_pages = nr_bytes >>PAGE_SHIFT ;
@@ -3350,19 +3406,47 @@ 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 = stock->drain_idx++; +if (stock->drain_idx ==NR_OBJ_STOCK ) + stock->drain_idx =0 ; +drain_obj_stock_slot (stock, slot); + }obj_cgroup_get (objcg); +/* +* Keep the xchg result in the unsigned int local; storing +* it directly into stock->nr_bytes[slot] (uint16_t) would +* silently truncate values >= U16_MAX and bypass the flush +* guard below, leaking page-counter charges. +*/ stock_nr_bytes =atomic_read (&objcg->nr_charged_bytes) ?atomic_xchg (&objcg->nr_charged_bytes,0 ) :0 ; -WRITE_ONCE (stock->cached_objcg, objcg); +WRITE_ONCE (stock->cached[slot], objcg); allow_uncharge = true;/* Allow uncharge when objcg changes */ + }else { + stock_nr_bytes = stock->nr_bytes[slot]; } + stock_nr_bytes += nr_bytes; -/* Since stock->nr_bytes is uint16_t, don't refill >= U16_MAX */ +/* nr_bytes[] is uint16_t; flush if we would refill >= U16_MAX. */ if ((allow_uncharge && (stock_nr_bytes >PAGE_SIZE )) || stock_nr_bytes >U16_MAX ) { nr_pages = stock_nr_bytes >>PAGE_SHIFT ;
@@ -3384,7 +3468,7 @@ static void __refill_obj_stock(struct obj_cgroup *objcg, stock_nr_bytes = kept; } } - stock->nr_bytes = stock_nr_bytes; + stock->nr_bytes[slot] = stock_nr_bytes; out:if (nr_pages)
Signature · 1 line
2.53.0-Meta
Re: [PATCH v2 2/4] memcg: uint16_t for nr_bytes in obj_stock_pcp
In reply to Shakeel ButtQuoted 70 lines · Shakeel Butt
Currently struct
obj_stock_pcpstoresnr_bytesin an 'unsigned int' which is 4 bytes on 64-bit machines. Switch the field touint16_tto shrink the per-CPU cache.The kernel supports
PAGE_SIZE_4KB,_8KB,_16KB,_32KB,_64KBand_256KB(see HAVE_PAGE_SIZE_* inarch/Kconfig). After the PAGE_SIZE-aligned flush in__refill_obj_stock(), the sub-page remainder fits inuint16_tup through 64KiB pages wherePAGE_SIZE- 1 ==U16_MAX, but on 256KiB pagesPAGE_SIZE- 1 ==0x3FFFFexceedsU16_MAX. The accumulator also needs to stay withinuint16_tbetween page-aligned flushes on 64KiB pages wherePAGE_SIZEitself isU16_MAX+ 1.Accumulate the new total in an 'unsigned int' local, then:
1. Flush whenever the accumulator would hit
U16_MAX. Together with the existingallow_unchargeflush atPAGE_SIZE, this keeps theuint16_tsafe onPAGE_SIZE<= 64KiB.2. On configs with
PAGE_SHIFT> 16 (PAGE_SIZE_256KBon hexagon and powerpc 44x), push any sub-page remainder aboveU16_MAXintoobjcg->nr_charged_bytesviaatomic_addbefore storing back, so the store cannot silently truncate. ThePAGE_SHIFT> 16 guard folds the branch out at compile time on smaller page sizes.Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Tested-by kernel test robot <[email protected]>Signed-off-by Shakeel Butt <[email protected]>Reviewed-by Harry Yoo (Oracle) <[email protected]>---
Changes since v1:
- Collected tags
- Rearrange fields ofobj_stock_pcp(David Laight)
- Fix comparison operator (Harry)mm/memcontrol.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-)mm/memcontrol.c +3
-2@@ -2019,8 +2019,8 @@ static DEFINE_PER_CPU_ALIGNED(struct memcg_stock_pcp, memcg_stock) = {struct obj_stock_pcp {local_trylock_t lock; -unsigned int nr_bytes;struct obj_cgroup *cached_objcg; +uint16_t nr_bytes;int16_t node_id;int nr_slab_reclaimable_b;int nr_slab_unreclaimable_b;@@ -3331,6 +3331,7 @@ static void __refill_obj_stock(struct obj_cgroup *objcg, bool allow_uncharge) {unsigned int nr_pages =0 ; +unsigned int stock_nr_bytes;if (!stock) { nr_pages = nr_bytes >>PAGE_SHIFT ;@@ -3339,21 +3340,41 @@ 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);obj_cgroup_get (objcg); - stock->nr_bytes =atomic_read (&objcg->nr_charged_bytes)
mm/memcontrol.c static void __refill_obj_stock(struct obj_cgroup *objcg, · line 3355
Quoted 10 lines
mm/memcontrol.c +4
-1+ stock_nr_bytes =atomic_read (&objcg->nr_charged_bytes) ?atomic_xchg (&objcg->nr_charged_bytes,0 ) :0 ;WRITE_ONCE (stock->cached_objcg, objcg); allow_uncharge = true;/* Allow uncharge when objcg changes */ } - stock->nr_bytes += nr_bytes; + stock_nr_bytes += nr_bytes; + +/* Since stock->nr_bytes is uint16_t, don't refill >= U16_MAX */
^
should also be changed to: don't refill > U16_MAX ?
Otherwise:
Thanks!
Quoted 28 lines
mm/memcontrol.c +20
-3+if ((allow_uncharge && (stock_nr_bytes >PAGE_SIZE )) || + stock_nr_bytes >U16_MAX ) { + nr_pages = stock_nr_bytes >>PAGE_SHIFT ; + stock_nr_bytes &= (PAGE_SIZE -1 ); + +/* +* On configs with PAGE_SHIFT > 16 (PAGE_SIZE_256KB on +* hexagon and powerpc 44x), the sub-page remainder can +* still exceed U16_MAX. Push the excess back to +* objcg->nr_charged_bytes so the store into uint16_t +* cannot silently truncate; folded out at compile time +* on smaller page sizes. +*/ +if (PAGE_SHIFT >16 && stock_nr_bytes >U16_MAX ) { +unsigned int kept = stock_nr_bytes &U16_MAX ; -if (allow_uncharge && (stock->nr_bytes >PAGE_SIZE )) { - nr_pages = stock->nr_bytes >>PAGE_SHIFT ; - stock->nr_bytes &= (PAGE_SIZE -1 ); +atomic_add (stock_nr_bytes - kept, + &objcg->nr_charged_bytes); + stock_nr_bytes = kept; + } } + stock->nr_bytes = stock_nr_bytes; out:if (nr_pages)
Re: [PATCH v2 1/4] memcg: store node_id instead of pglist_data pointer
In reply to Shakeel ButtQuoted 14 lines · Shakeel Butt
The struct
obj_stock_pcpstores a pointer topglist_datafor the slab stats cached on the cpu. On 64-bit machines, this costs 8 bytes. The pointer is not strictly required:NODE_DATA()can recover it from the node id. Replacecached_pgdatwithint16_tnode_idand useNUMA_NO_NODEas the "no stats cached" sentinel.At the moment all the archs limit
MAX_NUMNODESto 1024 soint16_tis plenty; aBUILD_BUG_ON()makes sure we notice if that ever changes.Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Tested-by kernel test robot <[email protected]>Signed-off-by Shakeel Butt <[email protected]>Acked-by Muchun Song <[email protected]>Reviewed-by Harry Yoo (Oracle) <[email protected]>
Quoted 8 lines
---
Changes since v1:
- Added tags in the commit messagemm/memcontrol.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
Thanks!
Re: [PATCH v2 3/4] memcg: int16_t for cached slab stats
In reply to Shakeel ButtQuoted 16 lines · Shakeel Butt
Currently struct
obj_stock_pcpstores cached slab stats in 'int' which is 4 bytes per counter on 64-bit machines. Switch them toint16_tto shrink the cached metadata.The existing
PAGE_SIZEflush in__account_obj_stock()bounds *bytes atPAGE_SIZEon 4KiB and 16KiB page archs, well withinint16_t. On 64KiB pagesPAGE_SIZEis well aboveS16_MAXso that flush never fires, and a sufficiently long run of accumulations would overflow the cache. Add an explicitS16_MAXguard before each add: when the next add would pushabs(*bytes)pastS16_MAX, fold the cached value into @nr and flush directly viamod_objcg_mlstate()before the accumulation.Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Tested-by kernel test robot <[email protected]>Signed-off-by Shakeel Butt <[email protected]>Reviewed-by Harry Yoo (Oracle) <[email protected]>
Quoted 8 lines
---
Changes since v2:
- Collected tagsmm/memcontrol.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
Thanks!
Re: [PATCH v2 3/4] memcg: int16_t for cached slab stats
In reply to Shakeel ButtQuoted 16 lines · Shakeel Butt
Currently struct
obj_stock_pcpstores cached slab stats in 'int' which is 4 bytes per counter on 64-bit machines. Switch them toint16_tto shrink the cached metadata.The existing
PAGE_SIZEflush in__account_obj_stock()bounds *bytes atPAGE_SIZEon 4KiB and 16KiB page archs, well withinint16_t. On 64KiB pagesPAGE_SIZEis well aboveS16_MAXso that flush never fires, and a sufficiently long run of accumulations would overflow the cache. Add an explicitS16_MAXguard before each add: when the next add would pushabs(*bytes)pastS16_MAX, fold the cached value into @nr and flush directly viamod_objcg_mlstate()before the accumulation.
Quoted 4 lines
Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Tested-by kernel test robot <[email protected]>Signed-off-by Shakeel Butt <[email protected]>Reviewed-by Harry Yoo (Oracle) <[email protected]>
Thanks.
Re: [PATCH v2 2/4] memcg: uint16_t for nr_bytes in obj_stock_pcp
In reply to Shakeel ButtQuoted 29 lines · Shakeel Butt
Currently struct
obj_stock_pcpstoresnr_bytesin an 'unsigned int' which is 4 bytes on 64-bit machines. Switch the field touint16_tto shrink the per-CPU cache.The kernel supports
PAGE_SIZE_4KB,_8KB,_16KB,_32KB,_64KBand_256KB(see HAVE_PAGE_SIZE_* inarch/Kconfig). After the PAGE_SIZE-aligned flush in__refill_obj_stock(), the sub-page remainder fits inuint16_tup through 64KiB pages wherePAGE_SIZE- 1 ==U16_MAX, but on 256KiB pagesPAGE_SIZE- 1 ==0x3FFFFexceedsU16_MAX. The accumulator also needs to stay withinuint16_tbetween page-aligned flushes on 64KiB pages wherePAGE_SIZEitself isU16_MAX+ 1.Accumulate the new total in an 'unsigned int' local, then:
1. Flush whenever the accumulator would hit
U16_MAX. Together with the existingallow_unchargeflush atPAGE_SIZE, this keeps theuint16_tsafe onPAGE_SIZE<= 64KiB.2. On configs with
PAGE_SHIFT> 16 (PAGE_SIZE_256KBon hexagon and powerpc 44x), push any sub-page remainder aboveU16_MAXintoobjcg->nr_charged_bytesviaatomic_addbefore storing back, so the store cannot silently truncate. ThePAGE_SHIFT> 16 guard folds the branch out at compile time on smaller page sizes.
Quoted 4 lines
Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Tested-by kernel test robot <[email protected]>Signed-off-by Shakeel Butt <[email protected]>Reviewed-by Harry Yoo (Oracle) <[email protected]>
Thanks.
Muchun Song
reviewerRe: [PATCH v2 4/4] memcg: multi objcg charge support
In reply to Shakeel ButtQuoted 51 lines · Shakeel Butt
Commit
01b9da291c49("mm: memcontrol: convert objcg to be per-memcg per-node type") split a memcg's singleobj_cgroupinto one per NUMA node so that reparenting LRU folios can take per-node lru locks. As a side effect, the per-CPUobj_stock_pcp-- which caches exactly onecached_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: turnnr_bytesandcached_objcginto 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_tindex records which slot the cached slab stats belong to; the stats are flushed on slot or pgdat change. WithNR_OBJ_STOCK= 5 the layout (verified with pahole) is:offset 0 : lock(1) + index(1) +
node_id(2)+ slab stats(4) = 8Boffset 8 : nr_bytes[5] = 10B offset 18 : padding = 6B offset 24 : cached[5] = 40Boffset 64 : (line 2)
work_struct+ flags (cold)so
consume_obj_stock,refill_obj_stockand the slab account path each touch exactly one 64-byte cache line on non-debug 64-bit builds.Reported-by kernel test robot <[email protected]>Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Signed-off-by Shakeel Butt <[email protected]>Tested-by kernel test robot <[email protected]>---
Changes since v1:
- Use round robin for drain
mm/memcontrol.c| 188 ++++++++++++++++++++++++++++++++++--------------1 file changed, 136 insertions(+), 52 deletions(-)mm/memcontrol.c +0
-0@@ -150,14 +150,14 @@ static void obj_cgroup_release(struct percpu_ref *ref)* However, it can be PAGE_SIZE or (x * PAGE_SIZE).
mm/memcontrol.c static void obj_cgroup_release(struct percpu_ref *ref) · line 159
Quoted 10 lines
mm/memcontrol.c +1
-1* * 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,
^
4Since 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 :)
mm/memcontrol.c static void obj_cgroup_release(struct percpu_ref *ref) · line 161
Quoted 3 lines
mm/memcontrol.c +1
-1-* 92 bytes are added to stock->nr_bytes +* 92 bytes are added to stock->nr_bytes[i] * 6) CPU0: stock is flushed,
^
5Thanks,
Muchun
Quoted 1 line
mm/memcontrol.c +0
-0* 92 bytes are added to objcg->nr_charged_bytes
Re: [PATCH v2 3/4] memcg: int16_t for cached slab stats
In reply to Shakeel ButtQuoted 52 lines · On Thu, 21 May 2026 18:19:07 -0700 Shakeel Butt <[email protected]> wrote:
Currently struct
obj_stock_pcpstores cached slab stats in 'int' which is 4 bytes per counter on 64-bit machines. Switch them toint16_tto shrink the cached metadata.The existing
PAGE_SIZEflush in__account_obj_stock()bounds *bytes atPAGE_SIZEon 4KiB and 16KiB page archs, well withinint16_t. On 64KiB pagesPAGE_SIZEis well aboveS16_MAXso that flush never fires, and a sufficiently long run of accumulations would overflow the cache. Add an explicitS16_MAXguard before each add: when the next add would pushabs(*bytes)pastS16_MAX, fold the cached value into @nr and flush directly viamod_objcg_mlstate()before the accumulation.Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Tested-by kernel test robot <[email protected]>Signed-off-by Shakeel Butt <[email protected]>Reviewed-by Harry Yoo (Oracle) <[email protected]>---
Changes since v2:
- Collected tagsmm/memcontrol.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)mm/memcontrol.c +3
-3@@ -2022,8 +2022,8 @@ struct obj_stock_pcp {struct obj_cgroup *cached_objcg;uint16_t nr_bytes;int16_t node_id; -int nr_slab_reclaimable_b; -int nr_slab_unreclaimable_b; +int16_t nr_slab_reclaimable_b; +int16_t nr_slab_unreclaimable_b;struct work_struct work;unsigned long flags;@@ -3158,7 +3158,7 @@ static void __account_obj_stock(struct obj_cgroup *objcg, struct obj_stock_pcp *stock, int nr, struct pglist_data *pgdat, enum node_stat_item idx) { - int *bytes; + int16_t *bytes;/* * Though at the moment MAX_NUMNODES <= 1024 in all archs but let's make @@ -3195,6 +3195,16 @@ static void __account_obj_stock(struct obj_cgroup *objcg, bytes = (idx ==NR_SLAB_RECLAIMABLE_B ) ? &stock->nr_slab_reclaimable_b : &stock->nr_slab_unreclaimable_b;
mm/memcontrol.c static void __account_obj_stock(struct obj_cgroup *objcg, · line 3207
Quoted 10 lines
mm/memcontrol.c +10
-0+/* +* To avoid overflow or underflow, flush directly if accumulating @nr +* would push the cached value past S16_MAX. +*/ +if (abs (nr + *bytes) >S16_MAX ) { + nr += *bytes; + *bytes =0 ; + goto direct; + } +
I think you should do the add first:
nr += *bytes;
if (abs(nr) < S16_MAX && (!*bytes || abs(nr) < PAGE_SIZE)) {
*bytes = nr;
} else {
*bytes = 0;
mod_objcg_mlstate(objcg, pgdat, idx, nr);
}-- David
Quoted 3 lines
mm/memcontrol.c +0
-0/* * Even for large object >= PAGE_SIZE, the vmstat data will still be * cached locally at least once before pushing it out.
Shakeel Butt
maintainerRe: [PATCH v2 2/4] memcg: uint16_t for nr_bytes in obj_stock_pcp
In reply to Qi ZhengQuoted 82 lines · Qi Zheng
Quoted 66 lines · Shakeel Butt
Currently struct
obj_stock_pcpstoresnr_bytesin an 'unsigned int' which is 4 bytes on 64-bit machines. Switch the field touint16_tto shrink the per-CPU cache.The kernel supports
PAGE_SIZE_4KB,_8KB,_16KB,_32KB,_64KBand_256KB(see HAVE_PAGE_SIZE_* inarch/Kconfig). After the PAGE_SIZE-aligned flush in__refill_obj_stock(), the sub-page remainder fits inuint16_tup through 64KiB pages wherePAGE_SIZE- 1 ==U16_MAX, but on 256KiB pagesPAGE_SIZE- 1 ==0x3FFFFexceedsU16_MAX. The accumulator also needs to stay withinuint16_tbetween page-aligned flushes on 64KiB pages wherePAGE_SIZEitself isU16_MAX+ 1.Accumulate the new total in an 'unsigned int' local, then:
1. Flush whenever the accumulator would hit
U16_MAX. Together with the existingallow_unchargeflush atPAGE_SIZE, this keeps theuint16_tsafe onPAGE_SIZE<= 64KiB.2. On configs with
PAGE_SHIFT> 16 (PAGE_SIZE_256KBon hexagon and powerpc 44x), push any sub-page remainder aboveU16_MAXintoobjcg->nr_charged_bytesviaatomic_addbefore storing back, so the store cannot silently truncate. ThePAGE_SHIFT> 16 guard folds the branch out at compile time on smaller page sizes.Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Tested-by kernel test robot <[email protected]>Signed-off-by Shakeel Butt <[email protected]>Reviewed-by Harry Yoo (Oracle) <[email protected]>---
Changes since v1:
- Collected tags
- Rearrange fields ofobj_stock_pcp(David Laight)
- Fix comparison operator (Harry)mm/memcontrol.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-)mm/memcontrol.c +3
-1@@ -2019,8 +2019,8 @@ static DEFINE_PER_CPU_ALIGNED(struct memcg_stock_pcp, memcg_stock) = {struct obj_stock_pcp {local_trylock_t lock; -unsigned int nr_bytes;struct obj_cgroup *cached_objcg; +uint16_t nr_bytes;int16_t node_id;int nr_slab_reclaimable_b;int nr_slab_unreclaimable_b;@@ -3331,6 +3331,7 @@ static void __refill_obj_stock(struct obj_cgroup *objcg, bool allow_uncharge) {unsigned int nr_pages =0 ; +unsigned int stock_nr_bytes;if (!stock) { nr_pages = nr_bytes >>PAGE_SHIFT ;@@ -3339,21 +3340,41 @@ 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);obj_cgroup_get (objcg);mm/memcontrol.c static void __refill_obj_stock(struct obj_cgroup *objcg, · line 3353
Quoted 10 lines
mm/memcontrol.c +4
-2- stock->nr_bytes =atomic_read (&objcg->nr_charged_bytes) + stock_nr_bytes =atomic_read (&objcg->nr_charged_bytes) ?atomic_xchg (&objcg->nr_charged_bytes,0 ) :0 ;WRITE_ONCE (stock->cached_objcg, objcg); allow_uncharge = true;/* Allow uncharge when objcg changes */ } - stock->nr_bytes += nr_bytes; + stock_nr_bytes += nr_bytes; + +/* Since stock->nr_bytes is uint16_t, don't refill >= U16_MAX */ ^
Quoted 5 lines
should also be changed to: don't refill >
U16_MAX?Otherwise:
Acked-by Qi Zheng <[email protected]>
Thanks. If I send a new version, I will fix this otherwise I will ask Andrew to fix this inplace.
Shakeel Butt
maintainerRe: [PATCH v2 4/4] memcg: multi objcg charge support
In reply to Muchun SongQuoted 64 lines · Muchun Song
Quoted 51 lines · Shakeel Butt
Commit
01b9da291c49("mm: memcontrol: convert objcg to be per-memcg per-node type") split a memcg's singleobj_cgroupinto one per NUMA node so that reparenting LRU folios can take per-node lru locks. As a side effect, the per-CPUobj_stock_pcp-- which caches exactly onecached_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: turnnr_bytesandcached_objcginto 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_tindex records which slot the cached slab stats belong to; the stats are flushed on slot or pgdat change. WithNR_OBJ_STOCK= 5 the layout (verified with pahole) is:offset 0 : lock(1) + index(1) +
node_id(2)+ slab stats(4) = 8Boffset 8 : nr_bytes[5] = 10B offset 18 : padding = 6B offset 24 : cached[5] = 40Boffset 64 : (line 2)
work_struct+ flags (cold)so
consume_obj_stock,refill_obj_stockand the slab account path each touch exactly one 64-byte cache line on non-debug 64-bit builds.Reported-by kernel test robot <[email protected]>Fixes 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")Signed-off-by Shakeel Butt <[email protected]>Tested-by kernel test robot <[email protected]>---
Changes since v1:
- Use round robin for drain
mm/memcontrol.c| 188 ++++++++++++++++++++++++++++++++++--------------1 file changed, 136 insertions(+), 52 deletions(-)mm/memcontrol.c +0
-0@@ -150,14 +150,14 @@ static void obj_cgroup_release(struct percpu_ref *ref)* However, it can be PAGE_SIZE or (x * PAGE_SIZE). mm/memcontrol.c static void obj_cgroup_release(struct percpu_ref *ref) · line 159
Quoted 10 lines
mm/memcontrol.c +1
-1* * 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
Quoted 3 lines
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.
Andrew Morton
maintainerQuoted 17 lines · On Thu, 21 May 2026 18:19:04 -0700 Shakeel Butt <[email protected]> wrote:
Commit
01b9da291c49("mm: memcontrol: convert objcg to be per-memcg per-node type") split a memcg's singleobj_cgroupinto one per NUMA node so that reparenting LRU folios can take per-node lru locks. As a side effect, the per-CPUobj_stock_pcp-- which caches a singlecached_objcgpointer -- 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.Commit
d0211878ce06("memcg: cacheobj_stockby memcg, not by objcg pointer") landed as a temporary fix by treating sibling per-node objcgs as equivalent for the cache lookup, intended to be reverted once per-node kmem accounting is introduced. This series takes a more general approach: cache multiple objcgs per CPU using the multi-slot patternmemcg_stock_pcpalready uses, so the per-node objcg variants of one memcg can all coexist in the stock without ever forcing a drain. The temporary fix can then be reverted.
Quoted 6 lines
To avoid increasing the per-CPU cache footprint, the first three patches shrink the existing single-slot
obj_stock_pcpfields. The final patch convertscached_objcgandnr_bytesinto NR_OBJ_STOCK=5 slot arrays and reorders the struct so the entire consume/refill/account hot path fits within a single 64-byte cache line on non-debug 64-bit builds (verified with pahole).
Thanks, I added this to mm.git's mm-new branch, along with a couple of possible todo notes from the review.
Sashiko asked a thing:
https://sashiko.dev/#/patchset/[email protected]
Did you already see this? The footers there indicate that an email was sent out but I don't know if it works?
Shakeel Butt
maintainerQuoted 26 lines · Andrew Morton
Quoted 17 lines · On Thu, 21 May 2026 18:19:04 -0700 Shakeel Butt <[email protected]> wrote:
Commit
01b9da291c49("mm: memcontrol: convert objcg to be per-memcg per-node type") split a memcg's singleobj_cgroupinto one per NUMA node so that reparenting LRU folios can take per-node lru locks. As a side effect, the per-CPUobj_stock_pcp-- which caches a singlecached_objcgpointer -- 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.Commit
d0211878ce06("memcg: cacheobj_stockby memcg, not by objcg pointer") landed as a temporary fix by treating sibling per-node objcgs as equivalent for the cache lookup, intended to be reverted once per-node kmem accounting is introduced. This series takes a more general approach: cache multiple objcgs per CPU using the multi-slot patternmemcg_stock_pcpalready uses, so the per-node objcg variants of one memcg can all coexist in the stock without ever forcing a drain. The temporary fix can then be reverted.Quoted 6 lines
To avoid increasing the per-CPU cache footprint, the first three patches shrink the existing single-slot
obj_stock_pcpfields. The final patch convertscached_objcgandnr_bytesinto NR_OBJ_STOCK=5 slot arrays and reorders the struct so the entire consume/refill/account hot path fits within a single 64-byte cache line on non-debug 64-bit builds (verified with pahole).
Quoted 8 lines
Thanks, I added this to mm.git's mm-new branch, along with a couple of possible todo notes from the review.
Sashiko asked a thing:
https://sashiko.dev/#/patchset/[email protected]Did you already see this? The footers there indicate that an email was sent out but I don't know if it works?
Yes, I saw that comment. It is kind of very specific to archs with 256KiB base page sizes. Anyways, I have a simple fix for that and there were minor suggestions from others for simple changes, I will send v3 with the requested changes.