* [PATCH] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook
@ 2024-08-15 7:21 Hao Ge
2024-08-15 21:00 ` Suren Baghdasaryan
0 siblings, 1 reply; 6+ messages in thread
From: Hao Ge @ 2024-08-15 7:21 UTC (permalink / raw)
To: cl, penberg, rientjes, iamjoonsoo.kim, akpm, vbabka,
roman.gushchin, 42.hyeyoo, surenb, kees, kent.overstreet
Cc: linux-mm, linux-kernel, Hao Ge
From: Hao Ge <gehao@kylinos.cn>
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 <gehao@kylinos.cn>
---
mm/slub.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mm/slub.c b/mm/slub.c
index c9d8a2497fd6..1f67621ba6dc 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -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;
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook 2024-08-15 7:21 [PATCH] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook Hao Ge @ 2024-08-15 21:00 ` Suren Baghdasaryan 2024-08-16 1:33 ` [PATCH v2] " Hao Ge 2024-08-16 1:45 ` [PATCH] " Hao Ge 0 siblings, 2 replies; 6+ messages in thread From: Suren Baghdasaryan @ 2024-08-15 21:00 UTC (permalink / raw) To: Hao Ge Cc: cl, penberg, rientjes, iamjoonsoo.kim, akpm, vbabka, roman.gushchin, 42.hyeyoo, kees, kent.overstreet, linux-mm, linux-kernel, Hao Ge On Thu, Aug 15, 2024 at 12:21 AM Hao Ge <hao.ge@linux.dev> wrote: > > From: Hao Ge <gehao@kylinos.cn> > > 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. 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. > > 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 <gehao@kylinos.cn> > --- > mm/slub.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/slub.c b/mm/slub.c > index c9d8a2497fd6..1f67621ba6dc 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -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. > obj_exts = slab_obj_exts(slab); > if (!obj_exts) > return; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook 2024-08-15 21:00 ` Suren Baghdasaryan @ 2024-08-16 1:33 ` Hao Ge 2024-08-27 6:52 ` Vlastimil Babka 2024-08-16 1:45 ` [PATCH] " Hao Ge 1 sibling, 1 reply; 6+ messages in thread From: Hao Ge @ 2024-08-16 1:33 UTC (permalink / raw) To: surenb Cc: 42.hyeyoo, akpm, cl, gehao, hao.ge, iamjoonsoo.kim, kees, kent.overstreet, linux-kernel, linux-mm, penberg, rientjes, roman.gushchin, vbabka From: Hao Ge <gehao@kylinos.cn> 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 ]--- Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths") Signed-off-by: Hao Ge <gehao@kylinos.cn> --- mm/slub.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mm/slub.c b/mm/slub.c index c9d8a2497fd6..a77f354f8325 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -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; -- 2.25.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook 2024-08-16 1:33 ` [PATCH v2] " Hao Ge @ 2024-08-27 6:52 ` Vlastimil Babka 0 siblings, 0 replies; 6+ messages in thread From: Vlastimil Babka @ 2024-08-27 6:52 UTC (permalink / raw) To: Hao Ge, surenb Cc: 42.hyeyoo, akpm, cl, gehao, iamjoonsoo.kim, kees, kent.overstreet, linux-kernel, linux-mm, penberg, rientjes, roman.gushchin On 8/16/24 03:33, Hao Ge wrote: > From: Hao Ge <gehao@kylinos.cn> > > 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 ]--- > > Fixes: 4b8736964640 ("mm/slab: add allocation accounting into slab allocation and free paths") > Signed-off-by: Hao Ge <gehao@kylinos.cn> Acked-by: Vlastimil Babka <vbabka@suse.cz> > --- > mm/slub.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/slub.c b/mm/slub.c > index c9d8a2497fd6..a77f354f8325 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook 2024-08-15 21:00 ` Suren Baghdasaryan 2024-08-16 1:33 ` [PATCH v2] " Hao Ge @ 2024-08-16 1:45 ` Hao Ge 2024-08-16 19:20 ` Suren Baghdasaryan 1 sibling, 1 reply; 6+ messages in thread From: Hao Ge @ 2024-08-16 1:45 UTC (permalink / raw) To: Suren Baghdasaryan Cc: cl, penberg, rientjes, iamjoonsoo.kim, akpm, vbabka, roman.gushchin, 42.hyeyoo, kees, kent.overstreet, linux-mm, linux-kernel, Hao Ge Hi Suren Thank you for taking the time to review this patch. On 8/16/24 05:00, Suren Baghdasaryan wrote: > On Thu, Aug 15, 2024 at 12:21 AM Hao Ge <hao.ge@linux.dev> wrote: >> From: Hao Ge <gehao@kylinos.cn> >> >> 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. > 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. 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." >> 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 <gehao@kylinos.cn> >> --- >> mm/slub.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/mm/slub.c b/mm/slub.c >> index c9d8a2497fd6..1f67621ba6dc 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -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. > OK,I have already made the modification in version 2. >> obj_exts = slab_obj_exts(slab); >> if (!obj_exts) >> return; >> -- >> 2.25.1 >> Thanks best regards Hao ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook 2024-08-16 1:45 ` [PATCH] " Hao Ge @ 2024-08-16 19:20 ` Suren Baghdasaryan 0 siblings, 0 replies; 6+ messages in thread From: Suren Baghdasaryan @ 2024-08-16 19:20 UTC (permalink / raw) To: Hao Ge Cc: cl, penberg, rientjes, iamjoonsoo.kim, akpm, vbabka, roman.gushchin, 42.hyeyoo, kees, kent.overstreet, linux-mm, linux-kernel, Hao Ge On Thu, Aug 15, 2024 at 6:46 PM Hao Ge <hao.ge@linux.dev> wrote: > > Hi Suren > > > Thank you for taking the time to review this patch. > > > On 8/16/24 05:00, Suren Baghdasaryan wrote: > > On Thu, Aug 15, 2024 at 12:21 AM Hao Ge <hao.ge@linux.dev> wrote: > >> From: Hao Ge <gehao@kylinos.cn> > >> > >> 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. > > 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. > > 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." > > >> 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 <gehao@kylinos.cn> > >> --- > >> mm/slub.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/mm/slub.c b/mm/slub.c > >> index c9d8a2497fd6..1f67621ba6dc 100644 > >> --- a/mm/slub.c > >> +++ b/mm/slub.c > >> @@ -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. > > > OK,I have already made the modification in version 2. > >> obj_exts = slab_obj_exts(slab); > >> if (!obj_exts) > >> return; > >> -- > >> 2.25.1 > >> > Thanks For all fixes to allocation tagging please: Cc: stable@vger.kernel.org # v6.10 Thanks, Suren. > > best regards > > Hao > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-08-27 6:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-08-15 7:21 [PATCH] mm/slub: Add check for s->flags in the alloc_tagging_slab_free_hook Hao Ge 2024-08-15 21:00 ` Suren Baghdasaryan 2024-08-16 1:33 ` [PATCH v2] " Hao Ge 2024-08-27 6:52 ` Vlastimil Babka 2024-08-16 1:45 ` [PATCH] " Hao Ge 2024-08-16 19:20 ` Suren Baghdasaryan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox