* [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions()
@ 2023-09-25 7:20 Jinjie Ruan
2023-09-25 15:14 ` Andrew Morton
2023-09-25 16:03 ` SeongJae Park
0 siblings, 2 replies; 5+ messages in thread
From: Jinjie Ruan @ 2023-09-25 7:20 UTC (permalink / raw)
To: linux-mm, damon, SeongJae Park, Andrew Morton; +Cc: ruanjinjie
When CONFIG_DAMON_VADDR_KUNIT_TEST=y and making CONFIG_DEBUG_KMEMLEAK=y
and CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y, the below memory leak is detected.
Since commit 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary
variables"), the damon_destroy_ctx() is removed, but still call
damon_new_target() and damon_new_region(), the damon_region which is
allocated by kmem_cache_alloc() in damon_new_region() and the damon_target
which is allocated by kmalloc in damon_new_target() are not freed. And the
damon_region which is allocated in damon_new_region() in
damon_set_regions() is also not freed.
So use damon_destroy_target to free all the damon_regions and damon_target.
unreferenced object 0xffff888107c9a940 (size 64):
comm "kunit_try_catch", pid 1069, jiffies 4294670592 (age 732.761s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
60 c7 9c 07 81 88 ff ff f8 cb 9c 07 81 88 ff ff `...............
backtrace:
[<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
[<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
[<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
[<ffffffff819c82be>] damon_test_apply_three_regions1+0x21e/0x260
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
unreferenced object 0xffff8881079cc740 (size 56):
comm "kunit_try_catch", pid 1069, jiffies 4294670592 (age 732.761s)
hex dump (first 32 bytes):
05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
backtrace:
[<ffffffff819bc492>] damon_new_region+0x22/0x1c0
[<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
[<ffffffff819c82be>] damon_test_apply_three_regions1+0x21e/0x260
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
unreferenced object 0xffff888107c9ac40 (size 64):
comm "kunit_try_catch", pid 1071, jiffies 4294670595 (age 732.843s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
a0 cc 9c 07 81 88 ff ff 78 a1 76 07 81 88 ff ff ........x.v.....
backtrace:
[<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
[<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
[<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
[<ffffffff819c851e>] damon_test_apply_three_regions2+0x21e/0x260
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
unreferenced object 0xffff8881079ccc80 (size 56):
comm "kunit_try_catch", pid 1071, jiffies 4294670595 (age 732.843s)
hex dump (first 32 bytes):
05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
backtrace:
[<ffffffff819bc492>] damon_new_region+0x22/0x1c0
[<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
[<ffffffff819c851e>] damon_test_apply_three_regions2+0x21e/0x260
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
unreferenced object 0xffff888107c9af40 (size 64):
comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.011s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
20 a2 76 07 81 88 ff ff b8 a6 76 07 81 88 ff ff .v.......v.....
backtrace:
[<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
[<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
[<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
[<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
unreferenced object 0xffff88810776a200 (size 56):
comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.011s)
hex dump (first 32 bytes):
05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
backtrace:
[<ffffffff819bc492>] damon_new_region+0x22/0x1c0
[<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
[<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
unreferenced object 0xffff88810776a740 (size 56):
comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.025s)
hex dump (first 32 bytes):
3d 00 00 00 00 00 00 00 3f 00 00 00 00 00 00 00 =.......?.......
6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
backtrace:
[<ffffffff819bc492>] damon_new_region+0x22/0x1c0
[<ffffffff819bfcc2>] damon_set_regions+0x4c2/0x8e0
[<ffffffff819c7dbb>] damon_do_test_apply_three_regions.constprop.0+0xfb/0x3e0
[<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
unreferenced object 0xffff888108038240 (size 64):
comm "kunit_try_catch", pid 1075, jiffies 4294670600 (age 733.022s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 03 00 00 00 6b 6b 6b 6b ............kkkk
48 ad 76 07 81 88 ff ff 98 ae 76 07 81 88 ff ff H.v.......v.....
backtrace:
[<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
[<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
[<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
[<ffffffff819c898d>] damon_test_apply_three_regions4+0x1cd/0x210
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
unreferenced object 0xffff88810776ad28 (size 56):
comm "kunit_try_catch", pid 1075, jiffies 4294670600 (age 733.022s)
hex dump (first 32 bytes):
05 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 ................
6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
backtrace:
[<ffffffff819bc492>] damon_new_region+0x22/0x1c0
[<ffffffff819bfcc2>] damon_set_regions+0x4c2/0x8e0
[<ffffffff819c7dbb>] damon_do_test_apply_three_regions.constprop.0+0xfb/0x3e0
[<ffffffff819c898d>] damon_test_apply_three_regions4+0x1cd/0x210
[<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
[<ffffffff81237cf6>] kthread+0x2b6/0x380
[<ffffffff81097add>] ret_from_fork+0x2d/0x70
[<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
Fixes: 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary variables")
Fixes: dae0087aeff4 ("mm/damon/vaddr: remove damon_va_apply_three_regions()")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
mm/damon/vaddr-test.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h
index c4b455b5ee30..dcf1ca6b31cc 100644
--- a/mm/damon/vaddr-test.h
+++ b/mm/damon/vaddr-test.h
@@ -148,6 +148,8 @@ static void damon_do_test_apply_three_regions(struct kunit *test,
KUNIT_EXPECT_EQ(test, r->ar.start, expected[i * 2]);
KUNIT_EXPECT_EQ(test, r->ar.end, expected[i * 2 + 1]);
}
+
+ damon_destroy_target(t);
}
/*
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions()
2023-09-25 7:20 [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions() Jinjie Ruan
@ 2023-09-25 15:14 ` Andrew Morton
2023-09-26 1:39 ` Ruan Jinjie
2023-09-25 16:03 ` SeongJae Park
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2023-09-25 15:14 UTC (permalink / raw)
To: Jinjie Ruan; +Cc: linux-mm, damon, SeongJae Park
On Mon, 25 Sep 2023 15:20:59 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> When CONFIG_DAMON_VADDR_KUNIT_TEST=y and making CONFIG_DEBUG_KMEMLEAK=y
> and CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y, the below memory leak is detected.
>
> Since commit 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary
> variables"), the damon_destroy_ctx() is removed, but still call
> damon_new_target() and damon_new_region(), the damon_region which is
> allocated by kmem_cache_alloc() in damon_new_region() and the damon_target
> which is allocated by kmalloc in damon_new_target() are not freed. And the
> damon_region which is allocated in damon_new_region() in
> damon_set_regions() is also not freed.
>
> So use damon_destroy_target to free all the damon_regions and damon_target.
>
> ...
>
> Fixes: 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary variables")
> Fixes: dae0087aeff4 ("mm/damon/vaddr: remove damon_va_apply_three_regions()")
Can we please identify a single Fixes: target? Otherwise, how are
-stable tree maintainers to determine which kernels need the fix?
I'll go with 9f86d624292c for now.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions()
2023-09-25 15:14 ` Andrew Morton
@ 2023-09-26 1:39 ` Ruan Jinjie
0 siblings, 0 replies; 5+ messages in thread
From: Ruan Jinjie @ 2023-09-26 1:39 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-mm, damon, SeongJae Park
On 2023/9/25 23:14, Andrew Morton wrote:
> On Mon, 25 Sep 2023 15:20:59 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
>> When CONFIG_DAMON_VADDR_KUNIT_TEST=y and making CONFIG_DEBUG_KMEMLEAK=y
>> and CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y, the below memory leak is detected.
>>
>> Since commit 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary
>> variables"), the damon_destroy_ctx() is removed, but still call
>> damon_new_target() and damon_new_region(), the damon_region which is
>> allocated by kmem_cache_alloc() in damon_new_region() and the damon_target
>> which is allocated by kmalloc in damon_new_target() are not freed. And the
>> damon_region which is allocated in damon_new_region() in
>> damon_set_regions() is also not freed.
>>
>> So use damon_destroy_target to free all the damon_regions and damon_target.
>>
>> ...
>>
>> Fixes: 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary variables")
>> Fixes: dae0087aeff4 ("mm/damon/vaddr: remove damon_va_apply_three_regions()")
>
> Can we please identify a single Fixes: target? Otherwise, how are
> -stable tree maintainers to determine which kernels need the fix?
Right！Thank you. Commit dae0087aeff4 just replaces
damon_va_apply_three_regions() with damon_set_regions()，not first
introduce damon_set_regions(). Sorry, there is a mistake.
>
> I'll go with 9f86d624292c for now.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions()
2023-09-25 7:20 [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions() Jinjie Ruan
2023-09-25 15:14 ` Andrew Morton
@ 2023-09-25 16:03 ` SeongJae Park
2023-09-26 1:37 ` Ruan Jinjie
1 sibling, 1 reply; 5+ messages in thread
From: SeongJae Park @ 2023-09-25 16:03 UTC (permalink / raw)
To: Jinjie Ruan; +Cc: linux-mm, damon, SeongJae Park, Andrew Morton
Hi Jinjie,
On Mon, 25 Sep 2023 15:20:59 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote:
> When CONFIG_DAMON_VADDR_KUNIT_TEST=y and making CONFIG_DEBUG_KMEMLEAK=y
> and CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y, the below memory leak is detected.
>
> Since commit 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary
> variables"), the damon_destroy_ctx() is removed, but still call
> damon_new_target() and damon_new_region(), the damon_region which is
> allocated by kmem_cache_alloc() in damon_new_region() and the damon_target
> which is allocated by kmalloc in damon_new_target() are not freed. And the
> damon_region which is allocated in damon_new_region() in
> damon_set_regions() is also not freed.
>
> So use damon_destroy_target to free all the damon_regions and damon_target.
Thank you for finding this bug and sending this patch!
>
> unreferenced object 0xffff888107c9a940 (size 64):
> comm "kunit_try_catch", pid 1069, jiffies 4294670592 (age 732.761s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
> 60 c7 9c 07 81 88 ff ff f8 cb 9c 07 81 88 ff ff `...............
> backtrace:
> [<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
> [<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
> [<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
> [<ffffffff819c82be>] damon_test_apply_three_regions1+0x21e/0x260
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
> unreferenced object 0xffff8881079cc740 (size 56):
> comm "kunit_try_catch", pid 1069, jiffies 4294670592 (age 732.761s)
> hex dump (first 32 bytes):
> 05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
> backtrace:
> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
> [<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
> [<ffffffff819c82be>] damon_test_apply_three_regions1+0x21e/0x260
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
> unreferenced object 0xffff888107c9ac40 (size 64):
> comm "kunit_try_catch", pid 1071, jiffies 4294670595 (age 732.843s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
> a0 cc 9c 07 81 88 ff ff 78 a1 76 07 81 88 ff ff ........x.v.....
> backtrace:
> [<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
> [<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
> [<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
> [<ffffffff819c851e>] damon_test_apply_three_regions2+0x21e/0x260
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
> unreferenced object 0xffff8881079ccc80 (size 56):
> comm "kunit_try_catch", pid 1071, jiffies 4294670595 (age 732.843s)
> hex dump (first 32 bytes):
> 05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
> backtrace:
> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
> [<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
> [<ffffffff819c851e>] damon_test_apply_three_regions2+0x21e/0x260
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
> unreferenced object 0xffff888107c9af40 (size 64):
> comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.011s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
> 20 a2 76 07 81 88 ff ff b8 a6 76 07 81 88 ff ff .v.......v.....
> backtrace:
> [<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
> [<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
> [<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
> [<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
> unreferenced object 0xffff88810776a200 (size 56):
> comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.011s)
> hex dump (first 32 bytes):
> 05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
> backtrace:
> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
> [<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
> [<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
> unreferenced object 0xffff88810776a740 (size 56):
> comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.025s)
> hex dump (first 32 bytes):
> 3d 00 00 00 00 00 00 00 3f 00 00 00 00 00 00 00 =.......?.......
> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
> backtrace:
> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
> [<ffffffff819bfcc2>] damon_set_regions+0x4c2/0x8e0
> [<ffffffff819c7dbb>] damon_do_test_apply_three_regions.constprop.0+0xfb/0x3e0
> [<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
> unreferenced object 0xffff888108038240 (size 64):
> comm "kunit_try_catch", pid 1075, jiffies 4294670600 (age 733.022s)
> hex dump (first 32 bytes):
> 00 00 00 00 00 00 00 00 03 00 00 00 6b 6b 6b 6b ............kkkk
> 48 ad 76 07 81 88 ff ff 98 ae 76 07 81 88 ff ff H.v.......v.....
> backtrace:
> [<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
> [<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
> [<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
> [<ffffffff819c898d>] damon_test_apply_three_regions4+0x1cd/0x210
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
> unreferenced object 0xffff88810776ad28 (size 56):
> comm "kunit_try_catch", pid 1075, jiffies 4294670600 (age 733.022s)
> hex dump (first 32 bytes):
> 05 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 ................
> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
> backtrace:
> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
> [<ffffffff819bfcc2>] damon_set_regions+0x4c2/0x8e0
> [<ffffffff819c7dbb>] damon_do_test_apply_three_regions.constprop.0+0xfb/0x3e0
> [<ffffffff819c898d>] damon_test_apply_three_regions4+0x1cd/0x210
> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
> [<ffffffff81237cf6>] kthread+0x2b6/0x380
> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>
> Fixes: 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary variables")
> Fixes: dae0087aeff4 ("mm/damon/vaddr: remove damon_va_apply_three_regions()")
Seems this patch fixes only 9f86d624292c if I'm not missing something? So,
could we simply remove above 'Fixes: dae0087aeff4' line?
Seems Andrew also found[1] this and made the change[2] when adding this into
the queue. Thank you, Andrew! If something wrong, please let us know, Jinjie.
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Other than above trivial nit,
Reviewed-by: SeongJae Park <sj@kernel.org>
[1] https://lore.kernel.org/damon/20230925081400.9593189a7665c6ff1f812855@linux-foundation.org/
[2] https://lore.kernel.org/mm-commits/20230925151450.EC64BC433C9@smtp.kernel.org/
Thanks,
SJ
> ---
> mm/damon/vaddr-test.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h
> index c4b455b5ee30..dcf1ca6b31cc 100644
> --- a/mm/damon/vaddr-test.h
> +++ b/mm/damon/vaddr-test.h
> @@ -148,6 +148,8 @@ static void damon_do_test_apply_three_regions(struct kunit *test,
> KUNIT_EXPECT_EQ(test, r->ar.start, expected[i * 2]);
> KUNIT_EXPECT_EQ(test, r->ar.end, expected[i * 2 + 1]);
> }
> +
> + damon_destroy_target(t);
> }
>
> /*
> --
> 2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions()
2023-09-25 16:03 ` SeongJae Park
@ 2023-09-26 1:37 ` Ruan Jinjie
0 siblings, 0 replies; 5+ messages in thread
From: Ruan Jinjie @ 2023-09-26 1:37 UTC (permalink / raw)
To: SeongJae Park; +Cc: linux-mm, damon, Andrew Morton
On 2023/9/26 0:03, SeongJae Park wrote:
> Hi Jinjie,
>
> On Mon, 25 Sep 2023 15:20:59 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote:
>
>> When CONFIG_DAMON_VADDR_KUNIT_TEST=y and making CONFIG_DEBUG_KMEMLEAK=y
>> and CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y, the below memory leak is detected.
>>
>> Since commit 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary
>> variables"), the damon_destroy_ctx() is removed, but still call
>> damon_new_target() and damon_new_region(), the damon_region which is
>> allocated by kmem_cache_alloc() in damon_new_region() and the damon_target
>> which is allocated by kmalloc in damon_new_target() are not freed. And the
>> damon_region which is allocated in damon_new_region() in
>> damon_set_regions() is also not freed.
>>
>> So use damon_destroy_target to free all the damon_regions and damon_target.
>
> Thank you for finding this bug and sending this patch!
>
>>
>> unreferenced object 0xffff888107c9a940 (size 64):
>> comm "kunit_try_catch", pid 1069, jiffies 4294670592 (age 732.761s)
>> hex dump (first 32 bytes):
>> 00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
>> 60 c7 9c 07 81 88 ff ff f8 cb 9c 07 81 88 ff ff `...............
>> backtrace:
>> [<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
>> [<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
>> [<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
>> [<ffffffff819c82be>] damon_test_apply_three_regions1+0x21e/0x260
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>> unreferenced object 0xffff8881079cc740 (size 56):
>> comm "kunit_try_catch", pid 1069, jiffies 4294670592 (age 732.761s)
>> hex dump (first 32 bytes):
>> 05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
>> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
>> backtrace:
>> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
>> [<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
>> [<ffffffff819c82be>] damon_test_apply_three_regions1+0x21e/0x260
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>> unreferenced object 0xffff888107c9ac40 (size 64):
>> comm "kunit_try_catch", pid 1071, jiffies 4294670595 (age 732.843s)
>> hex dump (first 32 bytes):
>> 00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
>> a0 cc 9c 07 81 88 ff ff 78 a1 76 07 81 88 ff ff ........x.v.....
>> backtrace:
>> [<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
>> [<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
>> [<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
>> [<ffffffff819c851e>] damon_test_apply_three_regions2+0x21e/0x260
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>> unreferenced object 0xffff8881079ccc80 (size 56):
>> comm "kunit_try_catch", pid 1071, jiffies 4294670595 (age 732.843s)
>> hex dump (first 32 bytes):
>> 05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
>> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
>> backtrace:
>> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
>> [<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
>> [<ffffffff819c851e>] damon_test_apply_three_regions2+0x21e/0x260
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>> unreferenced object 0xffff888107c9af40 (size 64):
>> comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.011s)
>> hex dump (first 32 bytes):
>> 00 00 00 00 00 00 00 00 06 00 00 00 6b 6b 6b 6b ............kkkk
>> 20 a2 76 07 81 88 ff ff b8 a6 76 07 81 88 ff ff .v.......v.....
>> backtrace:
>> [<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
>> [<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
>> [<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
>> [<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>> unreferenced object 0xffff88810776a200 (size 56):
>> comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.011s)
>> hex dump (first 32 bytes):
>> 05 00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 ................
>> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
>> backtrace:
>> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
>> [<ffffffff819c7d91>] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0
>> [<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>> unreferenced object 0xffff88810776a740 (size 56):
>> comm "kunit_try_catch", pid 1073, jiffies 4294670597 (age 733.025s)
>> hex dump (first 32 bytes):
>> 3d 00 00 00 00 00 00 00 3f 00 00 00 00 00 00 00 =.......?.......
>> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
>> backtrace:
>> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
>> [<ffffffff819bfcc2>] damon_set_regions+0x4c2/0x8e0
>> [<ffffffff819c7dbb>] damon_do_test_apply_three_regions.constprop.0+0xfb/0x3e0
>> [<ffffffff819c877e>] damon_test_apply_three_regions3+0x21e/0x260
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>> unreferenced object 0xffff888108038240 (size 64):
>> comm "kunit_try_catch", pid 1075, jiffies 4294670600 (age 733.022s)
>> hex dump (first 32 bytes):
>> 00 00 00 00 00 00 00 00 03 00 00 00 6b 6b 6b 6b ............kkkk
>> 48 ad 76 07 81 88 ff ff 98 ae 76 07 81 88 ff ff H.v.......v.....
>> backtrace:
>> [<ffffffff817e0167>] kmalloc_trace+0x27/0xa0
>> [<ffffffff819c11cf>] damon_new_target+0x3f/0x1b0
>> [<ffffffff819c7d55>] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0
>> [<ffffffff819c898d>] damon_test_apply_three_regions4+0x1cd/0x210
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>> unreferenced object 0xffff88810776ad28 (size 56):
>> comm "kunit_try_catch", pid 1075, jiffies 4294670600 (age 733.022s)
>> hex dump (first 32 bytes):
>> 05 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 ................
>> 6b 6b 6b 6b 6b 6b 6b 6b 00 00 00 00 6b 6b 6b 6b kkkkkkkk....kkkk
>> backtrace:
>> [<ffffffff819bc492>] damon_new_region+0x22/0x1c0
>> [<ffffffff819bfcc2>] damon_set_regions+0x4c2/0x8e0
>> [<ffffffff819c7dbb>] damon_do_test_apply_three_regions.constprop.0+0xfb/0x3e0
>> [<ffffffff819c898d>] damon_test_apply_three_regions4+0x1cd/0x210
>> [<ffffffff829fce6a>] kunit_generic_run_threadfn_adapter+0x4a/0x90
>> [<ffffffff81237cf6>] kthread+0x2b6/0x380
>> [<ffffffff81097add>] ret_from_fork+0x2d/0x70
>> [<ffffffff81003791>] ret_from_fork_asm+0x11/0x20
>>
>> Fixes: 9f86d624292c ("mm/damon/vaddr-test: remove unnecessary variables")
>> Fixes: dae0087aeff4 ("mm/damon/vaddr: remove damon_va_apply_three_regions()")
>
> Seems this patch fixes only 9f86d624292c if I'm not missing something? So,
> could we simply remove above 'Fixes: dae0087aeff4' line?
Right！Thank you. Commit dae0087aeff4 just replaces
damon_va_apply_three_regions() with damon_set_regions()，not first
introduce damon_set_regions().
>
> Seems Andrew also found[1] this and made the change[2] when adding this into
> the queue. Thank you, Andrew! If something wrong, please let us know, Jinjie.
>
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>
> Other than above trivial nit,
>
> Reviewed-by: SeongJae Park <sj@kernel.org>
>
> [1] https://lore.kernel.org/damon/20230925081400.9593189a7665c6ff1f812855@linux-foundation.org/
> [2] https://lore.kernel.org/mm-commits/20230925151450.EC64BC433C9@smtp.kernel.org/
>
>
> Thanks,
> SJ
>
>> ---
>> mm/damon/vaddr-test.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/damon/vaddr-test.h b/mm/damon/vaddr-test.h
>> index c4b455b5ee30..dcf1ca6b31cc 100644
>> --- a/mm/damon/vaddr-test.h
>> +++ b/mm/damon/vaddr-test.h
>> @@ -148,6 +148,8 @@ static void damon_do_test_apply_three_regions(struct kunit *test,
>> KUNIT_EXPECT_EQ(test, r->ar.start, expected[i * 2]);
>> KUNIT_EXPECT_EQ(test, r->ar.end, expected[i * 2 + 1]);
>> }
>> +
>> + damon_destroy_target(t);
>> }
>>
>> /*
>> --
>> 2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-26 1:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-25 7:20 [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions() Jinjie Ruan
2023-09-25 15:14 ` Andrew Morton
2023-09-26 1:39 ` Ruan Jinjie
2023-09-25 16:03 ` SeongJae Park
2023-09-26 1:37 ` Ruan Jinjie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox