* [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] 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
* 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
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