memcg: shrink obj_stock_pcp and cache multiple objcgs
Subsystems (3)
Shakeel Butt
maintainerv3 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 v2:
http://lore.kernel.org/[email protected]
- Fix comments (Muchun & Qi)
- Simplify code (David Laight)
- Fix handling of archs with base page size larger than 256 KiB (Sashiko)
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 v3 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 & v2:
- Added tags in the commit message
mm/memcontrol.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)
mm/memcontrol.c +19 -7
@@ -2022,7 +2022,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;
@@ -2032,6 +2032,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);
@@ -3162,6 +3163,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;
@@ -3169,9 +3177,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,
@@ -3183,7 +3193,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
@@ -3279,19 +3289,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 v3 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 on PAGE_SHIFT <= 16 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 configs with PAGE_SHIFT > 16 (PAGE_SIZE_256KB on hexagon and powerpc 44x, both 32-bit), uint16_t cannot represent the sub-page remainder. Define obj_stock_bytes_t as 'unsigned int' on those archs so nr_bytes can hold the full remainder and the normal page-boundary flush in __refill_obj_stock() and the page extraction in drain_obj_stock() both work correctly.
The single-cache-line layout target only applies to PAGE_SHIFT <= 16; those archs are 32-bit embedded and not the optimization target.
---
Changes since v2:
- Based on Sashiko's concern regarding archs with 256KiB base pages, added special handling for such arch and simplify the code overall.
- Updated commit message.
- Have kept the review tags as overall code remains same mostly.
Changes since v1:
- Collected tags
- Rearrange fields of obj_stock_pcp (David Laight)
- Fix comparison operator (Harry)
mm/memcontrol.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)
mm/memcontrol.c +19 -6
@@ -2020,8 +2020,17 @@ 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; +#if PAGE_SHIFT >16 +/* +* On rare archs with 256KiB base page size (hexagon and powerpc 44x) +* keep nr_bytes to unsigned int as uint16_t cannot represent the full +* sub-page remainder. +*/ +unsigned int nr_bytes; +#else +uint16_t nr_bytes; +#endif int16_t node_id;int nr_slab_reclaimable_b;int nr_slab_unreclaimable_b;
@@ -3334,6 +3343,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 ;
@@ -3342,21 +3352,24 @@ 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; -if (allow_uncharge && (stock->nr_bytes >PAGE_SIZE )) { - nr_pages = stock->nr_bytes >>PAGE_SHIFT ; - stock->nr_bytes &= (PAGE_SIZE -1 ); +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 ); } + stock->nr_bytes = stock_nr_bytes; out:if (nr_pages)
Signature · 1 line
2.53.0-Meta
Shakeel Butt
maintainerPATCH v3 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:
- Simplify code based on David Laight's suggestion.
- Collected tags
Changes since v1:
- Collected tags
mm/memcontrol.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
mm/memcontrol.c +12 -13
@@ -2035,8 +2035,8 @@ struct obj_stock_pcp {struct obj_cgroup *cached_objcg;obj_stock_bytes_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;
@@ -3173,7 +3173,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
@@ -3210,21 +3210,20 @@ 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; +/* -* Even for large object >= PAGE_SIZE, the vmstat data will still be -* cached locally at least once before pushing it out. +* Fold @nr into the cached value and decide whether to keep it cached +* or flush it directly. Cache the combined value when it fits in the +* int16_t storage and either the cache was empty (so even a value +* above PAGE_SIZE gets a chance to be canceled by a paired delta) or +* the combined value is within the PAGE_SIZE flush threshold. */ -if (!*bytes) { + nr += *bytes; +if (abs (nr) <=S16_MAX && (!*bytes ||abs (nr) <=PAGE_SIZE )) { *bytes = nr; nr =0 ; }else { - *bytes += nr; -if (abs (*bytes) >PAGE_SIZE ) { - nr = *bytes; - *bytes =0 ; - }else { - nr =0 ; - } + *bytes =0 ; } direct:if (nr)
Signature · 1 line
2.53.0-Meta
Shakeel Butt
maintainerPATCH v3 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 v2:
- fix comments (Muchun)
Changes since v1:
- Use round robin for drain
mm/memcontrol.c | 200 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 142 insertions(+), 58 deletions(-)
mm/memcontrol.c +142 -58
@@ -150,15 +150,15 @@ 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 -* 6) CPU0: stock is flushed, +* 4) CPU0: we do release this object, +* 92 bytes are added to stock->nr_bytes[i] +* 5) CPU0: stock is flushed, * 92 bytes are added to objcg->nr_charged_bytes * * In the result, nr_charged_bytes == PAGE_SIZE.
@@ -2018,34 +2018,49 @@ 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; +int8_t index; +int16_t node_id; +int16_t nr_slab_reclaimable_b; +int16_t nr_slab_unreclaimable_b;#if PAGE_SHIFT >16 /* * On rare archs with 256KiB base page size (hexagon and powerpc 44x) * keep nr_bytes to unsinged int as uint16_t cannot represent the full -* sub-page remainder. +* sub-page remainder. Such archs are not cacheline optimization target. */ -unsigned int nr_bytes; +unsigned int nr_bytes[NR_OBJ_STOCK ];#else -uint16_t nr_bytes; +uint16_t nr_bytes[NR_OBJ_STOCK ];#endif -int16_t node_id; -int16_t nr_slab_reclaimable_b; -int16_t nr_slab_unreclaimable_b; +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);
@@ -3165,12 +3180,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
@@ -3179,29 +3195,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; }
@@ -3231,10 +3257,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;
@@ -3255,16 +3287,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; +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_objcg); +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;
@@ -3290,46 +3348,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); +/* Flush vmstat data when its owning slot is being drained. */ +if (stock->index == i) +drain_obj_stock_stats (stock); -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 ; - } - -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 ();
@@ -3343,6 +3398,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 ;
@@ -3351,16 +3407,44 @@ 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; if ((allow_uncharge && (stock_nr_bytes > PAGE_SIZE)) ||
@@ -3368,7 +3452,7 @@ static void __refill_obj_stock(struct obj_cgroup *objcg, nr_pages = stock_nr_bytes >>PAGE_SHIFT ; stock_nr_bytes &= (PAGE_SIZE -1 ); } - 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 v3 4/4] memcg: multi objcg charge support
Quoted 8 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.
Quoted 6 lines
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.
nit: evict a random slot -> evict a slot round-robin
Quoted 13 lines
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.
Quoted 6 lines
Reported-by kernel test robot <[email protected]>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]>---
Looks good to me,
Signature · 2 lines
Cheers, Harry / Hyeonggon