* [PATCH RFC 0/5] memcg: fix dying memcg pinned by swapped out shmem pages
@ 2026-08-13 8:52 Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 1/5] memcg: move memcg private ID refcount to objcg Bingfang Guo via B4 Relay
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Bingfang Guo via B4 Relay @ 2026-08-13 8:52 UTC (permalink / raw)
To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, Dave Chinner, Qi Zheng,
David Hildenbrand, Lorenzo Stoakes, Kairui Song, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu
Cc: cgroups, linux-mm, linux-kernel, Bingfang Guo, Bingfang Guo
Currently the private ID of a memory cgroup holds a reference to its
css, so that users such as list_lru and swap entries can look the memcg
up by ID even after it has gone offline.
This has a downside: a memcg that still owns swapped out shmem pages is
pinned via its private ID and is never released until those pages are
swapped back in. Workloads that create and destroy many cgroups can
accumulate a large number of dying memcgs.
This series fixes that by moving the memcg private ID reference count
from the memcg to its object cgroup. The objcg is cheap to keep alive
and is reparented to an online ancestor when its memcg goes offline, so
holding the ID no longer pins the css.
Patch 1 (memcg: move memcg private ID refcount to objcg)
Move the ID refcount from the memcg to its objcg, so swapped out pages
no longer pin the dying memcg. The online state pins the objcg, and the
ID is released in css_released() after reparenting; this guarantees the
id refcount is alive for any caller holding a memcg reference. After
reparenting, mem_cgroup_from_private_id() returns a live ancestor
instead of the original memcg; callers that need the exact memcg are
fixed in patch 5.
Patch 2 (memcg: get stable memcg first before getting memcgid reference)
Both __memcg1_swapout() and __mem_cgroup_try_charge_swap() now take a
stable memcg reference first via get_mem_cgroup_from_objcg() and pin
the ID afterwards, dropping the rcu_read_lock() usage and the
get-error-put handling, and recording exactly the memcg the folio
belongs to in the swap entry.
Patch 3 (memcg: remove retry logic in mem_cgroup_private_id_get_online)
With the ID released only in css_released() (patch 1) and every caller
holding a stable reference (patch 2), the retry loop that walked up the
parent chain can never trigger. Remove it, rename the function to
mem_cgroup_private_id_get().
Patch 4 (memcg: add a helper to get online memcg from memcgid)
Add mem_cgroup_from_private_id_online(), which looks the ID up and
takes a reference through the objcg. Since the objcg is always
reparented to an online memcg, this guarantees an online memcg. Use it
in mem_cgroup_swapin_charge_folio(), dropping the manual id lookup and
css_tryget_online() check under the RCU read lock.
Patch 5 (memcg: filter out reparented memcgs got using memcgid)
After reparenting, mem_cgroup_from_private_id() can return a memcg that
no longer owns the ID. Callers such as list_lru and workingset refault
expect exactly the memcg the ID refers to, so check that the returned
memcg still owns the ID and return NULL otherwise, letting the callers
skip the entry.
The following program reproduces the problem: it repeatedly creates a
memcg, dirties one shared shmem page, pushes it to swap, and destroys
the cgroup. Each iteration leaves a dying memcg pinned by the swapped
out page.
shmem_dying_memcg.c:
```c
static int write_str(const char *path, const char *val) {
int fd = open(path, O_WRONLY);
if (fd < 0) return -1;
int rc = write(fd, val, strlen(val)) < 0 ? -1 : 0;
close(fd);
return rc;
}
static long long dying_memcgs(void) {
FILE *f = fopen(CG_ROOT "/cgroup.stat", "r");
if (!f) return -1;
char k[64];
long long v = -1;
while (fscanf(f, "%63s %lld", k, &v) == 2)
if (!strcmp(k, "nr_dying_subsys_memory")) break;
fclose(f);
return v;
}
// worker: join the memcg, dirty one shared shmem page, swap it out, exit.
static void worker(const char *cgdir, const char *shmfile) {
char pid[16];
snprintf(pid, sizeof pid, "%d", getpid());
if (write_str(cgdir, pid) < 0) _exit(1); // cgroup.procs
int fd = open(shmfile, O_CREAT | O_RDWR | O_TRUNC, 0644);
if (fd < 0 || ftruncate(fd, PAGE) < 0) _exit(2);
char *p = mmap(NULL, PAGE, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
if (p == MAP_FAILED) _exit(3);
close(fd);
p[0] = 1; // fault in + dirty
if (madvise(p, PAGE, MADV_PAGEOUT) < 0) _exit(4); // push to swap
_exit(0);
}
static void one_iter(long i) {
char cgdir[256], procs[300], shmfile[300];
snprintf(cgdir, sizeof cgdir, "%s/m%ld", CG_BASE, i);
snprintf(procs, sizeof procs, "%s/cgroup.procs", cgdir);
snprintf(shmfile, sizeof shmfile, "%s/f%ld", SHM_DIR, i);
if (mkdir(cgdir, 0755) < 0 && errno != EEXIST) return;
pid_t pid = fork();
if (pid == 0) worker(procs, shmfile);
waitpid(pid, NULL, 0);
rmdir(cgdir); // memcg goes dying; swapped page pins its id
}
static void cleanup(void) {
struct dirent *de;
char p[512];
DIR *d = opendir(CG_BASE);
if (d) {
while ((de = readdir(d)))
if (de->d_name[0] == 'm') {
snprintf(p, sizeof p, "%s/%s", CG_BASE, de->d_name);
rmdir(p);
}
closedir(d);
}
d = opendir(SHM_DIR);
if (d) {
while ((de = readdir(d)))
if (de->d_name[0] != '.') {
snprintf(p, sizeof p, "%s/%s", SHM_DIR, de->d_name);
unlink(p);
}
closedir(d);
}
rmdir(CG_BASE);
rmdir(SHM_DIR);
printf("cleanup done; nr_dying_subsys_memory = %lld\n", dying_memcgs());
}
int main(int argc, char **argv) {
if (geteuid() != 0) { fprintf(stderr, "run as root\n"); return 1; }
if (argc > 1 && !strcmp(argv[1], "--cleanup")) { cleanup(); return 0; }
long n = argc > 1 ? atol(argv[1]) : 1000;
mkdir(CG_BASE, 0755);
write_str(CG_ROOT "/cgroup.subtree_control", "+memory");
if (write_str(CG_BASE "/cgroup.subtree_control", "+memory") < 0) {
fprintf(stderr, "enable +memory: %s\n", strerror(errno));
return 1;
}
mkdir(SHM_DIR, 0755);
printf("before: nr_dying_subsys_memory = %lld\n", dying_memcgs());
for (long i = 0; i < n; i++) one_iter(i);
printf("after: nr_dying_subsys_memory = %lld (created %ld memcgs)\n",
dying_memcgs(), n);
printf("release with: sudo %s --cleanup\n", argv[0]);
return 0;
}
```
Run with:
```bash
echo "=== before ==="
grep dying /sys/fs/cgroup/cgroup.stat
sudo ./shmem_dying_memcg "$@"
sleep 1
echo
echo "=== after (root cgroup.stat) ==="
grep dying /sys/fs/cgroup/cgroup.stat
```
To release the pinned memcgs:
$ sudo ./shmem_dying_memcg --cleanup
Results on my machine (20000 iterations):
Before this series, all 20000 destroyed memcgs stay dying:
```
=== before ===
nr_dying_descendants 20002
nr_dying_subsys_cpuset 0
nr_dying_subsys_cpu 0
nr_dying_subsys_io 2
nr_dying_subsys_memory 20002
nr_dying_subsys_perf_event 0
nr_dying_subsys_hugetlb 0
nr_dying_subsys_pids 0
before: nr_dying_subsys_memory = 20002
after: nr_dying_subsys_memory = 20002 (created 20000 memcgs)
release with: sudo ./shmem_dying_memcg --cleanup
=== after (root cgroup.stat) ===
nr_dying_descendants 20002
nr_dying_subsys_cpuset 0
nr_dying_subsys_cpu 0
nr_dying_subsys_io 2
nr_dying_subsys_memory 20002
nr_dying_subsys_perf_event 0
nr_dying_subsys_hugetlb 0
nr_dying_subsys_pids 0
```
After this series, dying memcgs no longer accumulate:
```
=== before ===
nr_dying_descendants 0
nr_dying_subsys_cpuset 0
nr_dying_subsys_cpu 0
nr_dying_subsys_io 0
nr_dying_subsys_memory 0
nr_dying_subsys_perf_event 0
nr_dying_subsys_hugetlb 0
nr_dying_subsys_pids 0
before: nr_dying_subsys_memory = 0
after: nr_dying_subsys_memory = 314 (created 20000 memcgs)
release with: sudo ./shmem_dying_memcg --cleanup
=== after (root cgroup.stat) ===
nr_dying_descendants 0
nr_dying_subsys_cpuset 0
nr_dying_subsys_cpu 0
nr_dying_subsys_io 0
nr_dying_subsys_memory 0
nr_dying_subsys_perf_event 0
nr_dying_subsys_hugetlb 0
nr_dying_subsys_pids 0
```
Signed-off-by: Bingfang Guo <bingfangguo@tencent.com>
---
Bingfang Guo (5):
memcg: move memcg private ID refcount to objcg
memcg: get stable memcg first before getting memcgid reference
memcg: remove retry logic in mem_cgroup_private_id_get_online
memcg: add a helper to get online memcg from memcgid
memcg: filter out reparented memcgs got using memcgid
include/linux/memcontrol.h | 11 +++-
mm/list_lru.c | 2 +-
mm/memcontrol-v1.c | 23 +++----
mm/memcontrol-v1.h | 3 +-
mm/memcontrol.c | 151 ++++++++++++++++++++++++++++++---------------
mm/workingset.c | 6 +-
6 files changed, 126 insertions(+), 70 deletions(-)
---
base-commit: 288058d8db5d35623228d84f48d9bea3707d5c85
change-id: 20260812-memcgid-objcg-de9f578ef128
Best regards,
--
Bingfang Guo <bingfangguo@tencent.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH RFC 1/5] memcg: move memcg private ID refcount to objcg
2026-08-13 8:52 [PATCH RFC 0/5] memcg: fix dying memcg pinned by swapped out shmem pages Bingfang Guo via B4 Relay
@ 2026-08-13 8:52 ` Bingfang Guo via B4 Relay
2026-08-13 15:09 ` Bingfang Guo
2026-08-13 8:52 ` [PATCH RFC 2/5] memcg: get stable memcg first before getting memcgid reference Bingfang Guo via B4 Relay
` (4 subsequent siblings)
5 siblings, 1 reply; 8+ messages in thread
From: Bingfang Guo via B4 Relay @ 2026-08-13 8:52 UTC (permalink / raw)
To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, Dave Chinner, Qi Zheng,
David Hildenbrand, Lorenzo Stoakes, Kairui Song, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu
Cc: cgroups, linux-mm, linux-kernel, Bingfang Guo, Bingfang Guo
From: Bingfang Guo <bingfangguo@tencent.com>
In the previous series by Muchun Song and Qi Zheng, folios are charged
to the objcg and reparented as the memcg offlines. Same can be done to
memcg private ID and its main user: swap entries. Make the memcgid
xarray hold a pointer and a reference to an objcg of the memcg, which is
used to find the memcg (or its parent) later on. The online state now
pins the objcg instead of the css, so swapped out pages no longer pin
the dying memcg.
The id reference held by the online state is released in css_released()
after reparenting instead of in css_offline(). This is the key
invariant the rest of the series builds on: css_offline() runs while
other css references may still be held, but css_released() only runs
once the last reference is gone, so a caller holding a memcg reference
can always count on the id refcount being alive. To prevent races
between memcgid put in css offline and memcgid get, the release is put
off till css_released, which could delay the release of the memcgid and
the objcg it pins, but overall it should be fine.
After reparenting, the objcg points to a live ancestor, so
mem_cgroup_from_private_id() now returns that ancestor instead of the
memcg the ID originally belonged to. Callers that need the exact memcg
are fixed in patch 5.
Signed-off-by: Bingfang Guo <bingfangguo@tencent.com>
---
include/linux/memcontrol.h | 10 +++--
mm/memcontrol.c | 99 ++++++++++++++++++++++++++++++++--------------
2 files changed, 76 insertions(+), 33 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 8170bb8066a22..c33ec7efad50b 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -191,6 +191,7 @@ struct obj_cgroup {
struct rcu_head rcu;
};
bool is_root;
+ refcount_t id_ref;
};
/*
@@ -202,8 +203,8 @@ struct obj_cgroup {
struct mem_cgroup {
struct cgroup_subsys_state css;
- /* Private memcg ID. Used to ID objects that outlive the cgroup */
- struct mem_cgroup_private_id id;
+ /* The objcg holding private memcg ID. */
+ struct obj_cgroup *id_objcg;
/* Accounted resources */
struct page_counter memory; /* Both v1 & v2 */
@@ -270,6 +271,9 @@ struct mem_cgroup {
#endif
int kmemcg_id;
+ /* Private memcg ID. Used to ID objects that outlive the cgroup */
+ int id;
+
struct memcg_vmstats_percpu __percpu *vmstats_percpu;
#ifdef CONFIG_CGROUP_WRITEBACK
@@ -820,7 +824,7 @@ static inline unsigned short mem_cgroup_private_id(struct mem_cgroup *memcg)
if (mem_cgroup_disabled())
return 0;
- return memcg->id.id;
+ return memcg->id;
}
struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8319ad8c5c23a..5f30e76ee93d7 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3697,7 +3697,7 @@ static void memcg_online_kmem(struct mem_cgroup *memcg)
static_branch_enable(&memcg_kmem_online_key);
- memcg->kmemcg_id = memcg->id.id;
+ memcg->kmemcg_id = memcg->id;
}
static void memcg_offline_kmem(struct mem_cgroup *memcg)
@@ -3956,25 +3956,40 @@ static DEFINE_XARRAY_ALLOC1(mem_cgroup_private_ids);
static void mem_cgroup_private_id_remove(struct mem_cgroup *memcg)
{
- if (memcg->id.id > 0) {
- xa_erase(&mem_cgroup_private_ids, memcg->id.id);
- memcg->id.id = 0;
+ if (memcg->id > 0) {
+ xa_erase(&mem_cgroup_private_ids, memcg->id);
+ memcg->id = 0;
}
}
-static inline void mem_cgroup_private_id_put(struct mem_cgroup *memcg, unsigned int n)
+/**
+ * @objcg: the objcg returned by mem_cgroup_private_id_objcg
+ * @id: the corresponding memcg private id
+ */
+static void __mem_cgroup_private_id_put(struct obj_cgroup *objcg,
+ unsigned short id, unsigned int n)
{
- if (refcount_sub_and_test(n, &memcg->id.ref)) {
- mem_cgroup_private_id_remove(memcg);
+ struct obj_cgroup *objcg_free;
- /* Memcg ID pins CSS */
- css_put(&memcg->css);
+ if (refcount_sub_and_test(n, &objcg->id_ref)) {
+ objcg_free = xa_erase(&mem_cgroup_private_ids, id);
+ VM_WARN_ON(objcg_free != objcg);
+
+ /* Memcg ID pins the objcg */
+ obj_cgroup_put(objcg);
}
}
+static inline void mem_cgroup_private_id_put(struct mem_cgroup *memcg, unsigned int n)
+{
+ __mem_cgroup_private_id_put(memcg->id_objcg, memcg->id, n);
+}
+
struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg, unsigned int n)
{
- while (!refcount_add_not_zero(n, &memcg->id.ref)) {
+ struct obj_cgroup *objcg = memcg->id_objcg;
+
+ while (!refcount_add_not_zero(n, &objcg->id_ref)) {
/*
* The root cgroup cannot be destroyed, so it's refcount must
* always be >= 1.
@@ -3984,6 +3999,7 @@ struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg, un
break;
}
memcg = parent_mem_cgroup(memcg);
+ objcg = memcg->id_objcg;
}
return memcg;
}
@@ -3996,8 +4012,29 @@ struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg, un
*/
struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id)
{
+ struct obj_cgroup *objcg;
WARN_ON_ONCE(!rcu_read_lock_held());
- return xa_load(&mem_cgroup_private_ids, id);
+
+ objcg = xa_load(&mem_cgroup_private_ids, id);
+ if (!objcg)
+ return NULL;
+
+ return obj_cgroup_memcg(objcg);
+}
+
+static struct mem_cgroup *mem_cgroup_take_from_private_id(unsigned short id, unsigned int n)
+{
+ struct obj_cgroup *objcg;
+ struct mem_cgroup *memcg;
+
+ objcg = xa_load(&mem_cgroup_private_ids, id);
+ if (!objcg)
+ return NULL;
+
+ memcg = get_mem_cgroup_from_objcg(objcg);
+
+ __mem_cgroup_private_id_put(objcg, id, n);
+ return memcg;
}
struct mem_cgroup *mem_cgroup_get_from_id(u64 id)
@@ -4098,7 +4135,7 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
if (!memcg)
return ERR_PTR(-ENOMEM);
- error = xa_alloc(&mem_cgroup_private_ids, &memcg->id.id, NULL,
+ error = xa_alloc(&mem_cgroup_private_ids, &memcg->id, NULL,
XA_LIMIT(1, MEM_CGROUP_ID_MAX), GFP_KERNEL);
if (error)
goto fail;
@@ -4243,9 +4280,10 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
FLUSH_TIME);
lru_gen_online_memcg(memcg);
- /* Online state pins memcg ID, memcg ID pins CSS */
- refcount_set(&memcg->id.ref, 1);
- css_get(css);
+ /* CSS pins memcg ID, memcg ID pins obj cgroup */
+ memcg->id_objcg = memcg->nodeinfo[0]->objcg;
+ refcount_set(&memcg->id_objcg->id_ref, 1);
+ obj_cgroup_get(memcg->id_objcg);
/*
* Ensure mem_cgroup_from_private_id() works once we're fully online.
@@ -4257,7 +4295,7 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
* publish it here at the end of onlining. This matches the
* regular ID destruction during offlining.
*/
- xa_store(&mem_cgroup_private_ids, memcg->id.id, memcg, GFP_KERNEL);
+ xa_store(&mem_cgroup_private_ids, memcg->id, objcg, GFP_KERNEL);
return 0;
free_objcg:
@@ -4308,8 +4346,6 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
lru_gen_offline_memcg(memcg);
drain_all_stock(memcg);
-
- mem_cgroup_private_id_put(memcg, 1);
}
static void mem_cgroup_css_released(struct cgroup_subsys_state *css)
@@ -4318,6 +4354,9 @@ static void mem_cgroup_css_released(struct cgroup_subsys_state *css)
invalidate_reclaim_iterators(memcg);
lru_gen_release_memcg(memcg);
+
+ mem_cgroup_private_id_put(memcg, 1);
+ memcg->id_objcg = NULL;
}
static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
@@ -5651,19 +5690,19 @@ void __mem_cgroup_uncharge_swap(unsigned short id, unsigned int nr_pages)
{
struct mem_cgroup *memcg;
- rcu_read_lock();
- memcg = mem_cgroup_from_private_id(id);
- if (memcg) {
- if (!mem_cgroup_is_root(memcg)) {
- if (do_memsw_account())
- page_counter_uncharge(&memcg->memsw, nr_pages);
- else
- page_counter_uncharge(&memcg->swap, nr_pages);
- }
- mod_memcg_state(memcg, MEMCG_SWAP, -nr_pages);
- mem_cgroup_private_id_put(memcg, nr_pages);
+ memcg = mem_cgroup_take_from_private_id(id, nr_pages);
+ if (!memcg)
+ return;
+
+ if (!mem_cgroup_is_root(memcg)) {
+ if (do_memsw_account())
+ page_counter_uncharge(&memcg->memsw, nr_pages);
+ else
+ page_counter_uncharge(&memcg->swap, nr_pages);
}
- rcu_read_unlock();
+ mod_memcg_state(memcg, MEMCG_SWAP, -nr_pages);
+
+ mem_cgroup_put(memcg);
}
long mem_cgroup_get_nr_swap_pages(struct mem_cgroup *memcg)
--
2.43.7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC 2/5] memcg: get stable memcg first before getting memcgid reference
2026-08-13 8:52 [PATCH RFC 0/5] memcg: fix dying memcg pinned by swapped out shmem pages Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 1/5] memcg: move memcg private ID refcount to objcg Bingfang Guo via B4 Relay
@ 2026-08-13 8:52 ` Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 3/5] memcg: remove retry logic in mem_cgroup_private_id_get_online Bingfang Guo via B4 Relay
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Bingfang Guo via B4 Relay @ 2026-08-13 8:52 UTC (permalink / raw)
To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, Dave Chinner, Qi Zheng,
David Hildenbrand, Lorenzo Stoakes, Kairui Song, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu
Cc: cgroups, linux-mm, linux-kernel, Bingfang Guo, Bingfang Guo
From: Bingfang Guo <bingfangguo@tencent.com>
Storing the memcg private ID in a swap entry used to take the ID
reference under the RCU read lock and rely on
mem_cgroup_private_id_get_online() to hand back a usable (possibly
parent) memcg.
Now that the ID refcount lives on the objcg and stays alive until
css_released(), holding a memcg reference is enough to pin the ID. Both
__memcg1_swapout() and __mem_cgroup_try_charge_swap() take a stable
memcg reference first via get_mem_cgroup_from_objcg() and pin the ID
afterwards, dropping the rcu_read_lock() usage and the get-error-put
handling, and recording exactly the memcg the folio belongs to in the
swap entry.
Signed-off-by: Bingfang Guo <bingfangguo@tencent.com>
---
mm/memcontrol-v1.c | 23 +++++++++--------------
mm/memcontrol.c | 9 ++++++---
2 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index 2dc599484d006..a913d32ad1e17 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -618,7 +618,7 @@ void memcg1_commit_charge(struct folio *folio, struct mem_cgroup *memcg)
*/
void __memcg1_swapout(struct folio *folio, struct swap_cluster_info *ci)
{
- struct mem_cgroup *memcg, *swap_memcg;
+ struct mem_cgroup *memcg;
struct obj_cgroup *objcg;
unsigned int nr_entries;
@@ -638,19 +638,20 @@ void __memcg1_swapout(struct folio *folio, struct swap_cluster_info *ci)
if (!objcg)
return;
- rcu_read_lock();
- memcg = obj_cgroup_memcg(objcg);
/*
* In case the memcg owning these pages has been offlined and doesn't
* have an ID allocated to it anymore, charge the closest online
- * ancestor for the swap instead and transfer the memory+swap charge.
+ * ancestor for the swap instead.
*/
+ memcg = get_mem_cgroup_from_objcg(objcg);
nr_entries = folio_nr_pages(folio);
- swap_memcg = mem_cgroup_private_id_get_online(memcg, nr_entries);
- mod_memcg_state(swap_memcg, MEMCG_SWAP, nr_entries);
+ mod_memcg_state(memcg, MEMCG_SWAP, nr_entries);
+
+ /* we have a reference to it, so we should get exact memcg itself */
+ mem_cgroup_private_id_get_online(memcg, nr_entries);
__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_entries,
- mem_cgroup_private_id(swap_memcg));
+ mem_cgroup_private_id(memcg));
folio_unqueue_deferred_split(folio);
folio->memcg_data = 0;
@@ -658,12 +659,6 @@ void __memcg1_swapout(struct folio *folio, struct swap_cluster_info *ci)
if (!obj_cgroup_is_root(objcg))
page_counter_uncharge(&memcg->memory, nr_entries);
- if (memcg != swap_memcg) {
- if (!mem_cgroup_is_root(swap_memcg))
- page_counter_charge(&swap_memcg->memsw, nr_entries);
- page_counter_uncharge(&memcg->memsw, nr_entries);
- }
-
/*
* The caller must hold the swap cluster lock with IRQ off. It is
* important here to have the interrupts disabled because it is the
@@ -675,7 +670,7 @@ void __memcg1_swapout(struct folio *folio, struct swap_cluster_info *ci)
preempt_enable_nested();
memcg1_check_events(memcg, folio_nid(folio));
- rcu_read_unlock();
+ mem_cgroup_put(memcg);
obj_cgroup_put(objcg);
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 5f30e76ee93d7..a210fe2501219 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -5660,24 +5660,27 @@ int __mem_cgroup_try_charge_swap(struct folio *folio)
return 0;
}
- memcg = mem_cgroup_private_id_get_online(memcg, nr_pages);
- /* memcg is pined by memcg ID. */
+ memcg = get_mem_cgroup_from_objcg(objcg);
rcu_read_unlock();
if (!mem_cgroup_is_root(memcg) &&
!page_counter_try_charge(&memcg->swap, nr_pages, &counter)) {
memcg_memory_event(memcg, MEMCG_SWAP_MAX);
memcg_memory_event(memcg, MEMCG_SWAP_FAIL);
- mem_cgroup_private_id_put(memcg, nr_pages);
+ mem_cgroup_put(memcg);
return -ENOMEM;
}
mod_memcg_state(memcg, MEMCG_SWAP, nr_pages);
+ /* we have a reference to it, so we should get exact memcg itself */
+ mem_cgroup_private_id_get_online(memcg, nr_pages);
+
ci = swap_cluster_get_and_lock(folio);
__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_pages,
mem_cgroup_private_id(memcg));
swap_cluster_unlock(ci);
+ mem_cgroup_put(memcg);
return 0;
}
--
2.43.7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC 3/5] memcg: remove retry logic in mem_cgroup_private_id_get_online
2026-08-13 8:52 [PATCH RFC 0/5] memcg: fix dying memcg pinned by swapped out shmem pages Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 1/5] memcg: move memcg private ID refcount to objcg Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 2/5] memcg: get stable memcg first before getting memcgid reference Bingfang Guo via B4 Relay
@ 2026-08-13 8:52 ` Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 4/5] memcg: add a helper to get online memcg from memcgid Bingfang Guo via B4 Relay
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Bingfang Guo via B4 Relay @ 2026-08-13 8:52 UTC (permalink / raw)
To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, Dave Chinner, Qi Zheng,
David Hildenbrand, Lorenzo Stoakes, Kairui Song, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu
Cc: cgroups, linux-mm, linux-kernel, Bingfang Guo, Bingfang Guo
From: Bingfang Guo <bingfangguo@tencent.com>
With the ID released only in css_released() (patch 1) and every caller
holding a stable reference obtained from get_mem_cgroup_from_objcg()
(patch 2), the id refcount is guaranteed to be non-zero whenever the ID
is taken, so the retry loop that walked up the parent chain can never
trigger.
Remove the retry logic, rename the function to
mem_cgroup_private_id_get() and turn the fallible return value into a
VM_WARN_ON() that documents the invariant.
Signed-off-by: Bingfang Guo <bingfangguo@tencent.com>
---
mm/memcontrol-v1.c | 2 +-
mm/memcontrol-v1.h | 3 +--
mm/memcontrol.c | 20 +++++---------------
3 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index a913d32ad1e17..e5161e061bd11 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -648,7 +648,7 @@ void __memcg1_swapout(struct folio *folio, struct swap_cluster_info *ci)
mod_memcg_state(memcg, MEMCG_SWAP, nr_entries);
/* we have a reference to it, so we should get exact memcg itself */
- mem_cgroup_private_id_get_online(memcg, nr_entries);
+ mem_cgroup_private_id_get(memcg, nr_entries);
__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_entries,
mem_cgroup_private_id(memcg));
diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h
index 0f703f239c80f..9c74400aa7ddb 100644
--- a/mm/memcontrol-v1.h
+++ b/mm/memcontrol-v1.h
@@ -21,8 +21,7 @@ void drain_all_stock(struct mem_cgroup *root_memcg);
int memory_stat_show(struct seq_file *m, void *v);
-struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg,
- unsigned int n);
+void mem_cgroup_private_id_get(struct mem_cgroup *memcg, unsigned int n);
/* Cgroup v1-specific declarations */
#ifdef CONFIG_MEMCG_V1
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index a210fe2501219..12545ca48194d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3985,23 +3985,13 @@ static inline void mem_cgroup_private_id_put(struct mem_cgroup *memcg, unsigned
__mem_cgroup_private_id_put(memcg->id_objcg, memcg->id, n);
}
-struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg, unsigned int n)
+void mem_cgroup_private_id_get(struct mem_cgroup *memcg, unsigned int n)
{
+ bool success;
struct obj_cgroup *objcg = memcg->id_objcg;
- while (!refcount_add_not_zero(n, &objcg->id_ref)) {
- /*
- * The root cgroup cannot be destroyed, so it's refcount must
- * always be >= 1.
- */
- if (WARN_ON_ONCE(mem_cgroup_is_root(memcg))) {
- VM_BUG_ON(1);
- break;
- }
- memcg = parent_mem_cgroup(memcg);
- objcg = memcg->id_objcg;
- }
- return memcg;
+ success = refcount_add_not_zero(n, &objcg->id_ref);
+ VM_WARN_ON(!success);
}
/**
@@ -5673,7 +5663,7 @@ int __mem_cgroup_try_charge_swap(struct folio *folio)
mod_memcg_state(memcg, MEMCG_SWAP, nr_pages);
/* we have a reference to it, so we should get exact memcg itself */
- mem_cgroup_private_id_get_online(memcg, nr_pages);
+ mem_cgroup_private_id_get(memcg, nr_pages);
ci = swap_cluster_get_and_lock(folio);
__swap_cgroup_set(ci, swp_cluster_offset(folio->swap), nr_pages,
--
2.43.7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC 4/5] memcg: add a helper to get online memcg from memcgid
2026-08-13 8:52 [PATCH RFC 0/5] memcg: fix dying memcg pinned by swapped out shmem pages Bingfang Guo via B4 Relay
` (2 preceding siblings ...)
2026-08-13 8:52 ` [PATCH RFC 3/5] memcg: remove retry logic in mem_cgroup_private_id_get_online Bingfang Guo via B4 Relay
@ 2026-08-13 8:52 ` Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 5/5] memcg: filter out reparented memcgs got using memcgid Bingfang Guo via B4 Relay
2026-08-13 13:26 ` [syzbot ci] Re: memcg: fix dying memcg pinned by swapped out shmem pages syzbot ci
5 siblings, 0 replies; 8+ messages in thread
From: Bingfang Guo via B4 Relay @ 2026-08-13 8:52 UTC (permalink / raw)
To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, Dave Chinner, Qi Zheng,
David Hildenbrand, Lorenzo Stoakes, Kairui Song, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu
Cc: cgroups, linux-mm, linux-kernel, Bingfang Guo, Bingfang Guo
From: Bingfang Guo <bingfangguo@tencent.com>
When swapping in, the folio is charged back to the memcg that swapped
it out, or to one of its ancestors if that memcg is gone.
mem_cgroup_swapin_charge_folio() currently does the id lookup and the
css_tryget_online() check by hand under the RCU read lock.
The objcg behind the id is reparented to an online memcg when its own
memcg is destroyed, so looking the id up and taking a reference
through the objcg is enough to guarantee an online memcg. Add
mem_cgroup_from_private_id_online() for that purpose and use it in
mem_cgroup_swapin_charge_folio(), dropping the RCU read lock usage.
Signed-off-by: Bingfang Guo <bingfangguo@tencent.com>
---
include/linux/memcontrol.h | 1 +
mm/memcontrol.c | 22 ++++++++++++++++++----
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index c33ec7efad50b..fef8a1c4191b1 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -827,6 +827,7 @@ static inline unsigned short mem_cgroup_private_id(struct mem_cgroup *memcg)
return memcg->id;
}
struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id);
+struct mem_cgroup *mem_cgroup_from_private_id_online(unsigned short id);
static inline u64 mem_cgroup_id(struct mem_cgroup *memcg)
{
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 12545ca48194d..fdf2e0d1f17e5 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -4012,6 +4012,22 @@ struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id)
return obj_cgroup_memcg(objcg);
}
+/**
+ * mem_cgroup_from_private_id - look up an online memcg from a memcg id
+ * and get a reference.
+ * @id: the memcg id to look up
+ */
+struct mem_cgroup *mem_cgroup_from_private_id_online(unsigned short id)
+{
+ struct obj_cgroup *objcg;
+
+ objcg = xa_load(&mem_cgroup_private_ids, id);
+ if (!objcg)
+ return NULL;
+
+ return get_mem_cgroup_from_objcg(objcg);
+}
+
static struct mem_cgroup *mem_cgroup_take_from_private_id(unsigned short id, unsigned int n)
{
struct obj_cgroup *objcg;
@@ -5248,11 +5264,9 @@ int mem_cgroup_swapin_charge_folio(struct folio *folio, unsigned short id,
if (mem_cgroup_disabled())
return 0;
- rcu_read_lock();
- memcg = mem_cgroup_from_private_id(id);
- if (!memcg || !css_tryget_online(&memcg->css))
+ memcg = mem_cgroup_from_private_id_online(id);
+ if (!memcg)
memcg = get_mem_cgroup_from_mm(mm);
- rcu_read_unlock();
ret = charge_memcg(folio, memcg, gfp);
--
2.43.7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RFC 5/5] memcg: filter out reparented memcgs got using memcgid
2026-08-13 8:52 [PATCH RFC 0/5] memcg: fix dying memcg pinned by swapped out shmem pages Bingfang Guo via B4 Relay
` (3 preceding siblings ...)
2026-08-13 8:52 ` [PATCH RFC 4/5] memcg: add a helper to get online memcg from memcgid Bingfang Guo via B4 Relay
@ 2026-08-13 8:52 ` Bingfang Guo via B4 Relay
2026-08-13 13:26 ` [syzbot ci] Re: memcg: fix dying memcg pinned by swapped out shmem pages syzbot ci
5 siblings, 0 replies; 8+ messages in thread
From: Bingfang Guo via B4 Relay @ 2026-08-13 8:52 UTC (permalink / raw)
To: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, Dave Chinner, Qi Zheng,
David Hildenbrand, Lorenzo Stoakes, Kairui Song, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu
Cc: cgroups, linux-mm, linux-kernel, Bingfang Guo, Bingfang Guo
From: Bingfang Guo <bingfangguo@tencent.com>
mem_cgroup_from_private_id() looks up the objcg that owns the id and
returns the objcg's current memcg. After reparenting, that memcg can
differ from the one the id originally belonged to.
Callers such as the list lru and workingset refault code expect to get
back exactly the memcg referred to by the memcgid, so check that the
returned memcg still owns the id and return NULL otherwise, letting
the callers skip the entry.
Signed-off-by: Bingfang Guo <bingfangguo@tencent.com>
---
mm/list_lru.c | 2 +-
mm/memcontrol.c | 9 ++++++++-
mm/workingset.c | 6 +++++-
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/mm/list_lru.c b/mm/list_lru.c
index 36662d02ff963..bc956267f6835 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -428,7 +428,7 @@ unsigned long list_lru_walk_node(struct list_lru *lru, int nid,
xa_for_each(&lru->xa, index, mlru) {
rcu_read_lock();
memcg = mem_cgroup_from_private_id(index);
- if (!mem_cgroup_tryget(memcg)) {
+ if (!memcg || !mem_cgroup_tryget(memcg)) {
rcu_read_unlock();
continue;
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index fdf2e0d1f17e5..e7555eca77019 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3999,17 +3999,24 @@ void mem_cgroup_private_id_get(struct mem_cgroup *memcg, unsigned int n)
* @id: the memcg id to look up
*
* Caller must hold rcu_read_lock().
+ *
+ * @return: the memcg, or NULL if the memcg is already reparented.
*/
struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id)
{
struct obj_cgroup *objcg;
+ struct mem_cgroup *memcg;
WARN_ON_ONCE(!rcu_read_lock_held());
objcg = xa_load(&mem_cgroup_private_ids, id);
if (!objcg)
return NULL;
- return obj_cgroup_memcg(objcg);
+ memcg = obj_cgroup_memcg(objcg);
+ if (mem_cgroup_private_id(memcg) != id)
+ return NULL;
+
+ return memcg;
}
/**
diff --git a/mm/workingset.c b/mm/workingset.c
index f351798e723ac..b6e22536a5240 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -283,6 +283,10 @@ static bool lru_gen_test_recent(void *shadow, struct lruvec **lruvec,
memcg = mem_cgroup_from_private_id(memcg_id);
*lruvec = mem_cgroup_lruvec(memcg, pgdat);
+ /* reparented memcg loses its max_seq */
+ if (!memcg)
+ return false;
+
max_seq = READ_ONCE((*lruvec)->lrugen.max_seq);
max_seq &= (file ? EVICTION_MASK : EVICTION_MASK_ANON) >> LRU_REFS_WIDTH;
@@ -470,7 +474,7 @@ bool workingset_test_recent(void *shadow, bool file, bool *workingset,
* configurations instead.
*/
eviction_memcg = mem_cgroup_from_private_id(memcgid);
- if (!mem_cgroup_tryget(eviction_memcg))
+ if (!eviction_memcg || !mem_cgroup_tryget(eviction_memcg))
eviction_memcg = NULL;
rcu_read_unlock();
--
2.43.7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [syzbot ci] Re: memcg: fix dying memcg pinned by swapped out shmem pages
2026-08-13 8:52 [PATCH RFC 0/5] memcg: fix dying memcg pinned by swapped out shmem pages Bingfang Guo via B4 Relay
` (4 preceding siblings ...)
2026-08-13 8:52 ` [PATCH RFC 5/5] memcg: filter out reparented memcgs got using memcgid Bingfang Guo via B4 Relay
@ 2026-08-13 13:26 ` syzbot ci
5 siblings, 0 replies; 8+ messages in thread
From: syzbot ci @ 2026-08-13 13:26 UTC (permalink / raw)
To: akpm, axelrasmussen, baohua, bfguo, bingfangguo, cgroups, david,
david, devnull, hannes, kasong, linux-kernel, linux-mm, ljs,
mhocko, muchun.song, qi.zheng, roman.gushchin, shakeel.butt,
weixugc, yuanchu
Cc: syzbot, syzkaller-bugs
syzbot ci has tested the following series
[v1] memcg: fix dying memcg pinned by swapped out shmem pages
https://lore.kernel.org/all/20260813-memcgid-objcg-v1-0-83d21c685b77@tencent.com
* [PATCH RFC 1/5] memcg: move memcg private ID refcount to objcg
* [PATCH RFC 2/5] memcg: get stable memcg first before getting memcgid reference
* [PATCH RFC 3/5] memcg: remove retry logic in mem_cgroup_private_id_get_online
* [PATCH RFC 4/5] memcg: add a helper to get online memcg from memcgid
* [PATCH RFC 5/5] memcg: filter out reparented memcgs got using memcgid
and found the following issue:
WARNING: refcount bug in __mem_cgroup_uncharge_swap
Full report is available here:
https://ci.syzbot.org/series/3a8a2a5f-8b6d-4abf-954e-a1a18ee62748
***
WARNING: refcount bug in __mem_cgroup_uncharge_swap
tree: linux-next
URL: https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next
base: 288058d8db5d35623228d84f48d9bea3707d5c85
arch: amd64
compiler: Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
config: https://ci.syzbot.org/builds/35dac6f6-b44a-4ea8-9adf-2cdda5584e45/config
syz repro: https://ci.syzbot.org/findings/6e117c0b-d6e2-44a6-8e35-161cb33f2aff/syz_repro
------------[ cut here ]------------
refcount_t: underflow; use-after-free.
WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xb2/0x110 lib/refcount.c:28, CPU#0: syz.2.19/5863
Modules linked in:
CPU: 0 UID: 0 PID: 5863 Comm: syz.2.19 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
RIP: 0010:refcount_warn_saturate+0xb2/0x110 lib/refcount.c:28
Code: 64 84 8e 0b 67 48 0f b9 3a eb 4a e8 58 fa f2 fc 48 8d 3d 61 84 8e 0b 67 48 0f b9 3a eb 37 e8 45 fa f2 fc 48 8d 3d 5e 84 8e 0b <67> 48 0f b9 3a eb 24 e8 32 fa f2 fc 48 8d 3d 5b 84 8e 0b 67 48 0f
RSP: 0018:ffffc90003bbf1d0 EFLAGS: 00010293
RAX: ffffffff84d3ed9b RBX: 0000000000000003 RCX: ffff8881bdf58000
RDX: 0000000000000000 RSI: ffffffff8f363380 RDI: ffffffff90627200
RBP: 1ffffffff20ae968 R08: ffff8881bdf58000 R09: 0000000000000005
R10: 0000000000000004 R11: 0000000000000000 R12: ffff8881026f8000
R13: ffff88810a914100 R14: ffff88810a914134 R15: 1ffff110204df009
FS: 00007fe2ee63d6c0(0000) GS:ffff88818d960000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000056066247f0b8 CR3: 000000000e946000 CR4: 00000000000006f0
Call Trace:
<TASK>
mem_cgroup_take_from_private_id mm/memcontrol.c:4049 [inline]
__mem_cgroup_uncharge_swap+0x204/0x2a0 mm/memcontrol.c:5707
mem_cgroup_uncharge_swap include/linux/swap.h:520 [inline]
__swap_cluster_free_entries+0x735/0xc50 mm/swapfile.c:1955
swap_put_entries_cluster+0x3b1/0x4b0 mm/swapfile.c:1629
swap_put_entries_direct+0x137/0x210 mm/swapfile.c:2139
zap_nonpresent_ptes mm/memory.c:1838 [inline]
do_zap_pte_range mm/memory.c:1905 [inline]
zap_pte_range mm/memory.c:2003 [inline]
zap_pmd_range mm/memory.c:2089 [inline]
zap_pud_range mm/memory.c:2117 [inline]
zap_p4d_range mm/memory.c:2138 [inline]
__zap_vma_range+0x1d9d/0x4f10 mm/memory.c:2178
unmap_vmas+0x390/0x550 mm/memory.c:2247
exit_mmap+0x293/0x9f0 mm/mmap.c:1315
__mmput+0x118/0x420 kernel/fork.c:1187
exit_mm+0x221/0x2d0 kernel/exit.c:615
do_exit+0x6cd/0x2360 kernel/exit.c:997
do_group_exit+0x22d/0x2f0 kernel/exit.c:1152
get_signal+0x121b/0x12c0 kernel/signal.c:3046
arch_do_signal_or_restart+0xbb/0x860 arch/x86/kernel/signal.c:337
__exit_to_user_mode_loop kernel/entry/common.c:66 [inline]
exit_to_user_mode_loop+0x104/0x730 kernel/entry/common.c:101
__exit_to_user_mode_prepare include/linux/irq-entry-common.h:207 [inline]
syscall_exit_to_user_mode_prepare include/linux/irq-entry-common.h:230 [inline]
syscall_exit_to_user_mode include/linux/entry-common.h:318 [inline]
do_syscall_64+0x353/0x580 arch/x86/entry/syscall_64.c:100
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fe2ed79e0d9
Code: Unable to access opcode bytes at 0x7fe2ed79e0af.
RSP: 002b:00007fe2ee63d0e8 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
RAX: fffffffffffffe00 RBX: 00007fe2eda25fa8 RCX: 00007fe2ed79e0d9
RDX: 0000000000000000 RSI: 0000000000000080 RDI: 00007fe2eda25fa8
RBP: 00007fe2eda25fa0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fe2eda26038 R14: 00007fff0bed41b0 R15: 00007fff0bed4298
</TASK>
----------------
Code disassembly (best guess):
0: 64 84 8e 0b 67 48 0f test %cl,%fs:0xf48670b(%rsi)
7: b9 3a eb 4a e8 mov $0xe84aeb3a,%ecx
c: 58 pop %rax
d: fa cli
e: f2 fc repnz cld
10: 48 8d 3d 61 84 8e 0b lea 0xb8e8461(%rip),%rdi # 0xb8e8478
17: 67 48 0f b9 3a ud1 (%edx),%rdi
1c: eb 37 jmp 0x55
1e: e8 45 fa f2 fc call 0xfcf2fa68
23: 48 8d 3d 5e 84 8e 0b lea 0xb8e845e(%rip),%rdi # 0xb8e8488
* 2a: 67 48 0f b9 3a ud1 (%edx),%rdi <-- trapping instruction
2f: eb 24 jmp 0x55
31: e8 32 fa f2 fc call 0xfcf2fa68
36: 48 8d 3d 5b 84 8e 0b lea 0xb8e845b(%rip),%rdi # 0xb8e8498
3d: 67 addr32
3e: 48 rex.W
3f: 0f .byte 0xf
***
If these findings have caused you to resend the series or submit a
separate fix, please add the following tag to your commit message:
Tested-by: syzbot@syzkaller.appspotmail.com
---
This report is generated by a bot. It may contain errors.
syzbot ci engineers can be reached at syzkaller@googlegroups.com.
To test a fix for this bug, please reply with `#syz test`
(on a separate line) and attach the patch to the email.
Notes:
- The patch will be applied on top of the tested series (as an
incremental fix).
- To test a new version of the whole series, please send it directly
to syzbot@lists.linux.dev.
- Arguments like custom git repos and branches are not supported.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RFC 1/5] memcg: move memcg private ID refcount to objcg
2026-08-13 8:52 ` [PATCH RFC 1/5] memcg: move memcg private ID refcount to objcg Bingfang Guo via B4 Relay
@ 2026-08-13 15:09 ` Bingfang Guo
0 siblings, 0 replies; 8+ messages in thread
From: Bingfang Guo @ 2026-08-13 15:09 UTC (permalink / raw)
To: BINGFANG GUO
Cc: Johannes Weiner, Michal Hocko, Roman Gushchin, Shakeel Butt,
Muchun Song, Andrew Morton, Dave Chinner, Qi Zheng,
David Hildenbrand, Lorenzo Stoakes, Kairui Song, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, cgroups, linux-mm,
linux-kernel
> 2026年8月13日 16:52，Bingfang Guo via B4 Relay <devnull+bingfangguo.tencent.com@kernel.org> 写道：
>
> From: Bingfang Guo <bingfangguo@tencent.com>
>
> In the previous series by Muchun Song and Qi Zheng, folios are charged
> to the objcg and reparented as the memcg offlines. Same can be done to
> memcg private ID and its main user: swap entries. Make the memcgid
> xarray hold a pointer and a reference to an objcg of the memcg, which is
> used to find the memcg (or its parent) later on. The online state now
> pins the objcg instead of the css, so swapped out pages no longer pin
> the dying memcg.
>
> The id reference held by the online state is released in css_released()
> after reparenting instead of in css_offline(). This is the key
> invariant the rest of the series builds on: css_offline() runs while
> other css references may still be held, but css_released() only runs
> once the last reference is gone, so a caller holding a memcg reference
> can always count on the id refcount being alive. To prevent races
> between memcgid put in css offline and memcgid get, the release is put
> off till css_released, which could delay the release of the memcgid and
> the objcg it pins, but overall it should be fine.
>
> After reparenting, the objcg points to a live ancestor, so
> mem_cgroup_from_private_id() now returns that ancestor instead of the
> memcg the ID originally belonged to. Callers that need the exact memcg
> are fixed in patch 5.
>
> Signed-off-by: Bingfang Guo <bingfangguo@tencent.com>
> ---
> include/linux/memcontrol.h | 10 +++--
> mm/memcontrol.c | 99 ++++++++++++++++++++++++++++++++--------------
> 2 files changed, 76 insertions(+), 33 deletions(-)
>
> diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
> index 8170bb8066a22..c33ec7efad50b 100644
> --- a/include/linux/memcontrol.h
> +++ b/include/linux/memcontrol.h
> @@ -191,6 +191,7 @@ struct obj_cgroup {
> struct rcu_head rcu;
> };
> bool is_root;
> + refcount_t id_ref;
> };
>
> /*
> @@ -202,8 +203,8 @@ struct obj_cgroup {
> struct mem_cgroup {
> struct cgroup_subsys_state css;
>
> - /* Private memcg ID. Used to ID objects that outlive the cgroup */
> - struct mem_cgroup_private_id id;
> + /* The objcg holding private memcg ID. */
> + struct obj_cgroup *id_objcg;
>
> /* Accounted resources */
> struct page_counter memory; /* Both v1 & v2 */
> @@ -270,6 +271,9 @@ struct mem_cgroup {
> #endif
> int kmemcg_id;
>
> + /* Private memcg ID. Used to ID objects that outlive the cgroup */
> + int id;
> +
> struct memcg_vmstats_percpu __percpu *vmstats_percpu;
>
> #ifdef CONFIG_CGROUP_WRITEBACK
> @@ -820,7 +824,7 @@ static inline unsigned short mem_cgroup_private_id(struct mem_cgroup *memcg)
> if (mem_cgroup_disabled())
> return 0;
>
> - return memcg->id.id;
> + return memcg->id;
> }
> struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id);
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 8319ad8c5c23a..5f30e76ee93d7 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3697,7 +3697,7 @@ static void memcg_online_kmem(struct mem_cgroup *memcg)
>
> static_branch_enable(&memcg_kmem_online_key);
>
> - memcg->kmemcg_id = memcg->id.id;
> + memcg->kmemcg_id = memcg->id;
> }
>
> static void memcg_offline_kmem(struct mem_cgroup *memcg)
> @@ -3956,25 +3956,40 @@ static DEFINE_XARRAY_ALLOC1(mem_cgroup_private_ids);
>
> static void mem_cgroup_private_id_remove(struct mem_cgroup *memcg)
> {
> - if (memcg->id.id > 0) {
> - xa_erase(&mem_cgroup_private_ids, memcg->id.id);
> - memcg->id.id = 0;
> + if (memcg->id > 0) {
> + xa_erase(&mem_cgroup_private_ids, memcg->id);
> + memcg->id = 0;
> }
> }
>
> -static inline void mem_cgroup_private_id_put(struct mem_cgroup *memcg, unsigned int n)
> +/**
> + * @objcg: the objcg returned by mem_cgroup_private_id_objcg
> + * @id: the corresponding memcg private id
> + */
> +static void __mem_cgroup_private_id_put(struct obj_cgroup *objcg,
> + unsigned short id, unsigned int n)
> {
> - if (refcount_sub_and_test(n, &memcg->id.ref)) {
> - mem_cgroup_private_id_remove(memcg);
> + struct obj_cgroup *objcg_free;
>
> - /* Memcg ID pins CSS */
> - css_put(&memcg->css);
> + if (refcount_sub_and_test(n, &objcg->id_ref)) {
> + objcg_free = xa_erase(&mem_cgroup_private_ids, id);
> + VM_WARN_ON(objcg_free != objcg);
> +
> + /* Memcg ID pins the objcg */
> + obj_cgroup_put(objcg);
> }
> }
>
> +static inline void mem_cgroup_private_id_put(struct mem_cgroup *memcg, unsigned int n)
> +{
> + __mem_cgroup_private_id_put(memcg->id_objcg, memcg->id, n);
> +}
> +
> struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg, unsigned int n)
> {
> - while (!refcount_add_not_zero(n, &memcg->id.ref)) {
> + struct obj_cgroup *objcg = memcg->id_objcg;
> +
> + while (!refcount_add_not_zero(n, &objcg->id_ref)) {
> /*
> * The root cgroup cannot be destroyed, so it's refcount must
> * always be >= 1.
> @@ -3984,6 +3999,7 @@ struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg, un
> break;
> }
> memcg = parent_mem_cgroup(memcg);
> + objcg = memcg->id_objcg;
> }
> return memcg;
> }
> @@ -3996,8 +4012,29 @@ struct mem_cgroup *mem_cgroup_private_id_get_online(struct mem_cgroup *memcg, un
> */
> struct mem_cgroup *mem_cgroup_from_private_id(unsigned short id)
> {
> + struct obj_cgroup *objcg;
> WARN_ON_ONCE(!rcu_read_lock_held());
> - return xa_load(&mem_cgroup_private_ids, id);
> +
> + objcg = xa_load(&mem_cgroup_private_ids, id);
> + if (!objcg)
> + return NULL;
> +
> + return obj_cgroup_memcg(objcg);
> +}
> +
> +static struct mem_cgroup *mem_cgroup_take_from_private_id(unsigned short id, unsigned int n)
> +{
> + struct obj_cgroup *objcg;
> + struct mem_cgroup *memcg;
> +
> + objcg = xa_load(&mem_cgroup_private_ids, id);
> + if (!objcg)
> + return NULL;
> +
> + memcg = get_mem_cgroup_from_objcg(objcg);
> +
> + __mem_cgroup_private_id_put(objcg, id, n);
> + return memcg;
> }
>
> struct mem_cgroup *mem_cgroup_get_from_id(u64 id)
> @@ -4098,7 +4135,7 @@ static struct mem_cgroup *mem_cgroup_alloc(struct mem_cgroup *parent)
> if (!memcg)
> return ERR_PTR(-ENOMEM);
>
> - error = xa_alloc(&mem_cgroup_private_ids, &memcg->id.id, NULL,
> + error = xa_alloc(&mem_cgroup_private_ids, &memcg->id, NULL,
> XA_LIMIT(1, MEM_CGROUP_ID_MAX), GFP_KERNEL);
> if (error)
> goto fail;
> @@ -4243,9 +4280,10 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
> FLUSH_TIME);
> lru_gen_online_memcg(memcg);
>
> - /* Online state pins memcg ID, memcg ID pins CSS */
> - refcount_set(&memcg->id.ref, 1);
> - css_get(css);
> + /* CSS pins memcg ID, memcg ID pins obj cgroup */
> + memcg->id_objcg = memcg->nodeinfo[0]->objcg;
> + refcount_set(&memcg->id_objcg->id_ref, 1);
> + obj_cgroup_get(memcg->id_objcg);
>
> /*
> * Ensure mem_cgroup_from_private_id() works once we're fully online.
> @@ -4257,7 +4295,7 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
> * publish it here at the end of onlining. This matches the
> * regular ID destruction during offlining.
> */
> - xa_store(&mem_cgroup_private_ids, memcg->id.id, memcg, GFP_KERNEL);
> + xa_store(&mem_cgroup_private_ids, memcg->id, objcg, GFP_KERNEL);
Sashiko pointed out that objcg here is pointing to the wrong node here.
I made a mistake here while rebasing the patch set. Also the problem reported by syzbot and in
patch 2 is also caused by this…
It should be like this:
@@ -4257,7 +4295,7 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
* publish it here at the end of onlining. This matches the
* regular ID destruction during offlining.
*/
- xa_store(&mem_cgroup_private_ids, memcg->id.id, memcg, GFP_KERNEL);
+ xa_store(&mem_cgroup_private_ids, memcg->id, memcg->id_objcg, GFP_KERNEL);
But it also says that it could go wrong if node 0 is not present on sparse NUMA setups, so I
think it might be better to just use the last objcg set up above:
@@ -4294,7 +4295,7 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
lru_gen_online_memcg(memcg);
/* CSS pins memcg ID, memcg ID pins obj cgroup */
- memcg->id_objcg = memcg->nodeinfo[0]->objcg;
+ memcg->id_objcg = objcg;
refcount_set(&memcg->id_objcg->id_ref, 1);
obj_cgroup_get(memcg->id_objcg);
>
> return 0;
> free_objcg:
> @@ -4308,8 +4346,6 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
> lru_gen_offline_memcg(memcg);
>
> drain_all_stock(memcg);
> -
> - mem_cgroup_private_id_put(memcg, 1);
> }
>
> static void mem_cgroup_css_released(struct cgroup_subsys_state *css)
> @@ -4318,6 +4354,9 @@ static void mem_cgroup_css_released(struct cgroup_subsys_state *css)
>
> invalidate_reclaim_iterators(memcg);
> lru_gen_release_memcg(memcg);
> +
> + mem_cgroup_private_id_put(memcg, 1);
Sashiko says that if memcg onlining fails early, this could result in null pointer dereference.
I think we can fix it like this since mem_cgroup_private_id_put is only used in css releasing:
@@ -3980,9 +3980,13 @@ static void __mem_cgroup_private_id_put(struct obj_cgroup *objcg,
}
}
-static inline void mem_cgroup_private_id_put(struct mem_cgroup *memcg, unsigned int n)
+static inline void mem_cgroup_private_id_release(struct mem_cgroup *memcg, unsigned int n)
{
+ if (!memcg->id_objcg)
+ return;
+
__mem_cgroup_private_id_put(memcg->id_objcg, memcg->id, n);
+ memcg->id_objcg = NULL;
}
void mem_cgroup_private_id_get(struct mem_cgroup *memcg, unsigned int n)
@@ -4367,9 +4371,7 @@ static void mem_cgroup_css_released(struct cgroup_subsys_state *css)
invalidate_reclaim_iterators(memcg);
lru_gen_release_memcg(memcg);
-
- mem_cgroup_private_id_put(memcg, 1);
- memcg->id_objcg = NULL;
+ mem_cgroup_private_id_release(memcg, 1);
}
static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
> + memcg->id_objcg = NULL;
> }
>
> static void mem_cgroup_css_free(struct cgroup_subsys_state *css)
> @@ -5651,19 +5690,19 @@ void __mem_cgroup_uncharge_swap(unsigned short id, unsigned int nr_pages)
> {
> struct mem_cgroup *memcg;
>
> - rcu_read_lock();
> - memcg = mem_cgroup_from_private_id(id);
> - if (memcg) {
> - if (!mem_cgroup_is_root(memcg)) {
> - if (do_memsw_account())
> - page_counter_uncharge(&memcg->memsw, nr_pages);
> - else
> - page_counter_uncharge(&memcg->swap, nr_pages);
> - }
> - mod_memcg_state(memcg, MEMCG_SWAP, -nr_pages);
> - mem_cgroup_private_id_put(memcg, nr_pages);
> + memcg = mem_cgroup_take_from_private_id(id, nr_pages);
> + if (!memcg)
> + return;
> +
> + if (!mem_cgroup_is_root(memcg)) {
> + if (do_memsw_account())
> + page_counter_uncharge(&memcg->memsw, nr_pages);
> + else
> + page_counter_uncharge(&memcg->swap, nr_pages);
> }
> - rcu_read_unlock();
> + mod_memcg_state(memcg, MEMCG_SWAP, -nr_pages);
> +
> + mem_cgroup_put(memcg);
> }
>
> long mem_cgroup_get_nr_swap_pages(struct mem_cgroup *memcg)
>
> --
> 2.43.7
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-13 15:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 8:52 [PATCH RFC 0/5] memcg: fix dying memcg pinned by swapped out shmem pages Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 1/5] memcg: move memcg private ID refcount to objcg Bingfang Guo via B4 Relay
2026-08-13 15:09 ` Bingfang Guo
2026-08-13 8:52 ` [PATCH RFC 2/5] memcg: get stable memcg first before getting memcgid reference Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 3/5] memcg: remove retry logic in mem_cgroup_private_id_get_online Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 4/5] memcg: add a helper to get online memcg from memcgid Bingfang Guo via B4 Relay
2026-08-13 8:52 ` [PATCH RFC 5/5] memcg: filter out reparented memcgs got using memcgid Bingfang Guo via B4 Relay
2026-08-13 13:26 ` [syzbot ci] Re: memcg: fix dying memcg pinned by swapped out shmem pages syzbot ci
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox