From: SeongJae Park <sj@kernel.org>
To: Jinjie Ruan <ruanjinjie@huawei.com>
Cc: linux-mm@kvack.org, damon@lists.linux.dev,
SeongJae Park <sj@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] mm/damon/vaddr-test: Fix memory leak in damon_do_test_apply_three_regions()
Date: Mon, 25 Sep 2023 16:03:29 +0000 [thread overview]
Message-ID: <20230925160329.83109-1-sj@kernel.org> (raw)
In-Reply-To: <20230925072100.3725620-1-ruanjinjie@huawei.com>
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
next prev parent reply other threads:[~2023-09-25 16:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2023-09-26 1:37 ` Ruan Jinjie
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230925160329.83109-1-sj@kernel.org \
--to=sj@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=damon@lists.linux.dev \
--cc=linux-mm@kvack.org \
--cc=ruanjinjie@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.