slab: Add check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem

PATCH In review Hao Ge 6 participants 12 messages Oct 14, 2025 – Oct 15, 2025

Subsystems (3)

Memory management cgroup memcg

lore

Share & tools

lore

Hao Ge

author

2025-10-14 15:28 UTC #0 lore raw

PATCH v3 1/1 slab: Add check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem

From: Hao Ge <[email protected]>

Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.

If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.

So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.

Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")

Suggested-by Harry Yoo <[email protected]>

Signed-off-by Hao Ge <[email protected]>

---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>

---
 include/linux/memcontrol.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

include/linux/memcontrol.h +3 -1

index 873e510d6f8d..7ed15f858dc4 100644
@@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
 static inline bool folio_memcg_kmem(struct folio *folio)
 {
 	VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
-	VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
+	VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
+			(folio->memcg_data & MEMCG_DATA_OBJEXTS),
+			folio);
 	return folio->memcg_data & MEMCG_DATA_KMEM;
 }

Signature · 1 line

2.25.1

Suren Baghdasaryan

Reviewed-by on v3 1/1

2025-10-14 16:12 UTC #1 lore raw

Quoted 13 lines · Hao Ge

From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.

Quoted 5 lines

So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>

Reviewed-by Suren Baghdasaryan <[email protected]>

nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.

Quoted 25 lines

---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>

---
 include/linux/memcontrol.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

include/linux/memcontrol.h +3 -2

index 873e510d6f8d..7ed15f858dc4 100644
@@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
 static inline bool folio_memcg_kmem(struct folio *folio)
 {
        VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
-       VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
+       VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
+                       (folio->memcg_data & MEMCG_DATA_OBJEXTS),
+                       folio);
        return folio->memcg_data & MEMCG_DATA_KMEM;
 }
 
--

2.25.1

Shakeel Butt

maintainer Acked-by

2025-10-14 20:15 UTC #2 lore raw

Quoted 22 lines · Suren Baghdasaryan

Quoted 13 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.
Quoted 5 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>

Quoted 8 lines

nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.

With Suren's suggestion, you can add:

Acked-by Shakeel Butt <[email protected]>

Vlastimil Babka

2025-10-14 20:58 UTC #3 lore raw

Quoted 32 lines · Shakeel Butt

Quoted 22 lines · Suren Baghdasaryan
Quoted 13 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.
Quoted 5 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>
Quoted 8 lines
nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.

Quoted 3 lines

With Suren's suggestion, you can add:
Acked-by Shakeel Butt <[email protected]>

Thanks, I added Suren's suggestion and pushed to slab/for-next-fixes:

https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git/commit/?h=slab/for-next-fixes&id=711c435c89e59ee32bf8bb1c0d875a07931da5a8

Resisted the impulse to change the single VM_BUG_ON_FOLIO to VM_WARN_ON_ONCE_FOLIO because we're still going to do that systematically, right?

Shakeel Butt

maintainer

2025-10-14 22:41 UTC #4 lore raw

Quoted 37 lines · Vlastimil Babka

Quoted 32 lines · Shakeel Butt
Quoted 22 lines · Suren Baghdasaryan
Quoted 13 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.
Quoted 5 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>
Quoted 8 lines
nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.
Quoted 3 lines
With Suren's suggestion, you can add:
Acked-by Shakeel Butt <[email protected]>

Quoted 7 lines

Thanks, I added Suren's suggestion and pushed to slab/for-next-fixes:
https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/slab.git/commit/?h=slab/for-next-fixes&id=711c435c89e59ee32bf8bb1c0d875a07931da5a8
Resisted the impulse to change the single VM_BUG_ON_FOLIO to VM_WARN_ON_ONCE_FOLIO because we're still going to do that systematically, right?

Oh is there some coordinated effort happening for this conversion?

Harry Yoo

Reviewed-by

2025-10-15 09:26 UTC #5 lore raw

In reply to Suren Baghdasaryan

Quoted 9 lines · Suren Baghdasaryan

Quoted 8 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.

Quoted 4 lines

Quoted 4 lines
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.

nit: yesterday I was confused that this is sanity checks in buddy complaining folio->memcg_data not being cleared, but it's actually folio_memcg_kmem() complaining that MEMCG_OBJEXTS_DATA flag is set on non-slab folios (in free_pages_prepare(), if PageMemcgKmem(page) -> __memcg_kmem_uncharge_page())) So the paragraph above should be updated?

And as a side question, we clear slab->obj_exts when freeing obj_ext array, but don't clear OBJEXTS_ALLOC_FAIL when freeing a slab? That's not good.

Quoted 8 lines

Quoted 5 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>

Quoted 8 lines

nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.

Agreed.

In general LGTM,

Reviewed-by Harry Yoo <[email protected]>

By the way, maybe it'd be nice to introduce a new helper function that properly checks MEMCG_DATA_OBJEXTS flag.

Quoted 7 lines

~/slab (slab/for-next-fixes)> git grep -n MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:337: MEMCG_DATA_OBJEXTS = (1UL << 0),
include/linux/memcontrol.h:344:#define __OBJEXTS_ALLOC_FAIL MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:358: * MEMCG_DATA_OBJEXTS.
include/linux/memcontrol.h:400: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
include/linux/memcontrol.h:421: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);

these two,

Quoted 1 line

include/linux/memcontrol.h:492: if (memcg_data & MEMCG_DATA_OBJEXTS)

this,

Quoted 5 lines

include/linux/memcontrol.h:538: (folio->memcg_data & MEMCG_DATA_OBJEXTS),
include/linux/memcontrol.h:1491: * if MEMCG_DATA_OBJEXTS is set.
mm/kfence/core.c:624: MEMCG_DATA_OBJEXTS;
mm/page_owner.c:513: if (memcg_data & MEMCG_DATA_OBJEXTS)

this,

Quoted 6 lines

mm/slab.h:541: * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
mm/slab.h:543: VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
mm/slub.c:2137: new_exts |= MEMCG_DATA_OBJEXTS;
tools/mm/show_page_info.py:55: MEMCG_DATA_OBJEXTS = prog.constant("MEMCG_DATA_OBJEXTS").value_()
tools/mm/show_page_info.py:59: if memcg_data & MEMCG_DATA_OBJEXTS:

and this do not look good.

I mean technically they are fine since OBJEXTS_ALLOC_FAIL is set on slabs only, but that's just a coincidence.

Quoted 24 lines

---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>

---
 include/linux/memcontrol.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

include/linux/memcontrol.h +3 -2

index 873e510d6f8d..7ed15f858dc4 100644
@@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
 static inline bool folio_memcg_kmem(struct folio *folio)
 {
        VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
-       VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
+       VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
+                       (folio->memcg_data & MEMCG_DATA_OBJEXTS),
+                       folio);
        return folio->memcg_data & MEMCG_DATA_KMEM;
 }
 
--

2.25.1

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Vlastimil Babka

2025-10-15 09:54 UTC #6 lore raw

Quoted 15 lines · Harry Yoo

Quoted 9 lines · Suren Baghdasaryan
Quoted 8 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
Quoted 4 lines
Quoted 4 lines
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.

Quoted 8 lines

nit: yesterday I was confused that this is sanity checks in buddy complaining folio->memcg_data not being cleared, but it's actually folio_memcg_kmem() complaining that MEMCG_OBJEXTS_DATA flag is set on non-slab folios (in free_pages_prepare(), if PageMemcgKmem(page) -> __memcg_kmem_uncharge_page())) So the paragraph above should be updated?
And as a side question, we clear slab->obj_exts when freeing obj_ext array, but don't clear OBJEXTS_ALLOC_FAIL when freeing a slab? That's not good.

Hm great point. We should rather make sure it's cleared always, instead of adjusting the debugging check, which shouldn't be then necessary, right?

Quoted 17 lines

Quoted 8 lines
Quoted 5 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>
Quoted 8 lines
nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.

Quoted 7 lines

Agreed.
In general LGTM,
Reviewed-by Harry Yoo <[email protected]>
By the way, maybe it'd be nice to introduce a new helper function that properly checks MEMCG_DATA_OBJEXTS flag.

I thought so too at first...

Quoted 27 lines

Quoted 7 lines
~/slab (slab/for-next-fixes)> git grep -n MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:337: MEMCG_DATA_OBJEXTS = (1UL << 0),
include/linux/memcontrol.h:344:#define __OBJEXTS_ALLOC_FAIL MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:358: * MEMCG_DATA_OBJEXTS.
include/linux/memcontrol.h:400: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
include/linux/memcontrol.h:421: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
these two,
Quoted 1 line
include/linux/memcontrol.h:492: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 5 lines
include/linux/memcontrol.h:538: (folio->memcg_data & MEMCG_DATA_OBJEXTS),
include/linux/memcontrol.h:1491: * if MEMCG_DATA_OBJEXTS is set.
mm/kfence/core.c:624: MEMCG_DATA_OBJEXTS;
mm/page_owner.c:513: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 4 lines
mm/slab.h:541: * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
mm/slab.h:543: VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
mm/slub.c:2137: new_exts |= MEMCG_DATA_OBJEXTS;
tools/mm/show_page_info.py:55: MEMCG_DATA_OBJEXTS = prog.constant("MEMCG_DATA_OBJEXTS").value_()

Quoted 6 lines

Quoted 1 line
tools/mm/show_page_info.py:59: if memcg_data & MEMCG_DATA_OBJEXTS:
and this do not look good.
I mean technically they are fine since OBJEXTS_ALLOC_FAIL is set on slabs only, but that's just a coincidence.

And checked the the other debugging checks too. But then thought it's better that if these are not expected to see slabs, then they should not be adjusted. I don't see it as a coincidence but as intention to keep it slab specific. It will be also more future proof for the upcoming separation of struct slab from struct page.

Quoted 25 lines

Quoted 24 lines

---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>

---
 include/linux/memcontrol.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

include/linux/memcontrol.h +3 -2

index 873e510d6f8d..7ed15f858dc4 100644
@@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
 static inline bool folio_memcg_kmem(struct folio *folio)
 {
        VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
-       VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
+       VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
+                       (folio->memcg_data & MEMCG_DATA_OBJEXTS),
+                       folio);
        return folio->memcg_data & MEMCG_DATA_KMEM;
 }
 
--

2.25.1

Harry Yoo

2025-10-15 10:28 UTC #7 lore raw

Quoted 22 lines · Vlastimil Babka

Quoted 21 lines · Harry Yoo
Quoted 9 lines · Suren Baghdasaryan
Quoted 8 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
Quoted 4 lines
Quoted 4 lines
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.
nit: yesterday I was confused that this is sanity checks in buddy complaining folio->memcg_data not being cleared, but it's actually folio_memcg_kmem() complaining that MEMCG_OBJEXTS_DATA flag is set on non-slab folios (in free_pages_prepare(), if PageMemcgKmem(page) -> __memcg_kmem_uncharge_page())) So the paragraph above should be updated?

Quoted 5 lines

Quoted 2 lines
And as a side question, we clear slab->obj_exts when freeing obj_ext array, but don't clear OBJEXTS_ALLOC_FAIL when freeing a slab? That's not good.
Hm great point. We should rather make sure it's cleared always, instead of adjusting the debugging check, which shouldn't be then necessary, right?

Yeah folio_memcg_kmem() isn't supposed to be called on slabs anyway (it's not a slab at the time we free it to buddy), so we don't have to adjust the debug check.

Quoted 58 lines

Quoted 17 lines
Quoted 8 lines
Quoted 5 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>
Quoted 8 lines
nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.
Quoted 7 lines
Agreed.
In general LGTM,
Reviewed-by Harry Yoo <[email protected]>
By the way, maybe it'd be nice to introduce a new helper function that properly checks MEMCG_DATA_OBJEXTS flag.
I thought so too at first...
Quoted 31 lines
Quoted 7 lines
~/slab (slab/for-next-fixes)> git grep -n MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:337: MEMCG_DATA_OBJEXTS = (1UL << 0),
include/linux/memcontrol.h:344:#define __OBJEXTS_ALLOC_FAIL MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:358: * MEMCG_DATA_OBJEXTS.
include/linux/memcontrol.h:400: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
include/linux/memcontrol.h:421: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
these two,
Quoted 1 line
include/linux/memcontrol.h:492: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 5 lines
include/linux/memcontrol.h:538: (folio->memcg_data & MEMCG_DATA_OBJEXTS),
include/linux/memcontrol.h:1491: * if MEMCG_DATA_OBJEXTS is set.
mm/kfence/core.c:624: MEMCG_DATA_OBJEXTS;
mm/page_owner.c:513: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 6 lines
mm/slab.h:541: * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
mm/slab.h:543: VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
mm/slub.c:2137: new_exts |= MEMCG_DATA_OBJEXTS;
tools/mm/show_page_info.py:55: MEMCG_DATA_OBJEXTS = prog.constant("MEMCG_DATA_OBJEXTS").value_()
tools/mm/show_page_info.py:59: if memcg_data & MEMCG_DATA_OBJEXTS:
and this do not look good.

Quoted 8 lines

Quoted 2 lines
I mean technically they are fine since OBJEXTS_ALLOC_FAIL is set on slabs only, but that's just a coincidence.
And checked the the other debugging checks too. But then thought it's better that if these are not expected to see slabs, then they should not be adjusted. I don't see it as a coincidence but as intention to keep it slab specific. It will be also more future proof for the upcoming separation of struct slab from struct page.

Then we're intentionally using (folio->memcg_data & MEMCG_DATA_OBJEXTS) check as a way to determine whether the folio is a slab (either slabobj_ext array allocation succeeded or failed).

That makes sense to me!

Quoted 26 lines

Quoted 25 lines

Quoted 24 lines

---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>

---
 include/linux/memcontrol.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

include/linux/memcontrol.h +3 -2

index 873e510d6f8d..7ed15f858dc4 100644
@@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
 static inline bool folio_memcg_kmem(struct folio *folio)
 {
        VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
-       VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
+       VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
+                       (folio->memcg_data & MEMCG_DATA_OBJEXTS),
+                       folio);
        return folio->memcg_data & MEMCG_DATA_KMEM;
 }
 
--

2.25.1

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Vlastimil Babka

2025-10-15 10:37 UTC #8 lore raw

Quoted 26 lines · Harry Yoo

Quoted 25 lines · Vlastimil Babka
Quoted 23 lines · Harry Yoo
Quoted 9 lines · Suren Baghdasaryan
Quoted 8 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
Quoted 4 lines
Quoted 4 lines
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.
nit: yesterday I was confused that this is sanity checks in buddy complaining folio->memcg_data not being cleared, but it's actually folio_memcg_kmem() complaining that MEMCG_OBJEXTS_DATA flag is set on non-slab folios (in free_pages_prepare(), if PageMemcgKmem(page) -> __memcg_kmem_uncharge_page())) So the paragraph above should be updated?
And as a side question, we clear slab->obj_exts when freeing obj_ext array, but don't clear OBJEXTS_ALLOC_FAIL when freeing a slab? That's not good.

Quoted 6 lines

Quoted 2 lines
Hm great point. We should rather make sure it's cleared always, instead of adjusting the debugging check, which shouldn't be then necessary, right?
Yeah folio_memcg_kmem() isn't supposed to be called on slabs anyway (it's not a slab at the time we free it to buddy), so we don't have to adjust the debug check.

Great. Hao Ge, can you please send v4 that instead of adjusting the VM_BUG_ON modifies free_slab_obj_exts() to always clear slab->obj_exts? Thanks!

Quoted 100 lines

Quoted 58 lines
Quoted 17 lines
Quoted 8 lines
Quoted 5 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>
Quoted 8 lines
nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.
Quoted 7 lines
Agreed.
In general LGTM,
Reviewed-by Harry Yoo <[email protected]>
By the way, maybe it'd be nice to introduce a new helper function that properly checks MEMCG_DATA_OBJEXTS flag.
I thought so too at first...
Quoted 31 lines
Quoted 7 lines
~/slab (slab/for-next-fixes)> git grep -n MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:337: MEMCG_DATA_OBJEXTS = (1UL << 0),
include/linux/memcontrol.h:344:#define __OBJEXTS_ALLOC_FAIL MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:358: * MEMCG_DATA_OBJEXTS.
include/linux/memcontrol.h:400: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
include/linux/memcontrol.h:421: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
these two,
Quoted 1 line
include/linux/memcontrol.h:492: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 5 lines
include/linux/memcontrol.h:538: (folio->memcg_data & MEMCG_DATA_OBJEXTS),
include/linux/memcontrol.h:1491: * if MEMCG_DATA_OBJEXTS is set.
mm/kfence/core.c:624: MEMCG_DATA_OBJEXTS;
mm/page_owner.c:513: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 6 lines
mm/slab.h:541: * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
mm/slab.h:543: VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
mm/slub.c:2137: new_exts |= MEMCG_DATA_OBJEXTS;
tools/mm/show_page_info.py:55: MEMCG_DATA_OBJEXTS = prog.constant("MEMCG_DATA_OBJEXTS").value_()
tools/mm/show_page_info.py:59: if memcg_data & MEMCG_DATA_OBJEXTS:
and this do not look good.
Quoted 8 lines
Quoted 2 lines
I mean technically they are fine since OBJEXTS_ALLOC_FAIL is set on slabs only, but that's just a coincidence.
And checked the the other debugging checks too. But then thought it's better that if these are not expected to see slabs, then they should not be adjusted. I don't see it as a coincidence but as intention to keep it slab specific. It will be also more future proof for the upcoming separation of struct slab from struct page.
Then we're intentionally using (folio->memcg_data & MEMCG_DATA_OBJEXTS) check as a way to determine whether the folio is a slab (either slabobj_ext array allocation succeeded or failed).
That makes sense to me!
Quoted 26 lines
Quoted 25 lines
Quoted 24 lines
Quoted 24 lines
---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>
---
 include/linux/memcontrol.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
include/linux/memcontrol.h +3 -2
index 873e510d6f8d..7ed15f858dc4 100644
@@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
 static inline bool folio_memcg_kmem(struct folio *folio)
 {
        VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
-       VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
+       VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
+                       (folio->memcg_data & MEMCG_DATA_OBJEXTS),
+                       folio);
        return folio->memcg_data & MEMCG_DATA_KMEM;
 }
 
--
2.25.1

Hao Ge

author

2025-10-15 11:24 UTC #9 lore raw

Hi Vlastimil and Harry

Thank you for your professional guidance.

Quoted 16 lines · Vlastimil Babka

Quoted 15 lines · Harry Yoo
Quoted 14 lines · Vlastimil Babka
Quoted 13 lines · Harry Yoo
Quoted 12 lines · Suren Baghdasaryan
Quoted 11 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
nit: yesterday I was confused that this is sanity checks in buddy complaining folio->memcg_data not being cleared, but it's actually folio_memcg_kmem() complaining that MEMCG_OBJEXTS_DATA flag is set on non-slab folios (in free_pages_prepare(), if PageMemcgKmem(page) -> __memcg_kmem_uncharge_page())) So the paragraph above should be updated?

Hi Harry

We don't need to update the paragraph.

We did have cgroups running at that time, but they had no connection to this page.

The entry "[ 7108.343500] memcg:1" can also be seen in the v1 logs,

Therefore, the situation at that time was indeed consistent with what I described above.

As discussed below, this only occurs because the OBJEXTS_ALLOC_FAIL flag

was not cleared when the slab was about to be freed.

Or have I missed anything?

Quoted 10 lines

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
And as a side question, we clear slab->obj_exts when freeing obj_ext array, but don't clear OBJEXTS_ALLOC_FAIL when freeing a slab? That's not good.
Hm great point. We should rather make sure it's cleared always, instead of adjusting the debugging check, which shouldn't be then necessary, right?
Yeah folio_memcg_kmem() isn't supposed to be called on slabs anyway (it's not a slab at the time we free it to buddy), so we don't have to adjust the debug check.
Great. Hao Ge, can you please send v4 that instead of adjusting the VM_BUG_ON modifies free_slab_obj_exts() to always clear slab->obj_exts? Thanks!

Okay, I will send v4 as soon as possible.

Thanks

Best Regards

Hao

Quoted 86 lines

Quoted 86 lines
Quoted 49 lines
Quoted 16 lines
Quoted 8 lines
Quoted 6 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>
Quoted 8 lines
nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.
Quoted 7 lines
Agreed.
In general LGTM,
Reviewed-by Harry Yoo <[email protected]>
By the way, maybe it'd be nice to introduce a new helper function that properly checks MEMCG_DATA_OBJEXTS flag.
I thought so too at first...
Quoted 24 lines
Quoted 6 lines
~/slab (slab/for-next-fixes)> git grep -n MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:337: MEMCG_DATA_OBJEXTS = (1UL << 0),
include/linux/memcontrol.h:344:#define __OBJEXTS_ALLOC_FAIL MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:358: * MEMCG_DATA_OBJEXTS.
include/linux/memcontrol.h:400: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
include/linux/memcontrol.h:421: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
these two,
Quoted 1 line
include/linux/memcontrol.h:492: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 4 lines
include/linux/memcontrol.h:538: (folio->memcg_data & MEMCG_DATA_OBJEXTS),
include/linux/memcontrol.h:1491: * if MEMCG_DATA_OBJEXTS is set.
mm/kfence/core.c:624: MEMCG_DATA_OBJEXTS;
mm/page_owner.c:513: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 5 lines
mm/slab.h:541: * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
mm/slab.h:543: VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
mm/slub.c:2137: new_exts |= MEMCG_DATA_OBJEXTS;
tools/mm/show_page_info.py:55: MEMCG_DATA_OBJEXTS = prog.constant("MEMCG_DATA_OBJEXTS").value_()
tools/mm/show_page_info.py:59: if memcg_data & MEMCG_DATA_OBJEXTS:
and this do not look good.
Quoted 7 lines
Quoted 2 lines
I mean technically they are fine since OBJEXTS_ALLOC_FAIL is set on slabs only, but that's just a coincidence.
And checked the the other debugging checks too. But then thought it's better that if these are not expected to see slabs, then they should not be adjusted. I don't see it as a coincidence but as intention to keep it slab specific. It will be also more future proof for the upcoming separation of struct slab from struct page.
Then we're intentionally using (folio->memcg_data & MEMCG_DATA_OBJEXTS) check as a way to determine whether the folio is a slab (either slabobj_ext array allocation succeeded or failed).
That makes sense to me!
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>
---
  include/linux/memcontrol.h | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
include/linux/memcontrol.h +3 -2
index 873e510d6f8d..7ed15f858dc4 100644
@@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
  static inline bool folio_memcg_kmem(struct folio *folio)
  {
         VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
-       VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
+       VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
+                       (folio->memcg_data & MEMCG_DATA_OBJEXTS),
+                       folio);
         return folio->memcg_data & MEMCG_DATA_KMEM;
  }
 
--
2.25.1

Harry Yoo

2025-10-15 11:41 UTC #10 lore raw

Quoted 36 lines · Hao Ge

Hi Vlastimil and Harry
Thank you for your professional guidance.
Quoted 16 lines · Vlastimil Babka
Quoted 15 lines · Harry Yoo
Quoted 14 lines · Vlastimil Babka
Quoted 13 lines · Harry Yoo
Quoted 12 lines · Suren Baghdasaryan
Quoted 11 lines · Hao Ge
From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
nit: yesterday I was confused that this is sanity checks in buddy complaining folio->memcg_data not being cleared, but it's actually folio_memcg_kmem() complaining that MEMCG_OBJEXTS_DATA flag is set on non-slab folios (in free_pages_prepare(), if PageMemcgKmem(page) -> __memcg_kmem_uncharge_page())) So the paragraph above should be updated?
Hi Harry
We don't need to update the paragraph.
We did have cgroups running at that time, but they had no connection to this page.
The entry "[ 7108.343500] memcg:1" can also be seen in the v1 logs,

Quoted 8 lines

Therefore, the situation at that time was indeed consistent with what I described above.
As discussed below, this only occurs because the OBJEXTS_ALLOC_FAIL flag
was not cleared when the slab was about to be freed.
Or have I missed anything?

Oh, I meant "the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio" is misleading.

Not clearing folio->memcg_data before freeing is considered an error, and page_expected_state() indeed checks if ->memcg_data is cleared and reports an error if it's not cleared. But that's not what we're talking about, right?

Instead, what we're looking at is that "the BUG is triggered because the OBJEXTS_ALLOC_FAIL flag was not cleared, causing it to be interpreted as a kmem folio (non-slab) with MEMCG_OBJEXTS_DATA flag set, which is invalid because MEMCG_OBJEXTS_DATA is supposed to be set only on slabs."

Quoted 107 lines

Quoted 9 lines
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
And as a side question, we clear slab->obj_exts when freeing obj_ext array, but don't clear OBJEXTS_ALLOC_FAIL when freeing a slab? That's not good.
Hm great point. We should rather make sure it's cleared always, instead of adjusting the debugging check, which shouldn't be then necessary, right?
Yeah folio_memcg_kmem() isn't supposed to be called on slabs anyway (it's not a slab at the time we free it to buddy), so we don't have to adjust the debug check.
Great. Hao Ge, can you please send v4 that instead of adjusting the VM_BUG_ON modifies free_slab_obj_exts() to always clear slab->obj_exts? Thanks!
Okay, I will send v4 as soon as possible.
Thanks
Best Regards
Hao
Quoted 86 lines
Quoted 86 lines
Quoted 49 lines
Quoted 16 lines
Quoted 8 lines
Quoted 6 lines
So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
Reviewed-by Suren Baghdasaryan <[email protected]>
Quoted 8 lines
nit: I think it would be helpful if the changelog explained why we need the additional check. We can have the same bit set in two different situations:
1. object extension vector allocation failure;
2. memcg_data pointing to a valid mem_cgroup.
To distinguish between them, we need to check not only the bit itself but also the rest of this field. If the rest is NULL, we have case 1, otherwise case 2.
Quoted 7 lines
Agreed.
In general LGTM,
Reviewed-by Harry Yoo <[email protected]>
By the way, maybe it'd be nice to introduce a new helper function that properly checks MEMCG_DATA_OBJEXTS flag.
I thought so too at first...
Quoted 24 lines
Quoted 6 lines
~/slab (slab/for-next-fixes)> git grep -n MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:337: MEMCG_DATA_OBJEXTS = (1UL << 0),
include/linux/memcontrol.h:344:#define __OBJEXTS_ALLOC_FAIL MEMCG_DATA_OBJEXTS
include/linux/memcontrol.h:358: * MEMCG_DATA_OBJEXTS.
include/linux/memcontrol.h:400: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
include/linux/memcontrol.h:421: VM_BUG_ON_FOLIO(memcg_data & MEMCG_DATA_OBJEXTS, folio);
these two,
Quoted 1 line
include/linux/memcontrol.h:492: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 4 lines
include/linux/memcontrol.h:538: (folio->memcg_data & MEMCG_DATA_OBJEXTS),
include/linux/memcontrol.h:1491: * if MEMCG_DATA_OBJEXTS is set.
mm/kfence/core.c:624: MEMCG_DATA_OBJEXTS;
mm/page_owner.c:513: if (memcg_data & MEMCG_DATA_OBJEXTS)
this,
Quoted 5 lines
mm/slab.h:541: * MEMCG_DATA_OBJEXTS bit set or be equal to OBJEXTS_ALLOC_FAIL.
mm/slab.h:543: VM_BUG_ON_PAGE(obj_exts && !(obj_exts & MEMCG_DATA_OBJEXTS) &&
mm/slub.c:2137: new_exts |= MEMCG_DATA_OBJEXTS;
tools/mm/show_page_info.py:55: MEMCG_DATA_OBJEXTS = prog.constant("MEMCG_DATA_OBJEXTS").value_()
tools/mm/show_page_info.py:59: if memcg_data & MEMCG_DATA_OBJEXTS:
and this do not look good.
Quoted 7 lines
Quoted 2 lines
I mean technically they are fine since OBJEXTS_ALLOC_FAIL is set on slabs only, but that's just a coincidence.
And checked the the other debugging checks too. But then thought it's better that if these are not expected to see slabs, then they should not be adjusted. I don't see it as a coincidence but as intention to keep it slab specific. It will be also more future proof for the upcoming separation of struct slab from struct page.
Then we're intentionally using (folio->memcg_data & MEMCG_DATA_OBJEXTS) check as a way to determine whether the folio is a slab (either slabobj_ext array allocation succeeded or failed).
That makes sense to me!
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
Quoted 24 lines
---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>
---
  include/linux/memcontrol.h | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
include/linux/memcontrol.h +3 -2
index 873e510d6f8d..7ed15f858dc4 100644
@@ -534,7 +534,9 @@ static inline struct mem_cgroup *get_mem_cgroup_from_objcg(struct obj_cgroup *ob
  static inline bool folio_memcg_kmem(struct folio *folio)
  {
         VM_BUG_ON_PGFLAGS(PageTail(&folio->page), &folio->page);
-       VM_BUG_ON_FOLIO(folio->memcg_data & MEMCG_DATA_OBJEXTS, folio);
+       VM_BUG_ON_FOLIO((folio->memcg_data != OBJEXTS_ALLOC_FAIL) &&
+                       (folio->memcg_data & MEMCG_DATA_OBJEXTS),
+                       folio);
         return folio->memcg_data & MEMCG_DATA_KMEM;
  }
 
--
2.25.1

Signature · 2 lines

Cheers,
Harry / Hyeonggon

Roman Gushchin

maintainer Acked-by on v3 1/1

2025-10-14 21:13 UTC #11 lore raw

In reply to Hao Ge

Quoted 12 lines · Hao Ge

From: Hao Ge <[email protected]>
Since OBJEXTS_ALLOC_FAIL and MEMCG_DATA_OBJEXTS currently share the same bit position, we cannot determine whether memcg_data still points to the slabobj_ext vector simply by checking folio->memcg_data & MEMCG_DATA_OBJEXTS.
If obj_exts allocation failed, slab->obj_exts is set to OBJEXTS_ALLOC_FAIL, and during the release of the associated folio, the BUG check is triggered because it was mistakenly assumed that a valid folio->memcg_data was not cleared before freeing the folio.

Quoted 8 lines

So let's check for memcg_data != OBJEXTS_ALLOC_FAIL in folio_memcg_kmem.
Fixes 7612833192d5 ("slab: Reuse first bit for OBJEXTS_ALLOC_FAIL")
Suggested-by Harry Yoo <[email protected]>
Signed-off-by Hao Ge <[email protected]>
---
v3: Simplify the solution, per Harry's suggestion in the v1 comments
Add Suggested-by: Harry Yoo <[email protected]>

Acked-by Roman Gushchin <[email protected]>

Thanks!