mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook

PATCH Superseded Hao Ge 3 participants 6 messages Aug 15, 2024 – Aug 27, 2024

Subsystems (2)

Memory management Slab allocator

lore

Share & tools

lore

Hao Ge

author

2024-08-15 07:21 UTC #0 lore raw

PATCH 1/1 mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook

From: Hao Ge <[email protected]>

When enable CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK, the following warning always occurs,This is because the following call stack occurred:
mem_pool_alloc
kmem_cache_alloc_noprof
slab_alloc_node
kfence_alloc

Once the kfence allocation is successful,slab->obj_exts will not be empty. Since in the prepare_slab_obj_exts_hook function,we perform a check for s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE),the alloc_tag_add function will not be called as a result.Therefore,ref->ct remains NULL.

However,when we call mem_pool_free,since obj_ext is not empty, it eventually leads to the alloc_tag_sub scenario being invoked. This is where the warning occurs.

So we should add corresponding checks in the alloc_tagging_slab_free_hook. For __GFP_NO_OBJ_EXT case,I didn't see the specific case where it's using kfence,so I won't add the corresponding check in alloc_tagging_slab_free_hook for now.

[    3.734349] ------------[ cut here ]------------
[    3.734807] alloc_tag was not set
[    3.735129] WARNING: CPU: 4 PID: 40 at ./include/linux/alloc_tag.h:130 kmem_cache_free+0x444/0x574
[    3.735866] Modules linked in: autofs4
[    3.736211] CPU: 4 UID: 0 PID: 40 Comm: ksoftirqd/4 Tainted: G        W          6.11.0-rc3-dirty #1
[    3.736969] Tainted: [W]=WARN
[    3.737258] Hardware name: QEMU KVM Virtual Machine, BIOS unknown 2/2/2022
[    3.737875] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.738501] pc : kmem_cache_free+0x444/0x574
[    3.738951] lr : kmem_cache_free+0x444/0x574
[    3.739361] sp : ffff80008357bb60
[    3.739693] x29: ffff80008357bb70 x28: 0000000000000000 x27: 0000000000000000
[    3.740338] x26: ffff80008207f000 x25: ffff000b2eb2fd60 x24: ffff0000c0005700
[    3.740982] x23: ffff8000804229e4 x22: ffff800082080000 x21: ffff800081756000
[    3.741630] x20: fffffd7ff8253360 x19: 00000000000000a8 x18: ffffffffffffffff
[    3.742274] x17: ffff800ab327f000 x16: ffff800083398000 x15: ffff800081756df0
[    3.742919] x14: 0000000000000000 x13: 205d344320202020 x12: 5b5d373038343337
[    3.743560] x11: ffff80008357b650 x10: 000000000000005d x9 : 00000000ffffffd0
[    3.744231] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008237bad0 x6 : c0000000ffff7fff
[    3.744907] x5 : ffff80008237ba78 x4 : ffff8000820bbad0 x3 : 0000000000000001
[    3.745580] x2 : 68d66547c09f7800 x1 : 68d66547c09f7800 x0 : 0000000000000000
[    3.746255] Call trace:
[    3.746530]  kmem_cache_free+0x444/0x574
[    3.746931]  mem_pool_free+0x44/0xf4
[    3.747306]  free_object_rcu+0xc8/0xdc
[    3.747693]  rcu_do_batch+0x234/0x8a4
[    3.748075]  rcu_core+0x230/0x3e4
[    3.748424]  rcu_core_si+0x14/0x1c
[    3.748780]  handle_softirqs+0x134/0x378
[    3.749189]  run_ksoftirqd+0x70/0x9c
[    3.749560]  smpboot_thread_fn+0x148/0x22c
[    3.749978]  kthread+0x10c/0x118
[    3.750323]  ret_from_fork+0x10/0x20
[    3.750696] ---[ end trace 0000000000000000 ]---

Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")

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

---
 mm/slub.c | 3 +++
 1 file changed, 3 insertions(+)

mm/slub.c +3 -0

index c9d8a2497fd6..1f67621ba6dc 100644
@@ -2116,6 +2116,9 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
 	if (!mem_alloc_profiling_enabled())
 		return;
 
+	if (s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE))
+		return;
+
 	obj_exts = slab_obj_exts(slab);
 	if (!obj_exts)
 		return;

Signature · 1 line

2.25.1

Suren Baghdasaryan

2024-08-15 21:00 UTC #1 lore raw

Quoted 11 lines · Hao Ge

From: Hao Ge <[email protected]>
When enable CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK, the following warning always occurs,This is because the following call stack occurred:
mem_pool_alloc
kmem_cache_alloc_noprof
slab_alloc_node
kfence_alloc

Quoted 8 lines

Once the kfence allocation is successful,slab->obj_exts will not be empty. Since in the prepare_slab_obj_exts_hook function,we perform a check for s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE),the alloc_tag_add function will not be called as a result.Therefore,ref->ct remains NULL.
However,when we call mem_pool_free,since obj_ext is not empty, it eventually leads to the alloc_tag_sub scenario being invoked. This is where the warning occurs.

Ok, I was trying to understand why "Once the kfence allocation is successful,slab->obj_exts will not be empty.". alloc_slab_obj_exts() has to be called to create slab->obj_exts. Normally that is done in prepare_slab_obj_exts_hook() which has the s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE) check and the cache that kfence_alloc() uses here is the object_cache (passed from mem_pool_alloc()) which has SLAB_NOLEAKTRACE:
https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/kmemleak.c#L453.
Therefore, prepare_slab_obj_exts_hook() should have skipped it and did not create the slab->obj_exts. So, it must have been called from account_slab() or __memcg_slab_post_alloc_hook() to create slab->obj_exts for memcg accounting in slab->obj_exts.objcg. Ok, now I understand why you need the CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK combination.
Please confirm that slab->obj_exts creation is happening the way I described above and for those reasons and if so, please update the description of this patch to explain that.

Quoted 52 lines

[    3.734349] ------------[ cut here ]------------
[    3.734807] alloc_tag was not set
[    3.735129] WARNING: CPU: 4 PID: 40 at ./include/linux/alloc_tag.h:130 kmem_cache_free+0x444/0x574
[    3.735866] Modules linked in: autofs4
[    3.736211] CPU: 4 UID: 0 PID: 40 Comm: ksoftirqd/4 Tainted: G        W          6.11.0-rc3-dirty #1
[    3.736969] Tainted: [W]=WARN
[    3.737258] Hardware name: QEMU KVM Virtual Machine, BIOS unknown 2/2/2022
[    3.737875] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.738501] pc : kmem_cache_free+0x444/0x574
[    3.738951] lr : kmem_cache_free+0x444/0x574
[    3.739361] sp : ffff80008357bb60
[    3.739693] x29: ffff80008357bb70 x28: 0000000000000000 x27: 0000000000000000
[    3.740338] x26: ffff80008207f000 x25: ffff000b2eb2fd60 x24: ffff0000c0005700
[    3.740982] x23: ffff8000804229e4 x22: ffff800082080000 x21: ffff800081756000
[    3.741630] x20: fffffd7ff8253360 x19: 00000000000000a8 x18: ffffffffffffffff
[    3.742274] x17: ffff800ab327f000 x16: ffff800083398000 x15: ffff800081756df0
[    3.742919] x14: 0000000000000000 x13: 205d344320202020 x12: 5b5d373038343337
[    3.743560] x11: ffff80008357b650 x10: 000000000000005d x9 : 00000000ffffffd0
[    3.744231] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008237bad0 x6 : c0000000ffff7fff
[    3.744907] x5 : ffff80008237ba78 x4 : ffff8000820bbad0 x3 : 0000000000000001
[    3.745580] x2 : 68d66547c09f7800 x1 : 68d66547c09f7800 x0 : 0000000000000000
[    3.746255] Call trace:
[    3.746530]  kmem_cache_free+0x444/0x574
[    3.746931]  mem_pool_free+0x44/0xf4
[    3.747306]  free_object_rcu+0xc8/0xdc
[    3.747693]  rcu_do_batch+0x234/0x8a4
[    3.748075]  rcu_core+0x230/0x3e4
[    3.748424]  rcu_core_si+0x14/0x1c
[    3.748780]  handle_softirqs+0x134/0x378
[    3.749189]  run_ksoftirqd+0x70/0x9c
[    3.749560]  smpboot_thread_fn+0x148/0x22c
[    3.749978]  kthread+0x10c/0x118
[    3.750323]  ret_from_fork+0x10/0x20
[    3.750696] ---[ end trace 0000000000000000 ]---

Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")

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

---
 mm/slub.c | 3 +++
 1 file changed, 3 insertions(+)

mm/slub.c +0 -0

index c9d8a2497fd6..1f67621ba6dc 100644

mm/slub.c alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p, · line 2121

Quoted 6 lines

mm/slub.c +3 -0

@@ -2116,6 +2116,9 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
        if (!mem_alloc_profiling_enabled())
                return;
 
+       if (s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE))
+               return;
+

Please add a comment before this check saying something like:
/* slab->obj_exts might not be NULL if it was created for MEMCG accounting. */

Other than that the fix seems fine to me.
Thanks,
Suren.

Quoted 6 lines

mm/slub.c +0 -1

        obj_exts = slab_obj_exts(slab);
        if (!obj_exts)
                return;
--

2.25.1

Hao Ge

author

2024-08-16 01:34 UTC #2 lore raw

PATCH v2 1/1 mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook

From: Hao Ge <[email protected]>

Once the kfence allocation is successful,slab->obj_exts will not be empty, because it has already been assigned a value in kfence_init_pool.

Since in the prepare_slab_obj_exts_hook function,we perform a check for s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE),the alloc_tag_add function will not be called as a result.Therefore,ref->ct remains NULL.

However,when we call mem_pool_free,since obj_ext is not empty, it eventually leads to the alloc_tag_sub scenario being invoked. This is where the warning occurs.

[    3.734349] ------------[ cut here ]------------
[    3.734807] alloc_tag was not set
[    3.735129] WARNING: CPU: 4 PID: 40 at ./include/linux/alloc_tag.h:130 kmem_cache_free+0x444/0x574
[    3.735866] Modules linked in: autofs4
[    3.736211] CPU: 4 UID: 0 PID: 40 Comm: ksoftirqd/4 Tainted: G        W          6.11.0-rc3-dirty #1
[    3.736969] Tainted: [W]=WARN
[    3.737258] Hardware name: QEMU KVM Virtual Machine, BIOS unknown 2/2/2022
[    3.737875] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.738501] pc : kmem_cache_free+0x444/0x574
[    3.738951] lr : kmem_cache_free+0x444/0x574
[    3.739361] sp : ffff80008357bb60
[    3.739693] x29: ffff80008357bb70 x28: 0000000000000000 x27: 0000000000000000
[    3.740338] x26: ffff80008207f000 x25: ffff000b2eb2fd60 x24: ffff0000c0005700
[    3.740982] x23: ffff8000804229e4 x22: ffff800082080000 x21: ffff800081756000
[    3.741630] x20: fffffd7ff8253360 x19: 00000000000000a8 x18: ffffffffffffffff
[    3.742274] x17: ffff800ab327f000 x16: ffff800083398000 x15: ffff800081756df0
[    3.742919] x14: 0000000000000000 x13: 205d344320202020 x12: 5b5d373038343337
[    3.743560] x11: ffff80008357b650 x10: 000000000000005d x9 : 00000000ffffffd0
[    3.744231] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008237bad0 x6 : c0000000ffff7fff
[    3.744907] x5 : ffff80008237ba78 x4 : ffff8000820bbad0 x3 : 0000000000000001
[    3.745580] x2 : 68d66547c09f7800 x1 : 68d66547c09f7800 x0 : 0000000000000000
[    3.746255] Call trace:
[    3.746530]  kmem_cache_free+0x444/0x574
[    3.746931]  mem_pool_free+0x44/0xf4
[    3.747306]  free_object_rcu+0xc8/0xdc
[    3.747693]  rcu_do_batch+0x234/0x8a4
[    3.748075]  rcu_core+0x230/0x3e4
[    3.748424]  rcu_core_si+0x14/0x1c
[    3.748780]  handle_softirqs+0x134/0x378
[    3.749189]  run_ksoftirqd+0x70/0x9c
[    3.749560]  smpboot_thread_fn+0x148/0x22c
[    3.749978]  kthread+0x10c/0x118
[    3.750323]  ret_from_fork+0x10/0x20
[    3.750696] ---[ end trace 0000000000000000 ]---

Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")

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

---
 mm/slub.c | 4 ++++
 1 file changed, 4 insertions(+)

mm/slub.c +4 -0

index c9d8a2497fd6..a77f354f8325 100644
@@ -2116,6 +2116,10 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
 	if (!mem_alloc_profiling_enabled())
 		return;
 
+	/* slab->obj_exts might not be NULL if it was created for MEMCG accounting. */
+	if (s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE))
+		return;
+
 	obj_exts = slab_obj_exts(slab);
 	if (!obj_exts)
 		return;

Signature · 1 line

2.25.1

Vlastimil Babka

Acked-by on v2 1/1

2024-08-27 06:52 UTC #3 lore raw

Quoted 61 lines · Hao Ge

From: Hao Ge <[email protected]>
When enable CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK, the following warning always occurs,This is because the following call stack occurred:
mem_pool_alloc
kmem_cache_alloc_noprof
slab_alloc_node
kfence_alloc
Once the kfence allocation is successful,slab->obj_exts will not be empty, because it has already been assigned a value in kfence_init_pool.
Since in the prepare_slab_obj_exts_hook function,we perform a check for s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE),the alloc_tag_add function will not be called as a result.Therefore,ref->ct remains NULL.
However,when we call mem_pool_free,since obj_ext is not empty, it eventually leads to the alloc_tag_sub scenario being invoked. This is where the warning occurs.
So we should add corresponding checks in the alloc_tagging_slab_free_hook. For __GFP_NO_OBJ_EXT case,I didn't see the specific case where it's using kfence,so I won't add the corresponding check in alloc_tagging_slab_free_hook for now.
[    3.734349] ------------[ cut here ]------------
[    3.734807] alloc_tag was not set
[    3.735129] WARNING: CPU: 4 PID: 40 at ./include/linux/alloc_tag.h:130 kmem_cache_free+0x444/0x574
[    3.735866] Modules linked in: autofs4
[    3.736211] CPU: 4 UID: 0 PID: 40 Comm: ksoftirqd/4 Tainted: G        W          6.11.0-rc3-dirty #1
[    3.736969] Tainted: [W]=WARN
[    3.737258] Hardware name: QEMU KVM Virtual Machine, BIOS unknown 2/2/2022
[    3.737875] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.738501] pc : kmem_cache_free+0x444/0x574
[    3.738951] lr : kmem_cache_free+0x444/0x574
[    3.739361] sp : ffff80008357bb60
[    3.739693] x29: ffff80008357bb70 x28: 0000000000000000 x27: 0000000000000000
[    3.740338] x26: ffff80008207f000 x25: ffff000b2eb2fd60 x24: ffff0000c0005700
[    3.740982] x23: ffff8000804229e4 x22: ffff800082080000 x21: ffff800081756000
[    3.741630] x20: fffffd7ff8253360 x19: 00000000000000a8 x18: ffffffffffffffff
[    3.742274] x17: ffff800ab327f000 x16: ffff800083398000 x15: ffff800081756df0
[    3.742919] x14: 0000000000000000 x13: 205d344320202020 x12: 5b5d373038343337
[    3.743560] x11: ffff80008357b650 x10: 000000000000005d x9 : 00000000ffffffd0
[    3.744231] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008237bad0 x6 : c0000000ffff7fff
[    3.744907] x5 : ffff80008237ba78 x4 : ffff8000820bbad0 x3 : 0000000000000001
[    3.745580] x2 : 68d66547c09f7800 x1 : 68d66547c09f7800 x0 : 0000000000000000
[    3.746255] Call trace:
[    3.746530]  kmem_cache_free+0x444/0x574
[    3.746931]  mem_pool_free+0x44/0xf4
[    3.747306]  free_object_rcu+0xc8/0xdc
[    3.747693]  rcu_do_batch+0x234/0x8a4
[    3.748075]  rcu_core+0x230/0x3e4
[    3.748424]  rcu_core_si+0x14/0x1c
[    3.748780]  handle_softirqs+0x134/0x378
[    3.749189]  run_ksoftirqd+0x70/0x9c
[    3.749560]  smpboot_thread_fn+0x148/0x22c
[    3.749978]  kthread+0x10c/0x118
[    3.750323]  ret_from_fork+0x10/0x20
[    3.750696] ---[ end trace 0000000000000000 ]---

Quoted 2 lines

Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")
Signed-off-by Hao Ge <[email protected]>

Acked-by Vlastimil Babka <[email protected]>

Quoted 19 lines

---
 mm/slub.c | 4 ++++
 1 file changed, 4 insertions(+)

mm/slub.c +4 -0

index c9d8a2497fd6..a77f354f8325 100644
@@ -2116,6 +2116,10 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
 	if (!mem_alloc_profiling_enabled())
 		return;
 
+	/* slab->obj_exts might not be NULL if it was created for MEMCG accounting. */
+	if (s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE))
+		return;
+
 	obj_exts = slab_obj_exts(slab);
 	if (!obj_exts)
 		return;

Hao Ge

author

2024-08-16 01:46 UTC #4 lore raw

In reply to Suren Baghdasaryan

Hi Suren

Thank you for taking the time to review this patch.

Quoted 28 lines · Suren Baghdasaryan

Quoted 10 lines · Hao Ge
From: Hao Ge <[email protected]>
When enable CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK, the following warning always occurs,This is because the following call stack occurred:
mem_pool_alloc
kmem_cache_alloc_noprof
slab_alloc_node
kfence_alloc
Quoted 8 lines
Once the kfence allocation is successful,slab->obj_exts will not be empty. Since in the prepare_slab_obj_exts_hook function,we perform a check for s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE),the alloc_tag_add function will not be called as a result.Therefore,ref->ct remains NULL.
However,when we call mem_pool_free,since obj_ext is not empty, it eventually leads to the alloc_tag_sub scenario being invoked. This is where the warning occurs.
Ok, I was trying to understand why "Once the kfence allocation is successful,slab->obj_exts will not be empty.". alloc_slab_obj_exts() has to be called to create slab->obj_exts. Normally that is done in prepare_slab_obj_exts_hook() which has the s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE) check and the cache that kfence_alloc() uses here is the object_cache (passed from mem_pool_alloc()) which has SLAB_NOLEAKTRACE:
https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/kmemleak.c#L453.
Therefore, prepare_slab_obj_exts_hook() should have skipped it and did

Quoted 8 lines

not create the slab->obj_exts. So, it must have been called from account_slab() or __memcg_slab_post_alloc_hook() to create slab->obj_exts for memcg accounting in slab->obj_exts.objcg. Ok, now I understand why you need the CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK combination.
Please confirm that slab->obj_exts creation is happening the way I described above and for those reasons and if so, please update the description of this patch to explain that.

It's mostly correct, but in the context of kfence, the assignment actually takes place within the kfence_init_pool function.

https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/kfence/core.c#L606

So in version 2, I have included this explain "because it has already been assigned a value in kfence_init_pool."

Quoted 60 lines

Quoted 51 lines

[    3.734349] ------------[ cut here ]------------
[    3.734807] alloc_tag was not set
[    3.735129] WARNING: CPU: 4 PID: 40 at ./include/linux/alloc_tag.h:130 kmem_cache_free+0x444/0x574
[    3.735866] Modules linked in: autofs4
[    3.736211] CPU: 4 UID: 0 PID: 40 Comm: ksoftirqd/4 Tainted: G        W          6.11.0-rc3-dirty #1
[    3.736969] Tainted: [W]=WARN
[    3.737258] Hardware name: QEMU KVM Virtual Machine, BIOS unknown 2/2/2022
[    3.737875] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.738501] pc : kmem_cache_free+0x444/0x574
[    3.738951] lr : kmem_cache_free+0x444/0x574
[    3.739361] sp : ffff80008357bb60
[    3.739693] x29: ffff80008357bb70 x28: 0000000000000000 x27: 0000000000000000
[    3.740338] x26: ffff80008207f000 x25: ffff000b2eb2fd60 x24: ffff0000c0005700
[    3.740982] x23: ffff8000804229e4 x22: ffff800082080000 x21: ffff800081756000
[    3.741630] x20: fffffd7ff8253360 x19: 00000000000000a8 x18: ffffffffffffffff
[    3.742274] x17: ffff800ab327f000 x16: ffff800083398000 x15: ffff800081756df0
[    3.742919] x14: 0000000000000000 x13: 205d344320202020 x12: 5b5d373038343337
[    3.743560] x11: ffff80008357b650 x10: 000000000000005d x9 : 00000000ffffffd0
[    3.744231] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008237bad0 x6 : c0000000ffff7fff
[    3.744907] x5 : ffff80008237ba78 x4 : ffff8000820bbad0 x3 : 0000000000000001
[    3.745580] x2 : 68d66547c09f7800 x1 : 68d66547c09f7800 x0 : 0000000000000000
[    3.746255] Call trace:
[    3.746530]  kmem_cache_free+0x444/0x574
[    3.746931]  mem_pool_free+0x44/0xf4
[    3.747306]  free_object_rcu+0xc8/0xdc
[    3.747693]  rcu_do_batch+0x234/0x8a4
[    3.748075]  rcu_core+0x230/0x3e4
[    3.748424]  rcu_core_si+0x14/0x1c
[    3.748780]  handle_softirqs+0x134/0x378
[    3.749189]  run_ksoftirqd+0x70/0x9c
[    3.749560]  smpboot_thread_fn+0x148/0x22c
[    3.749978]  kthread+0x10c/0x118
[    3.750323]  ret_from_fork+0x10/0x20
[    3.750696] ---[ end trace 0000000000000000 ]---

Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")

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

---
  mm/slub.c | 3 +++
  1 file changed, 3 insertions(+)

mm/slub.c +0 -0

index c9d8a2497fd6..1f67621ba6dc 100644

mm/slub.c alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p, · line 2121

Quoted 6 lines

mm/slub.c +3 -0

@@ -2116,6 +2116,9 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
         if (!mem_alloc_profiling_enabled())
                 return;
 
+       if (s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE))
+               return;
+

Please add a comment before this check saying something like:
/* slab->obj_exts might not be NULL if it was created for MEMCG accounting. */

Quoted 4 lines

Other than that the fix seems fine to me.
Thanks,
Suren.

OK,I have already made the modification in version 2.

Quoted 6 lines

mm/slub.c +0 -1

         obj_exts = slab_obj_exts(slab);
         if (!obj_exts)
                 return;
--

2.25.1

Thanks

best regards

Hao

Suren Baghdasaryan

2024-08-16 19:21 UTC #5 lore raw

Quoted 117 lines · Hao Ge

Hi Suren
Thank you for taking the time to review this patch.
Quoted 28 lines · Suren Baghdasaryan
Quoted 10 lines · Hao Ge
From: Hao Ge <[email protected]>
When enable CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK, the following warning always occurs,This is because the following call stack occurred:
mem_pool_alloc
kmem_cache_alloc_noprof
slab_alloc_node
kfence_alloc
Quoted 8 lines
Once the kfence allocation is successful,slab->obj_exts will not be empty. Since in the prepare_slab_obj_exts_hook function,we perform a check for s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE),the alloc_tag_add function will not be called as a result.Therefore,ref->ct remains NULL.
However,when we call mem_pool_free,since obj_ext is not empty, it eventually leads to the alloc_tag_sub scenario being invoked. This is where the warning occurs.
Ok, I was trying to understand why "Once the kfence allocation is successful,slab->obj_exts will not be empty.". alloc_slab_obj_exts() has to be called to create slab->obj_exts. Normally that is done in prepare_slab_obj_exts_hook() which has the s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE) check and the cache that kfence_alloc() uses here is the object_cache (passed from mem_pool_alloc()) which has SLAB_NOLEAKTRACE:
https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/kmemleak.c#L453.
Therefore, prepare_slab_obj_exts_hook() should have skipped it and did
Quoted 8 lines
not create the slab->obj_exts. So, it must have been called from account_slab() or __memcg_slab_post_alloc_hook() to create slab->obj_exts for memcg accounting in slab->obj_exts.objcg. Ok, now I understand why you need the CONFIG_MEMCG & CONFIG_KFENCE & CONFIG_KMEMLEAK combination.
Please confirm that slab->obj_exts creation is happening the way I described above and for those reasons and if so, please update the description of this patch to explain that.
It's mostly correct, but in the context of kfence, the assignment actually takes place within the kfence_init_pool function.
https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/kfence/core.c#L606
So in version 2, I have included this explain "because it has already been assigned a value in kfence_init_pool."
Quoted 60 lines
Quoted 51 lines
So we should add corresponding checks in the alloc_tagging_slab_free_hook. For __GFP_NO_OBJ_EXT case,I didn't see the specific case where it's using kfence,so I won't add the corresponding check in alloc_tagging_slab_free_hook for now.
[    3.734349] ------------[ cut here ]------------
[    3.734807] alloc_tag was not set
[    3.735129] WARNING: CPU: 4 PID: 40 at ./include/linux/alloc_tag.h:130 kmem_cache_free+0x444/0x574
[    3.735866] Modules linked in: autofs4
[    3.736211] CPU: 4 UID: 0 PID: 40 Comm: ksoftirqd/4 Tainted: G        W          6.11.0-rc3-dirty #1
[    3.736969] Tainted: [W]=WARN
[    3.737258] Hardware name: QEMU KVM Virtual Machine, BIOS unknown 2/2/2022
[    3.737875] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    3.738501] pc : kmem_cache_free+0x444/0x574
[    3.738951] lr : kmem_cache_free+0x444/0x574
[    3.739361] sp : ffff80008357bb60
[    3.739693] x29: ffff80008357bb70 x28: 0000000000000000 x27: 0000000000000000
[    3.740338] x26: ffff80008207f000 x25: ffff000b2eb2fd60 x24: ffff0000c0005700
[    3.740982] x23: ffff8000804229e4 x22: ffff800082080000 x21: ffff800081756000
[    3.741630] x20: fffffd7ff8253360 x19: 00000000000000a8 x18: ffffffffffffffff
[    3.742274] x17: ffff800ab327f000 x16: ffff800083398000 x15: ffff800081756df0
[    3.742919] x14: 0000000000000000 x13: 205d344320202020 x12: 5b5d373038343337
[    3.743560] x11: ffff80008357b650 x10: 000000000000005d x9 : 00000000ffffffd0
[    3.744231] x8 : 7f7f7f7f7f7f7f7f x7 : ffff80008237bad0 x6 : c0000000ffff7fff
[    3.744907] x5 : ffff80008237ba78 x4 : ffff8000820bbad0 x3 : 0000000000000001
[    3.745580] x2 : 68d66547c09f7800 x1 : 68d66547c09f7800 x0 : 0000000000000000
[    3.746255] Call trace:
[    3.746530]  kmem_cache_free+0x444/0x574
[    3.746931]  mem_pool_free+0x44/0xf4
[    3.747306]  free_object_rcu+0xc8/0xdc
[    3.747693]  rcu_do_batch+0x234/0x8a4
[    3.748075]  rcu_core+0x230/0x3e4
[    3.748424]  rcu_core_si+0x14/0x1c
[    3.748780]  handle_softirqs+0x134/0x378
[    3.749189]  run_ksoftirqd+0x70/0x9c
[    3.749560]  smpboot_thread_fn+0x148/0x22c
[    3.749978]  kthread+0x10c/0x118
[    3.750323]  ret_from_fork+0x10/0x20
[    3.750696] ---[ end trace 0000000000000000 ]---
Fixes 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths")
Signed-off-by Hao Ge <[email protected]>
---
  mm/slub.c | 3 +++
  1 file changed, 3 insertions(+)
mm/slub.c +0 -0
index c9d8a2497fd6..1f67621ba6dc 100644
mm/slub.c alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p, · line 2121
Quoted 6 lines
mm/slub.c +3 -0
@@ -2116,6 +2116,9 @@ alloc_tagging_slab_free_hook(struct kmem_cache *s, struct slab *slab, void **p,
         if (!mem_alloc_profiling_enabled())
                 return;
 
+       if (s->flags & (SLAB_NO_OBJ_EXT | SLAB_NOLEAKTRACE))
+               return;
+
Please add a comment before this check saying something like:
/* slab->obj_exts might not be NULL if it was created for MEMCG accounting. */
Quoted 4 lines
Other than that the fix seems fine to me.
Thanks,
Suren.

Quoted 8 lines

OK,I have already made the modification in version 2.
Quoted 6 lines
Quoted 6 lines
mm/slub.c +0 -1
         obj_exts = slab_obj_exts(slab);
         if (!obj_exts)
                 return;
--
2.25.1
Thanks

For all fixes to allocation tagging please:

Cc [email protected] # v6.10

Thanks,
Suren.

Quoted 5 lines

best regards
Hao