* [PATCH v2] memcg: keep folio's objcg same as its node
@ 2026-08-06 16:58 Shakeel Butt
2026-08-06 18:15 ` Johannes Weiner
0 siblings, 1 reply; 4+ messages in thread
From: Shakeel Butt @ 2026-08-06 16:58 UTC (permalink / raw)
To: Andrew Morton
Cc: Michal Hocko, Johannes Weiner, Roman Gushchin, Muchun Song,
Qi Zheng, Meta kernel team, linux-mm, cgroups, linux-kernel,
Karl Erik Hofseth, stable
memcg_reparent_objcgs() has an inherent assumption that a folio's objcg
is the objcg of the folio's node. Folio migration across nodes breaks
that assumption: the new folio simply inherits the old folio's objcg
while living on a different node.
Once the assumption is broken, the reparenting of the folio's objcg and
the reparenting of the folio's LRU list are no longer atomic.
memcg_reparent_objcgs() handles one node per iteration and drops all the
locks in between, so the objcg gets reparented in the iteration for the
objcg's node while the LRU list gets spliced in the iteration for the
folio's node. Any LRU operation on that folio in between resolves its
lruvec through the objcg, and thus takes the lru_lock of the wrong
memcg, not the lru_lock of the list the folio is actually on.
Fix this by selecting the objcg by folio_nid() at charge time, and by
re-deriving it for the destination node in mem_cgroup_migrate() and
mem_cgroup_replace_folio().
Reported-by: Karl Erik Hofseth <karl.e.hofseth@opoint.com>
Closes: https://lore.kernel.org/all/anMmd1ADrDVwMO6v@work/
Fixes: f1cf8d2f36dc ("mm: memcontrol: eliminate the problem of dying memory cgroup for LRU folios")
Cc: stable@vger.kernel.org
Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
---
Changes since v1: http://lore.kernel.org/20260806061830.3294679-1-shakeel.butt@linux.dev
- In mem_cgroup_migrate, do obj_cgroup_put at the end (Sashiko)
- Handle scenario where destination node has been reparented to the root but the
source node's objcg has not yet (Sashiko)
- Add comment explaining the race between migration and reparenting (Johannes)
mm/memcontrol.c | 66 ++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 54 insertions(+), 12 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3057396dda53..02c108cdd0f5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2966,10 +2966,9 @@ struct mem_cgroup *mem_cgroup_from_virt(void *p)
return folio_memcg_check(virt_to_folio(p));
}
-static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
+static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg,
+ int nid)
{
- int nid = numa_node_id();
-
for (; memcg; memcg = parent_mem_cgroup(memcg)) {
struct obj_cgroup *objcg = rcu_dereference(memcg->nodeinfo[nid]->objcg);
@@ -2980,12 +2979,13 @@ static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
return NULL;
}
-static inline struct obj_cgroup *get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
+static inline struct obj_cgroup *get_obj_cgroup_from_memcg(struct mem_cgroup *memcg,
+ int nid)
{
struct obj_cgroup *objcg;
rcu_read_lock();
- objcg = __get_obj_cgroup_from_memcg(memcg);
+ objcg = __get_obj_cgroup_from_memcg(memcg, nid);
rcu_read_unlock();
return objcg;
@@ -3029,7 +3029,7 @@ static struct obj_cgroup *current_objcg_update(void)
rcu_read_lock();
memcg = mem_cgroup_from_task(current);
- objcg = __get_obj_cgroup_from_memcg(memcg);
+ objcg = __get_obj_cgroup_from_memcg(memcg, numa_node_id());
rcu_read_unlock();
/*
@@ -5197,7 +5197,7 @@ static int charge_memcg(struct folio *folio, struct mem_cgroup *memcg,
int ret = 0;
struct obj_cgroup *objcg;
- objcg = get_obj_cgroup_from_memcg(memcg);
+ objcg = get_obj_cgroup_from_memcg(memcg, folio_nid(folio));
/* Do not account at the root objcg level. */
if (!obj_cgroup_is_root(objcg))
ret = try_charge_memcg(memcg, gfp, folio_nr_pages(folio));
@@ -5408,8 +5408,8 @@ void __mem_cgroup_uncharge_folios(struct folio_batch *folios)
*/
void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
{
+ struct obj_cgroup *objcg, *new_objcg = NULL;
struct mem_cgroup *memcg;
- struct obj_cgroup *objcg;
long nr_pages = folio_nr_pages(new);
VM_BUG_ON_FOLIO(!folio_test_locked(old), old);
@@ -5431,6 +5431,7 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
rcu_read_lock();
memcg = obj_cgroup_memcg(objcg);
+
/* Force-charge the new page. The old one will be freed soon */
if (!obj_cgroup_is_root(objcg)) {
page_counter_charge(&memcg->memory, nr_pages);
@@ -5438,7 +5439,31 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
page_counter_charge(&memcg->memsw, nr_pages);
}
- obj_cgroup_get(objcg);
+ /*
+ * memcg_reparent_objcgs() reparents a node's objcgs and its LRU lists
+ * together, under that node's lru_locks. If a folio's objcg is on a
+ * different node, the two happen in separate iterations with the locks
+ * dropped in between, and an LRU operation in that window takes the
+ * lru_lock of the wrong memcg. Keep the objcg on the folio's node.
+ */
+ if (folio_nid(old) != folio_nid(new))
+ new_objcg = __get_obj_cgroup_from_memcg(memcg, folio_nid(new));
+
+ /*
+ * LRU folios are not accounted at the root level: swapping a non-root
+ * objcg for the root one would skip the uncharge and leak the charge.
+ * Keep the old one, the root memcg never reparents. No put needed,
+ * tryget takes no reference on the root objcg.
+ */
+ if (new_objcg && obj_cgroup_is_root(new_objcg) &&
+ !obj_cgroup_is_root(objcg))
+ new_objcg = NULL;
+
+ if (new_objcg)
+ objcg = new_objcg;
+ else
+ obj_cgroup_get(objcg);
+
commit_charge(new, objcg);
memcg1_commit_charge(new, memcg);
rcu_read_unlock();
@@ -5457,7 +5482,7 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
*/
void mem_cgroup_migrate(struct folio *old, struct folio *new)
{
- struct obj_cgroup *objcg;
+ struct obj_cgroup *objcg, *new_objcg = NULL;
VM_BUG_ON_FOLIO(!folio_test_locked(old), old);
VM_BUG_ON_FOLIO(!folio_test_locked(new), new);
@@ -5478,12 +5503,29 @@ void mem_cgroup_migrate(struct folio *old, struct folio *new)
if (!objcg)
return;
- /* Transfer the charge and the objcg ref */
- commit_charge(new, objcg);
+ /* Keep the objcg on the folio's node, see mem_cgroup_replace_folio() */
+ if (folio_nid(old) != folio_nid(new)) {
+ rcu_read_lock();
+ new_objcg = __get_obj_cgroup_from_memcg(obj_cgroup_memcg(objcg),
+ folio_nid(new));
+ rcu_read_unlock();
+
+ /* No root swap, see mem_cgroup_replace_folio(). */
+ if (new_objcg && obj_cgroup_is_root(new_objcg) &&
+ !obj_cgroup_is_root(objcg))
+ new_objcg = NULL;
+ }
+
+ /* Transfer the charge and, unless it was swapped, the objcg ref */
+ commit_charge(new, new_objcg ? : objcg);
/* Warning should never happen, so don't worry about refcount non-0 */
WARN_ON_ONCE(folio_unqueue_deferred_split(old));
old->memcg_data = 0;
+
+ /* @new took its own reference, drop @old's. */
+ if (new_objcg)
+ obj_cgroup_put(objcg);
}
DEFINE_STATIC_KEY_FALSE(memcg_sockets_enabled_key);
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] memcg: keep folio's objcg same as its node
2026-08-06 16:58 [PATCH v2] memcg: keep folio's objcg same as its node Shakeel Butt
@ 2026-08-06 18:15 ` Johannes Weiner
2026-08-06 19:22 ` Johannes Weiner
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Weiner @ 2026-08-06 18:15 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, Michal Hocko, Roman Gushchin, Muchun Song,
Qi Zheng, Meta kernel team, linux-mm, cgroups, linux-kernel,
Karl Erik Hofseth, stable
On Thu, Aug 06, 2026 at 09:58:13AM -0700, Shakeel Butt wrote:
> memcg_reparent_objcgs() has an inherent assumption that a folio's objcg
> is the objcg of the folio's node. Folio migration across nodes breaks
> that assumption: the new folio simply inherits the old folio's objcg
> while living on a different node.
>
> Once the assumption is broken, the reparenting of the folio's objcg and
> the reparenting of the folio's LRU list are no longer atomic.
> memcg_reparent_objcgs() handles one node per iteration and drops all the
> locks in between, so the objcg gets reparented in the iteration for the
> objcg's node while the LRU list gets spliced in the iteration for the
> folio's node. Any LRU operation on that folio in between resolves its
> lruvec through the objcg, and thus takes the lru_lock of the wrong
> memcg, not the lru_lock of the list the folio is actually on.
>
> Fix this by selecting the objcg by folio_nid() at charge time, and by
> re-deriving it for the destination node in mem_cgroup_migrate() and
> mem_cgroup_replace_folio().
>
> Reported-by: Karl Erik Hofseth <karl.e.hofseth@opoint.com>
> Closes: https://lore.kernel.org/all/anMmd1ADrDVwMO6v@work/
> Fixes: f1cf8d2f36dc ("mm: memcontrol: eliminate the problem of dying memory cgroup for LRU folios")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shakeel Butt <shakeel.butt@linux.dev>
> ---
>
> Changes since v1: http://lore.kernel.org/20260806061830.3294679-1-shakeel.butt@linux.dev
>
> - In mem_cgroup_migrate, do obj_cgroup_put at the end (Sashiko)
> - Handle scenario where destination node has been reparented to the root but the
> source node's objcg has not yet (Sashiko)
> - Add comment explaining the race between migration and reparenting (Johannes)
>
> mm/memcontrol.c | 66 ++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 3057396dda53..02c108cdd0f5 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -2966,10 +2966,9 @@ struct mem_cgroup *mem_cgroup_from_virt(void *p)
> return folio_memcg_check(virt_to_folio(p));
> }
>
> -static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
> +static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg,
> + int nid)
> {
> - int nid = numa_node_id();
> -
> for (; memcg; memcg = parent_mem_cgroup(memcg)) {
> struct obj_cgroup *objcg = rcu_dereference(memcg->nodeinfo[nid]->objcg);
>
> @@ -2980,12 +2979,13 @@ static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
> return NULL;
> }
>
> -static inline struct obj_cgroup *get_obj_cgroup_from_memcg(struct mem_cgroup *memcg)
> +static inline struct obj_cgroup *get_obj_cgroup_from_memcg(struct mem_cgroup *memcg,
> + int nid)
> {
> struct obj_cgroup *objcg;
>
> rcu_read_lock();
> - objcg = __get_obj_cgroup_from_memcg(memcg);
> + objcg = __get_obj_cgroup_from_memcg(memcg, nid);
> rcu_read_unlock();
>
> return objcg;
> @@ -3029,7 +3029,7 @@ static struct obj_cgroup *current_objcg_update(void)
>
> rcu_read_lock();
> memcg = mem_cgroup_from_task(current);
> - objcg = __get_obj_cgroup_from_memcg(memcg);
> + objcg = __get_obj_cgroup_from_memcg(memcg, numa_node_id());
> rcu_read_unlock();
>
> /*
> @@ -5197,7 +5197,7 @@ static int charge_memcg(struct folio *folio, struct mem_cgroup *memcg,
> int ret = 0;
> struct obj_cgroup *objcg;
>
> - objcg = get_obj_cgroup_from_memcg(memcg);
> + objcg = get_obj_cgroup_from_memcg(memcg, folio_nid(folio));
> /* Do not account at the root objcg level. */
> if (!obj_cgroup_is_root(objcg))
> ret = try_charge_memcg(memcg, gfp, folio_nr_pages(folio));
> @@ -5408,8 +5408,8 @@ void __mem_cgroup_uncharge_folios(struct folio_batch *folios)
> */
> void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
> {
> + struct obj_cgroup *objcg, *new_objcg = NULL;
> struct mem_cgroup *memcg;
> - struct obj_cgroup *objcg;
> long nr_pages = folio_nr_pages(new);
>
> VM_BUG_ON_FOLIO(!folio_test_locked(old), old);
> @@ -5431,6 +5431,7 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
>
> rcu_read_lock();
> memcg = obj_cgroup_memcg(objcg);
> +
> /* Force-charge the new page. The old one will be freed soon */
> if (!obj_cgroup_is_root(objcg)) {
> page_counter_charge(&memcg->memory, nr_pages);
> @@ -5438,7 +5439,31 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
> page_counter_charge(&memcg->memsw, nr_pages);
> }
>
> - obj_cgroup_get(objcg);
> + /*
> + * memcg_reparent_objcgs() reparents a node's objcgs and its LRU lists
> + * together, under that node's lru_locks. If a folio's objcg is on a
> + * different node, the two happen in separate iterations with the locks
> + * dropped in between, and an LRU operation in that window takes the
> + * lru_lock of the wrong memcg. Keep the objcg on the folio's node.
> + */
> + if (folio_nid(old) != folio_nid(new))
> + new_objcg = __get_obj_cgroup_from_memcg(memcg, folio_nid(new));
> +
> + /*
> + * LRU folios are not accounted at the root level: swapping a non-root
> + * objcg for the root one would skip the uncharge and leak the charge.
> + * Keep the old one, the root memcg never reparents. No put needed,
> + * tryget takes no reference on the root objcg.
> + */
> + if (new_objcg && obj_cgroup_is_root(new_objcg) &&
> + !obj_cgroup_is_root(objcg))
> + new_objcg = NULL;
The two versions have quite some overlap. The refcounting is
different, but IMO that part is also hard to square.
How about a helper that always returns a referenced objcg?
static struct obj_cgroup *get_migration_objcg(struct folio *old, struct folio *new)
{
struct obj_cgroup *old_objcg, *new_objcg;
int new_nid = folio_nid(new);
struct mem_cgroup *memcg;
old_objcg = get_obj_cgroup_from_folio(old);
if (folio_nid(old) == new_nid)
return old_objcg;
memcg = obj_cgroup_memcg(old_objcg);
new_objcg = __get_obj_cgroup_from_memcg(memcg, new_nid);
if (new_objcg && obj_cgroup_is_root(new_objcg) &&
!obj_cgroup_is_root(old_objcg))
return old_objcg;
obj_cgroup_put(old_objcg);
return new_objcg;
}
In mem_cgroup_replace_folio(), this becomes:
rcu_read_lock();
... page counter update ...
objcg = get_migration_objcg(old, new);
commit_charge(new, objcg);
memcg1_commit_charge(new, memcg);
rcu_read_unlock();
> +
> + if (new_objcg)
> + objcg = new_objcg;
> + else
> + obj_cgroup_get(objcg);
> +
> commit_charge(new, objcg);
> memcg1_commit_charge(new, memcg);
> rcu_read_unlock();
> @@ -5457,7 +5482,7 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
> */
> void mem_cgroup_migrate(struct folio *old, struct folio *new)
> {
> - struct obj_cgroup *objcg;
> + struct obj_cgroup *objcg, *new_objcg = NULL;
>
> VM_BUG_ON_FOLIO(!folio_test_locked(old), old);
> VM_BUG_ON_FOLIO(!folio_test_locked(new), new);
> @@ -5478,12 +5503,29 @@ void mem_cgroup_migrate(struct folio *old, struct folio *new)
> if (!objcg)
> return;
>
> - /* Transfer the charge and the objcg ref */
> - commit_charge(new, objcg);
> + /* Keep the objcg on the folio's node, see mem_cgroup_replace_folio() */
> + if (folio_nid(old) != folio_nid(new)) {
> + rcu_read_lock();
> + new_objcg = __get_obj_cgroup_from_memcg(obj_cgroup_memcg(objcg),
> + folio_nid(new));
> + rcu_read_unlock();
> +
> + /* No root swap, see mem_cgroup_replace_folio(). */
> + if (new_objcg && obj_cgroup_is_root(new_objcg) &&
> + !obj_cgroup_is_root(objcg))
> + new_objcg = NULL;
> + }
> +
> + /* Transfer the charge and, unless it was swapped, the objcg ref */
> + commit_charge(new, new_objcg ? : objcg);
>
> /* Warning should never happen, so don't worry about refcount non-0 */
> WARN_ON_ONCE(folio_unqueue_deferred_split(old));
> old->memcg_data = 0;
> +
> + /* @new took its own reference, drop @old's. */
> + if (new_objcg)
> + obj_cgroup_put(objcg);
And here it becomes:
rcu_read_lock();
objcg = get_migration_objcg(old, new);
rcu_read_unlock();
commit_charge(new, objcg);
objcg = folio_objcg(old);
old->memcg_data = 0;
obj_cgroup_put(objcg);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] memcg: keep folio's objcg same as its node
2026-08-06 18:15 ` Johannes Weiner
@ 2026-08-06 19:22 ` Johannes Weiner
2026-08-06 20:03 ` Shakeel Butt
0 siblings, 1 reply; 4+ messages in thread
From: Johannes Weiner @ 2026-08-06 19:22 UTC (permalink / raw)
To: Shakeel Butt
Cc: Andrew Morton, Michal Hocko, Roman Gushchin, Muchun Song,
Qi Zheng, Meta kernel team, linux-mm, cgroups, linux-kernel,
Karl Erik Hofseth, stable
Hm, it's still not quite right.
When there is a root mismatch, we cannot fall back to committing the
old objcg. That would reintroduce Karl's issue. Not as broadly as
before, but can still happen if the memcg tree died up to the root.
So we have to commit to the new objcg, always.
What obj_cgroup_is_root() then comes down to is whether uncharge will
balance the page counters or not.
mem_cgroup_replace_folio() gets a new charge for the new page. We can
just conditionalize that right away on whether the new objcg will
actually uncharge.
mem_cgroup_migrate() currently trades the charge, but that won't work
if the new objcg is root and won't uncharge. So we have to settle it
right then and there.
This?
@@ -5319,6 +5319,46 @@ void __mem_cgroup_uncharge_folios(struct folio_batch *folios)
uncharge_batch(&ug);
}
+/*
+ * An LRU folio must hold the objcg belonging to its own node.
+ *
+ * memcg_reparent_objcgs() reparents a dying cgroup one node at a time:
+ * the folios on that node's LRU lists move to the parent and that
+ * node's objcg is redirected to the parent, atomically under the
+ * node's lru_lock. folio_lruvec_lock() relies on this to provide a
+ * stable folio<->lruvec binding. If a folio holds another node's
+ * objcg, its list membership and its lruvec resolution change in
+ * separate lock sections, and an LRU operation in between can re-add
+ * the folio to, and strand it on, the LRU list of a dead memcg.
+ *
+ * So when migration transfers the memcg state to a folio on another
+ * node, re-derive the objcg for the destination node. If the memcg is
+ * dying and the destination node has already been reparented, the
+ * lookup walks up to the nearest live ancestor - which is also where
+ * that node's LRU lists went.
+ *
+ * Returns the objcg to commit to @new, with a reference for the caller.
+ */
+static struct obj_cgroup *get_migration_objcg(struct folio *old, struct folio *new)
+{
+ struct obj_cgroup *old_objcg, *new_objcg;
+ int new_nid = folio_nid(new);
+
+ old_objcg = get_obj_cgroup_from_folio(old);
+
+ if (folio_nid(old) == new_nid)
+ return old_objcg;
+
+ rcu_read_lock();
+ new_objcg = __get_obj_cgroup_from_memcg(obj_cgroup_memcg(old_objcg),
+ new_nid);
+ rcu_read_unlock();
+
+ obj_cgroup_put(old_objcg);
+
+ return new_objcg;
+}
+
/**
* mem_cgroup_replace_folio - Charge a folio's replacement.
* @old: Currently circulating folio.
@@ -5347,21 +5387,27 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
if (folio_memcg_charged(new))
return;
- objcg = folio_objcg(old);
- VM_WARN_ON_ONCE_FOLIO(!objcg, old);
- if (!objcg)
+ VM_WARN_ON_ONCE_FOLIO(!folio_objcg(old), old);
+ if (!folio_objcg(old))
return;
+ objcg = get_migration_objcg(old, new);
+
rcu_read_lock();
memcg = obj_cgroup_memcg(objcg);
- /* Force-charge the new page. The old one will be freed soon */
+ /*
+ * Force-charge the new page. The old one will be freed soon.
+ *
+ * The rootness of the committed objcg decides whether the final
+ * uncharge of @new goes through the page counters (see
+ * uncharge_folio()); charge them only if the uncharge will.
+ */
if (!obj_cgroup_is_root(objcg)) {
page_counter_charge(&memcg->memory, nr_pages);
if (do_memsw_account())
page_counter_charge(&memcg->memsw, nr_pages);
}
- obj_cgroup_get(objcg);
commit_charge(new, objcg);
memcg1_commit_charge(new, memcg);
rcu_read_unlock();
@@ -5373,14 +5419,15 @@ void mem_cgroup_replace_folio(struct folio *old, struct folio *new)
* @new: Replacement folio.
*
* Transfer the memcg data from the old folio to the new folio for migration.
- * The old folio's data info will be cleared. Note that the memory counters
- * will remain unchanged throughout the process.
+ * The old folio's data info will be cleared. The memory counters remain
+ * unchanged, unless the charge moves out of a fully reparented ancestry
+ * and has to be settled (see below).
*
* Both folios must be locked, @new->mapping must be set up.
*/
void mem_cgroup_migrate(struct folio *old, struct folio *new)
{
- struct obj_cgroup *objcg;
+ struct obj_cgroup *objcg, *new_objcg;
VM_BUG_ON_FOLIO(!folio_test_locked(old), old);
VM_BUG_ON_FOLIO(!folio_test_locked(new), new);
@@ -5401,12 +5448,30 @@ void mem_cgroup_migrate(struct folio *old, struct folio *new)
if (!objcg)
return;
- /* Transfer the charge and the objcg ref */
- commit_charge(new, objcg);
+ new_objcg = get_migration_objcg(old, new);
+
+ /*
+ * @old was charged through a non-root objcg, so its charge is in
+ * the page counters. If the re-derivation walked up to the root
+ * objcg - @old's entire ancestry is dying and already reparented
+ * - the final uncharge of @new will skip the page counters (see
+ * uncharge_folio()). Settle them now: this is @old's eventual
+ * uncharge, moved up to the point where its charge record ends.
+ */
+ if (obj_cgroup_is_root(new_objcg) && !obj_cgroup_is_root(objcg)) {
+ rcu_read_lock();
+ memcg_uncharge(obj_cgroup_memcg(objcg), folio_nr_pages(old));
+ rcu_read_unlock();
+ }
+
+ commit_charge(new, new_objcg);
/* Warning should never happen, so don't worry about refcount non-0 */
WARN_ON_ONCE(folio_unqueue_deferred_split(old));
old->memcg_data = 0;
+
+ /* @new holds its own reference now, drop @old's */
+ obj_cgroup_put(objcg);
}
DEFINE_STATIC_KEY_FALSE(memcg_sockets_enabled_key);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] memcg: keep folio's objcg same as its node
2026-08-06 19:22 ` Johannes Weiner
@ 2026-08-06 20:03 ` Shakeel Butt
0 siblings, 0 replies; 4+ messages in thread
From: Shakeel Butt @ 2026-08-06 20:03 UTC (permalink / raw)
To: Johannes Weiner
Cc: Andrew Morton, Michal Hocko, Roman Gushchin, Muchun Song,
Qi Zheng, Meta kernel team, linux-mm, cgroups, linux-kernel,
Karl Erik Hofseth, stable
On Thu, Aug 06, 2026 at 03:22:50PM -0400, Johannes Weiner wrote:
> Hm, it's still not quite right.
>
> When there is a root mismatch, we cannot fall back to committing the
> old objcg. That would reintroduce Karl's issue. Not as broadly as
> before, but can still happen if the memcg tree died up to the root.
>
> So we have to commit to the new objcg, always.
>
> What obj_cgroup_is_root() then comes down to is whether uncharge will
> balance the page counters or not.
>
> mem_cgroup_replace_folio() gets a new charge for the new page. We can
> just conditionalize that right away on whether the new objcg will
> actually uncharge.
>
> mem_cgroup_migrate() currently trades the charge, but that won't work
> if the new objcg is root and won't uncharge. So we have to settle it
> right then and there.
>
> This?
Thanks a lot Johannes, this looks much better. I will incorporate this and run
through the reproducer before sending out the v3.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-06 20:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 16:58 [PATCH v2] memcg: keep folio's objcg same as its node Shakeel Butt
2026-08-06 18:15 ` Johannes Weiner
2026-08-06 19:22 ` Johannes Weiner
2026-08-06 20:03 ` Shakeel Butt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox