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@xxxxxxxxx> wrote:
From: Hao Ge <gehao@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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
