* + memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch added to mm-hotfixes-unstable branch
@ 2026-05-18 1:27 Andrew Morton
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2026-05-18 1:27 UTC (permalink / raw)
To: mm-commits, roman.gushchin, qi.zheng, oliver.sang, muchun.song,
mhocko, hannes, shakeel.butt, akpm
The patch titled
Subject: memcg: cache obj_stock by memcg, not by objcg pointer
has been added to the -mm mm-hotfixes-unstable branch. Its filename is
memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch
This patch will later appear in the mm-hotfixes-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via various
branches at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there most days
------------------------------------------------------
From: Shakeel Butt <shakeel.butt@linux.dev>
Subject: memcg: cache obj_stock by memcg, not by objcg pointer
Date: Sun, 17 May 2026 12:43:08 -0700
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. Treat
the cache as keyed by memcg in __consume_obj_stock() and
__refill_obj_stock() so siblings share the reserve. Compare via
READ_ONCE(objcg->memcg) directly: pointer-compare only, no deref, so the
rcu_read_lock contract on obj_cgroup_memcg() does not apply.
In the same-memcg refill path also fold the incoming objcg's
nr_charged_bytes into the stock; otherwise sub-page residue accumulates on
whichever sibling was cached at drain time and obj_cgroup_release()
silently drops it, leaking up to nr_node_ids * (PAGE_SIZE - 1) bytes per
memcg lifecycle from the page_counter. This issue was reported by
Sashiko.
Update the now-stale invariant comment on __account_obj_stock().
Qi Zheng built a specialized reproducer [1] for the corner case and
confirmed the fix.
Link: https://lore.kernel.org/20260517194308.952655-1-shakeel.butt@linux.dev
Fixes: 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202605121641.b6a60cb0-lkp@intel.com
Link: https://lore.kernel.org/19693be6-7132-446e-b3fc-b7e9f56e5949@linux.dev/ [1]
Debugged-by: Qi Zheng <qi.zheng@linux.dev>
Tested-by: Qi Zheng <qi.zheng@linux.dev>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Muchun Song <muchun.song@linux.dev>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
mm/memcontrol.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
--- a/mm/memcontrol.c~memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer
+++ a/mm/memcontrol.c
@@ -3152,7 +3152,12 @@ static void unlock_stock(struct obj_stoc
local_unlock(&obj_stock.lock);
}
-/* Call after __refill_obj_stock() to ensure stock->cached_objg == objcg */
+/*
+ * Call after __consume_obj_stock() / __refill_obj_stock(). The stock may be
+ * cached for a sibling per-node objcg of the same memcg; in that case the
+ * vmstat batching slot does not match objcg and we fall through to the
+ * direct path.
+ */
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)
@@ -3210,7 +3215,11 @@ static bool __consume_obj_stock(struct o
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);
+
+ /* Sibling per-node objcgs share the reserve. */
+ if ((cached == objcg ||
+ (cached && READ_ONCE(cached->memcg) == READ_ONCE(objcg->memcg))) &&
stock->nr_bytes >= nr_bytes) {
stock->nr_bytes -= nr_bytes;
return true;
@@ -3318,6 +3327,7 @@ static void __refill_obj_stock(struct ob
unsigned int nr_bytes,
bool allow_uncharge)
{
+ struct obj_cgroup *cached;
unsigned int nr_pages = 0;
if (!stock) {
@@ -3327,7 +3337,11 @@ static void __refill_obj_stock(struct ob
goto out;
}
- if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */
+ cached = READ_ONCE(stock->cached_objcg);
+ if (cached == objcg)
+ goto add_bytes;
+ /* Direct READ_ONCE due to just pointer comparison. */
+ if (!cached || READ_ONCE(cached->memcg) != READ_ONCE(objcg->memcg)) {
drain_obj_stock(stock);
obj_cgroup_get(objcg);
stock->nr_bytes = atomic_read(&objcg->nr_charged_bytes)
@@ -3335,7 +3349,12 @@ static void __refill_obj_stock(struct ob
WRITE_ONCE(stock->cached_objcg, objcg);
allow_uncharge = true; /* Allow uncharge when objcg changes */
+ } else if (atomic_read(&objcg->nr_charged_bytes)) {
+ /* Fold sibling's stranded ncb into stock; else release leaks it. */
+ stock->nr_bytes += atomic_xchg(&objcg->nr_charged_bytes, 0);
+ allow_uncharge = true;
}
+add_bytes:
stock->nr_bytes += nr_bytes;
if (allow_uncharge && (stock->nr_bytes > PAGE_SIZE)) {
_
Patches currently in -mm which might be from shakeel.butt@linux.dev are
memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch
^ permalink raw reply [flat|nested] 2+ messages in thread
* + memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch added to mm-hotfixes-unstable branch
@ 2026-05-18 22:41 Andrew Morton
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2026-05-18 22:41 UTC (permalink / raw)
To: mm-commits, roman.gushchin, qi.zheng, oliver.sang, muchun.song,
mhocko, joshua.hahnjy, hannes, alex, shakeel.butt, akpm
The patch titled
Subject: memcg: cache obj_stock by memcg, not by objcg pointer
has been added to the -mm mm-hotfixes-unstable branch. Its filename is
memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch
This patch will later appear in the mm-hotfixes-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via various
branches at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there most days
------------------------------------------------------
From: Shakeel Butt <shakeel.butt@linux.dev>
Subject: memcg: cache obj_stock by memcg, not by objcg pointer
Date: Mon, 18 May 2026 15:28:27 -0700
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. Treat
the cache as keyed by memcg in __consume_obj_stock() and
__refill_obj_stock() so siblings share the reserve. Compare via
READ_ONCE(objcg->memcg) directly: pointer-compare only, no deref, so the
rcu_read_lock contract on obj_cgroup_memcg() does not apply.
Sharing the reserve without re-caching means bytes funded by one per-node
objcg's slow path can be consumed/freed under a different sibling, leaving
sub-page residue on whichever sibling was cached at drain time. The
pre-existing obj_cgroup_release() path would WARN and silently drop that
residue, leaking up to nr_node_ids * (PAGE_SIZE - 1) bytes per memcg
lifecycle from the page_counter. Forward the residue into a per-node
objcg of the same (post-reparent) memcg at release time instead, so it can
be reconciled later via a refill atomic_xchg or another release; the chain
terminates at root_mem_cgroup, whose page_counter has no enforced limit.
Please note that this is temporary fix and will be reverted when per-node
kmem accounting is introduced.
Update the stale invariant comment on __account_obj_stock().
Qi Zheng built a specialized reproducer [1] for the corner case and
confirmed the fix.
Link: https://lore.kernel.org/20260518222827.110696-1-shakeel.butt@linux.dev
Fixes: 01b9da291c49 ("mm: memcontrol: convert objcg to be per-memcg per-node type")
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202605121641.b6a60cb0-lkp@intel.com
Link: https://lore.kernel.org/19693be6-7132-446e-b3fc-b7e9f56e5949@linux.dev/ [1]
Debugged-by: Qi Zheng <qi.zheng@linux.dev>
Cc: Alexandre Ghiti <alex@ghiti.fr>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Joshua Hahn <joshua.hahnjy@gmail.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Muchun Song <muchun.song@linux.dev>
Cc: Roman Gushchin <roman.gushchin@linux.dev>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
mm/memcontrol.c | 69 ++++++++++++++++++++++++++++++++++------------
1 file changed, 51 insertions(+), 18 deletions(-)
--- a/mm/memcontrol.c~memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer
+++ a/mm/memcontrol.c
@@ -142,14 +142,24 @@ static void obj_cgroup_release(struct pe
struct obj_cgroup *objcg = container_of(ref, struct obj_cgroup, refcnt);
unsigned int nr_bytes;
unsigned int nr_pages;
+ unsigned int sub_bytes;
unsigned long flags;
/*
- * At this point all allocated objects are freed, and
- * objcg->nr_charged_bytes can't have an arbitrary byte value.
- * However, it can be PAGE_SIZE or (x * PAGE_SIZE).
+ * At this point all allocated objects are freed, but
+ * objcg->nr_charged_bytes can still hold either
+ * - (x * PAGE_SIZE) if a small-alloc/drain race left whole pages
+ * stranded (see the historical sequence below), or
+ * - any sub-page residue, now that the stock is keyed by memcg and
+ * sibling per-node objcgs share its reserve: bytes consumed by
+ * one sibling can spill into another sibling's nr_charged_bytes
+ * when the stock is drained.
*
- * The following sequence can lead to it:
+ * Uncharge the page-aligned portion from this objcg's (post-reparent)
+ * memcg, and forward any sub-page residue into a per-node objcg of
+ * the same memcg so it can be reconciled later instead of being lost.
+ *
+ * Historical race producing the (x * PAGE_SIZE) case:
* 1) CPU0: objcg == stock->cached_objcg
* 2) CPU1: we do a small allocation (e.g. 92 bytes),
* PAGE_SIZE bytes are charged
@@ -160,23 +170,33 @@ static void obj_cgroup_release(struct pe
* 92 bytes are added to stock->nr_bytes
* 6) CPU0: stock is flushed,
* 92 bytes are added to objcg->nr_charged_bytes
- *
- * In the result, nr_charged_bytes == PAGE_SIZE.
- * This page will be uncharged in obj_cgroup_release().
*/
nr_bytes = atomic_read(&objcg->nr_charged_bytes);
- WARN_ON_ONCE(nr_bytes & (PAGE_SIZE - 1));
nr_pages = nr_bytes >> PAGE_SHIFT;
+ sub_bytes = nr_bytes & (PAGE_SIZE - 1);
- if (nr_pages) {
+ if (nr_pages || sub_bytes) {
struct mem_cgroup *memcg;
- memcg = get_mem_cgroup_from_objcg(objcg);
- mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages);
- memcg1_account_kmem(memcg, -nr_pages);
- if (!mem_cgroup_is_root(memcg))
- memcg_uncharge(memcg, nr_pages);
- mem_cgroup_put(memcg);
+ rcu_read_lock();
+ memcg = obj_cgroup_memcg(objcg);
+
+ if (nr_pages) {
+ mod_memcg_state(memcg, MEMCG_KMEM, -nr_pages);
+ memcg1_account_kmem(memcg, -nr_pages);
+ if (!mem_cgroup_is_root(memcg))
+ memcg_uncharge(memcg, nr_pages);
+ }
+
+ if (sub_bytes && !mem_cgroup_is_root(memcg)) {
+ struct obj_cgroup *fwd;
+
+ fwd = rcu_dereference(
+ memcg->nodeinfo[numa_node_id()]->objcg);
+ if (fwd)
+ atomic_add(sub_bytes, &fwd->nr_charged_bytes);
+ }
+ rcu_read_unlock();
}
spin_lock_irqsave(&objcg_lock, flags);
@@ -3152,7 +3172,12 @@ static void unlock_stock(struct obj_stoc
local_unlock(&obj_stock.lock);
}
-/* Call after __refill_obj_stock() to ensure stock->cached_objg == objcg */
+/*
+ * Call after __consume_obj_stock() / __refill_obj_stock(). The stock may be
+ * cached for a sibling per-node objcg of the same memcg; in that case the
+ * vmstat batching slot does not match objcg and we fallthrough to the
+ * direct path.
+ */
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)
@@ -3210,7 +3235,11 @@ static bool __consume_obj_stock(struct o
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);
+
+ /* Sibling per-node objcgs share the reserve. */
+ if ((cached == objcg ||
+ (cached && READ_ONCE(cached->memcg) == READ_ONCE(objcg->memcg))) &&
stock->nr_bytes >= nr_bytes) {
stock->nr_bytes -= nr_bytes;
return true;
@@ -3318,6 +3347,7 @@ static void __refill_obj_stock(struct ob
unsigned int nr_bytes,
bool allow_uncharge)
{
+ struct obj_cgroup *cached;
unsigned int nr_pages = 0;
if (!stock) {
@@ -3327,7 +3357,10 @@ static void __refill_obj_stock(struct ob
goto out;
}
- if (READ_ONCE(stock->cached_objcg) != objcg) { /* reset if necessary */
+ cached = READ_ONCE(stock->cached_objcg);
+ /* Direct READ_ONCE due to just pointer comparison. */
+ if (cached != objcg &&
+ (!cached || READ_ONCE(cached->memcg) != READ_ONCE(objcg->memcg))) {
drain_obj_stock(stock);
obj_cgroup_get(objcg);
stock->nr_bytes = atomic_read(&objcg->nr_charged_bytes)
_
Patches currently in -mm which might be from shakeel.butt@linux.dev are
memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-18 22:41 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-18 1:27 + memcg-cache-obj_stock-by-memcg-not-by-objcg-pointer.patch added to mm-hotfixes-unstable branch Andrew Morton
-- strict thread matches above, loose matches on Subject: below --
2026-05-18 22:41 Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.