Re: [PATCH v1 06/26] mm: memcontrol: return root object cgroup for root memory cgroup
From: Qi Zheng
Date: Wed Nov 19 2025 - 01:41:50 EST
On 11/18/25 8:12 PM, Harry Yoo wrote:
On Tue, Nov 18, 2025 at 07:28:41PM +0800, Qi Zheng wrote:
Hi Harry,
On 11/17/25 5:17 PM, Harry Yoo wrote:
On Tue, Oct 28, 2025 at 09:58:19PM +0800, Qi Zheng wrote:
From: Muchun Song <songmuchun@xxxxxxxxxxxxx>
Memory cgroup functions such as get_mem_cgroup_from_folio() and
get_mem_cgroup_from_mm() return a valid memory cgroup pointer,
even for the root memory cgroup. In contrast, the situation for
object cgroups has been different.
Previously, the root object cgroup couldn't be returned because
it didn't exist. Now that a valid root object cgroup exists, for
the sake of consistency, it's necessary to align the behavior of
object-cgroup-related operations with that of memory cgroup APIs.
Signed-off-by: Muchun Song <songmuchun@xxxxxxxxxxxxx>
Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
---
include/linux/memcontrol.h | 29 +++++++++++++++++-------
mm/memcontrol.c | 45 ++++++++++++++++++++------------------
mm/percpu.c | 2 +-
3 files changed, 46 insertions(+), 30 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 6185d8399a54e..9fdbd4970021d 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -332,6 +332,7 @@ struct mem_cgroup {
#define MEMCG_CHARGE_BATCH 64U
extern struct mem_cgroup *root_mem_cgroup;
+extern struct obj_cgroup *root_obj_cgroup;
enum page_memcg_data_flags {
/* page->memcg_data is a pointer to an slabobj_ext vector */
@@ -549,6 +550,11 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)
return (memcg == root_mem_cgroup);
}
+static inline bool obj_cgroup_is_root(const struct obj_cgroup *objcg)
+{
+ return objcg == root_obj_cgroup;
+}
After reparenting, an objcg may satisfy objcg->memcg == root_mem_cgroup
while objcg != root_obj_cgroup. Should they be considered as
root objcgs?
Indeed, it's pointless to charge to root_mem_cgroup (objcg->memcg).
So it should be:
static inline bool obj_cgroup_is_root(const struct obj_cgroup *objcg)
{
return (objcg == root_obj_cgroup) || (objcg->memcg == root_mem_cgroup);
}
Thanks and tomorrow I'll try to review if will be correct ;)
static inline bool mem_cgroup_disabled(void)
{
return !cgroup_subsys_enabled(memory_cgrp_subsys);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 2afd7f99ca101..d484b632c790f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2871,7 +2865,7 @@ int __memcg_kmem_charge_page(struct page *page, gfp_t gfp, int order)
int ret = 0;
objcg = current_obj_cgroup();
- if (objcg) {
+ if (!obj_cgroup_is_root(objcg)) {
Now that we support the page and slab allocators support allocating memory
in NMI contexts (on some archs), current_obj_cgroup() can return NULL
if (IS_ENABLED(CONFIG_MEMCG_NMI_UNSAFE) && in_nmi()) returns true
(then it leads to a NULL-pointer-deref bug).
But IIUC this is applied to kmem charging only (as they use this_cpu ops
for stats update), and we don't have to apply the same restriction to
charging LRU pages with objcg.
Maybe Shakeel has more insight on this.
Link: https://lore.kernel.org/all/20250519063142.111219-1-shakeel.butt@xxxxxxxxx
Thanks for this information, and it seems there's nothing wrong here.
I mean at least we should not introduce a NULL-pointer-deref bug in
__memcg_kmem_charge_page(), by assuming objcg returned by
current_obj_cgroup() is non-NULL?
1. Someone allocates non-slab kmem in an NMI context (in_nmi() == true),
calling __memcg_kmem_charge_page().
2. current_obj_cgruop() returns NULL because the architectures
has CONFIG_MEMCG_NMI_UNSAFE and it's in an NMI context.
3. obj_cgroup_is_root() returns false since
objcg (NULL) != root_obj_cgroup
4. we pass NULL to obj_cgroup_charge_pages().
5. obj_cgroup_charge_pages() calls get_mem_cgroup_from_objcg(),
dereference objcg->memcg (! a NULL-pointer-deref).
Oh, indeed. After adding MEMCG_NMI_UNSAFE, we should first check
if objcg is NULL.
Thanks!
Thanks,
Qi