* [PATCH RESEND 0/2] mm/damon/core-test: Fix memory leaks in core-test @ 2023-09-18 5:10 Jinjie Ruan 2023-09-18 5:10 ` [PATCH RESEND 1/2] mm/damon/core-test: Fix memory leak in damon_new_region() Jinjie Ruan 2023-09-18 5:10 ` [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() Jinjie Ruan 0 siblings, 2 replies; 8+ messages in thread From: Jinjie Ruan @ 2023-09-18 5:10 UTC (permalink / raw) To: sj, akpm, brendan.higgins, feng.tang, damon, linux-mm, kunit-dev, linux-kselftest, linux-kernel Cc: ruanjinjie There are a few memory leak in core-test which is detected by kmemleak, the patch set fix the above issue. Jinjie Ruan (2): mm/damon/core-test: Fix memory leak in damon_new_region() mm/damon/core-test: Fix memory leak in damon_new_ctx() mm/damon/core-test.h | 7 +++++++ 1 file changed, 7 insertions(+) -- 2.34.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH RESEND 1/2] mm/damon/core-test: Fix memory leak in damon_new_region() 2023-09-18 5:10 [PATCH RESEND 0/2] mm/damon/core-test: Fix memory leaks in core-test Jinjie Ruan @ 2023-09-18 5:10 ` Jinjie Ruan 2023-09-18 5:33 ` SeongJae Park 2023-09-18 5:10 ` [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() Jinjie Ruan 1 sibling, 1 reply; 8+ messages in thread From: Jinjie Ruan @ 2023-09-18 5:10 UTC (permalink / raw) To: sj, akpm, brendan.higgins, feng.tang, damon, linux-mm, kunit-dev, linux-kselftest, linux-kernel Cc: ruanjinjie The damon_region which is allocated by kmem_cache_alloc() in damon_new_region() in damon_test_regions() and damon_test_update_monitoring_result() are not freed and it causes below memory leak. So use damon_free_region() to free it. unreferenced object 0xffff2b49c3edc000 (size 56): comm "kunit_try_catch", pid 338, jiffies 4294895280 (age 557.084s) hex dump (first 32 bytes): 01 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 00 00 00 00 49 2b ff ff ............I+.. backtrace: [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 [<00000000b528f67c>] kmem_cache_alloc+0x168/0x284 [<000000008603f022>] damon_new_region+0x28/0x54 [<00000000a3b8c64e>] damon_test_regions+0x38/0x270 [<00000000559c4801>] kunit_try_run_case+0x50/0xac [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c [<000000003c3e9211>] kthread+0x124/0x130 [<0000000028f85bdd>] ret_from_fork+0x10/0x20 unreferenced object 0xffff2b49c5b20000 (size 56): comm "kunit_try_catch", pid 354, jiffies 4294895304 (age 556.988s) hex dump (first 32 bytes): 03 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 96 00 00 00 49 2b ff ff ............I+.. backtrace: [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 [<00000000b528f67c>] kmem_cache_alloc+0x168/0x284 [<000000008603f022>] damon_new_region+0x28/0x54 [<00000000ca019f80>] damon_test_update_monitoring_result+0x18/0x34 [<00000000559c4801>] kunit_try_run_case+0x50/0xac [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c [<000000003c3e9211>] kthread+0x124/0x130 [<0000000028f85bdd>] ret_from_fork+0x10/0x20 Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests") Fixes: f4c978b6594b ("mm/damon/core-test: add a test for damon_update_monitoring_results()") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- mm/damon/core-test.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h index 6cc8b245586d..255f8c925c00 100644 --- a/mm/damon/core-test.h +++ b/mm/damon/core-test.h @@ -34,6 +34,7 @@ static void damon_test_regions(struct kunit *test) KUNIT_EXPECT_EQ(test, 0u, damon_nr_regions(t)); damon_free_target(t); + damon_free_region(r); } static unsigned int nr_damon_targets(struct damon_ctx *ctx) @@ -316,6 +317,8 @@ static void damon_test_update_monitoring_result(struct kunit *test) damon_update_monitoring_result(r, &old_attrs, &new_attrs); KUNIT_EXPECT_EQ(test, r->nr_accesses, 150); KUNIT_EXPECT_EQ(test, r->age, 20); + + damon_free_region(r); } static void damon_test_set_attrs(struct kunit *test) -- 2.34.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 1/2] mm/damon/core-test: Fix memory leak in damon_new_region() 2023-09-18 5:10 ` [PATCH RESEND 1/2] mm/damon/core-test: Fix memory leak in damon_new_region() Jinjie Ruan @ 2023-09-18 5:33 ` SeongJae Park 2023-09-18 7:13 ` Ruan Jinjie 0 siblings, 1 reply; 8+ messages in thread From: SeongJae Park @ 2023-09-18 5:33 UTC (permalink / raw) To: Jinjie Ruan Cc: sj, akpm, brendan.higgins, feng.tang, damon, linux-mm, kunit-dev, linux-kselftest, linux-kernel Hi Jinjie, Thank you for this patchset! On Mon, 18 Sep 2023 13:10:43 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote: > The damon_region which is allocated by kmem_cache_alloc() in > damon_new_region() in damon_test_regions() and > damon_test_update_monitoring_result() are not freed and it causes below > memory leak. So use damon_free_region() to free it. > > unreferenced object 0xffff2b49c3edc000 (size 56): > comm "kunit_try_catch", pid 338, jiffies 4294895280 (age 557.084s) > hex dump (first 32 bytes): > 01 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00 ................ > 00 00 00 00 00 00 00 00 00 00 00 00 49 2b ff ff ............I+.. > backtrace: > [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 > [<00000000b528f67c>] kmem_cache_alloc+0x168/0x284 > [<000000008603f022>] damon_new_region+0x28/0x54 > [<00000000a3b8c64e>] damon_test_regions+0x38/0x270 > [<00000000559c4801>] kunit_try_run_case+0x50/0xac > [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c > [<000000003c3e9211>] kthread+0x124/0x130 > [<0000000028f85bdd>] ret_from_fork+0x10/0x20 > unreferenced object 0xffff2b49c5b20000 (size 56): > comm "kunit_try_catch", pid 354, jiffies 4294895304 (age 556.988s) > hex dump (first 32 bytes): > 03 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 ................ > 00 00 00 00 00 00 00 00 96 00 00 00 49 2b ff ff ............I+.. > backtrace: > [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 > [<00000000b528f67c>] kmem_cache_alloc+0x168/0x284 > [<000000008603f022>] damon_new_region+0x28/0x54 > [<00000000ca019f80>] damon_test_update_monitoring_result+0x18/0x34 > [<00000000559c4801>] kunit_try_run_case+0x50/0xac > [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c > [<000000003c3e9211>] kthread+0x124/0x130 > [<0000000028f85bdd>] ret_from_fork+0x10/0x20 Nice finding! Could you please share just a brief more detail about above cool output, e.g., just the name of the tool you used, so that others can learn it from your awesome commit message? > > Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests") > Fixes: f4c978b6594b ("mm/damon/core-test: add a test for damon_update_monitoring_results()") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > mm/damon/core-test.h | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h > index 6cc8b245586d..255f8c925c00 100644 > --- a/mm/damon/core-test.h > +++ b/mm/damon/core-test.h > @@ -34,6 +34,7 @@ static void damon_test_regions(struct kunit *test) > KUNIT_EXPECT_EQ(test, 0u, damon_nr_regions(t)); > > damon_free_target(t); > + damon_free_region(r); There is damon_destroy_region() function, which simply calls damon_del_region() and damon_free_region(). Unless there is needs to access the region before removing from the region, doing memory return together via the function is recommended. And this test code calls damon_del_region() just beofre above KUNIT_EXPECT_EQ(). Hence, I think replacing the damon_del_region() call with damon_destroy_region() rather than calling damon_free_region() may be simpler and shorter. Could you please do so? > } > > static unsigned int nr_damon_targets(struct damon_ctx *ctx) > @@ -316,6 +317,8 @@ static void damon_test_update_monitoring_result(struct kunit *test) > damon_update_monitoring_result(r, &old_attrs, &new_attrs); > KUNIT_EXPECT_EQ(test, r->nr_accesses, 150); > KUNIT_EXPECT_EQ(test, r->age, 20); > + > + damon_free_region(r); This looks nice. Thank you for fixing this! > } > > static void damon_test_set_attrs(struct kunit *test) > -- > 2.34.1 > Thanks, SJ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 1/2] mm/damon/core-test: Fix memory leak in damon_new_region() 2023-09-18 5:33 ` SeongJae Park @ 2023-09-18 7:13 ` Ruan Jinjie 0 siblings, 0 replies; 8+ messages in thread From: Ruan Jinjie @ 2023-09-18 7:13 UTC (permalink / raw) To: SeongJae Park Cc: akpm, brendan.higgins, feng.tang, damon, linux-mm, kunit-dev, linux-kselftest, linux-kernel On 2023/9/18 13:33, SeongJae Park wrote: > Hi Jinjie, > > > Thank you for this patchset! > > On Mon, 18 Sep 2023 13:10:43 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote: > >> The damon_region which is allocated by kmem_cache_alloc() in >> damon_new_region() in damon_test_regions() and >> damon_test_update_monitoring_result() are not freed and it causes below >> memory leak. So use damon_free_region() to free it. >> >> unreferenced object 0xffff2b49c3edc000 (size 56): >> comm "kunit_try_catch", pid 338, jiffies 4294895280 (age 557.084s) >> hex dump (first 32 bytes): >> 01 00 00 00 00 00 00 00 02 00 00 00 00 00 00 00 ................ >> 00 00 00 00 00 00 00 00 00 00 00 00 49 2b ff ff ............I+.. >> backtrace: >> [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 >> [<00000000b528f67c>] kmem_cache_alloc+0x168/0x284 >> [<000000008603f022>] damon_new_region+0x28/0x54 >> [<00000000a3b8c64e>] damon_test_regions+0x38/0x270 >> [<00000000559c4801>] kunit_try_run_case+0x50/0xac >> [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c >> [<000000003c3e9211>] kthread+0x124/0x130 >> [<0000000028f85bdd>] ret_from_fork+0x10/0x20 >> unreferenced object 0xffff2b49c5b20000 (size 56): >> comm "kunit_try_catch", pid 354, jiffies 4294895304 (age 556.988s) >> hex dump (first 32 bytes): >> 03 00 00 00 00 00 00 00 07 00 00 00 00 00 00 00 ................ >> 00 00 00 00 00 00 00 00 96 00 00 00 49 2b ff ff ............I+.. >> backtrace: >> [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 >> [<00000000b528f67c>] kmem_cache_alloc+0x168/0x284 >> [<000000008603f022>] damon_new_region+0x28/0x54 >> [<00000000ca019f80>] damon_test_update_monitoring_result+0x18/0x34 >> [<00000000559c4801>] kunit_try_run_case+0x50/0xac >> [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c >> [<000000003c3e9211>] kthread+0x124/0x130 >> [<0000000028f85bdd>] ret_from_fork+0x10/0x20 > > Nice finding! Could you please share just a brief more detail about above cool > output, e.g., just the name of the tool you used, so that others can learn it > from your awesome commit message? > >> >> Fixes: 17ccae8bb5c9 ("mm/damon: add kunit tests") >> Fixes: f4c978b6594b ("mm/damon/core-test: add a test for damon_update_monitoring_results()") >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >> --- >> mm/damon/core-test.h | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h >> index 6cc8b245586d..255f8c925c00 100644 >> --- a/mm/damon/core-test.h >> +++ b/mm/damon/core-test.h >> @@ -34,6 +34,7 @@ static void damon_test_regions(struct kunit *test) >> KUNIT_EXPECT_EQ(test, 0u, damon_nr_regions(t)); >> >> damon_free_target(t); >> + damon_free_region(r); > > There is damon_destroy_region() function, which simply calls damon_del_region() > and damon_free_region(). Unless there is needs to access the region before > removing from the region, doing memory return together via the function is > recommended. > > And this test code calls damon_del_region() just beofre above > KUNIT_EXPECT_EQ(). Hence, I think replacing the damon_del_region() call with > damon_destroy_region() rather than calling damon_free_region() may be simpler > and shorter. Could you please do so? Sure. Thank you very much! > >> } >> >> static unsigned int nr_damon_targets(struct damon_ctx *ctx) >> @@ -316,6 +317,8 @@ static void damon_test_update_monitoring_result(struct kunit *test) >> damon_update_monitoring_result(r, &old_attrs, &new_attrs); >> KUNIT_EXPECT_EQ(test, r->nr_accesses, 150); >> KUNIT_EXPECT_EQ(test, r->age, 20); >> + >> + damon_free_region(r); > > This looks nice. Thank you for fixing this! > >> } >> >> static void damon_test_set_attrs(struct kunit *test) >> -- >> 2.34.1 >> > > Thanks, > SJ ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() 2023-09-18 5:10 [PATCH RESEND 0/2] mm/damon/core-test: Fix memory leaks in core-test Jinjie Ruan 2023-09-18 5:10 ` [PATCH RESEND 1/2] mm/damon/core-test: Fix memory leak in damon_new_region() Jinjie Ruan @ 2023-09-18 5:10 ` Jinjie Ruan 2023-09-18 5:38 ` Feng Tang 2023-09-18 5:43 ` SeongJae Park 1 sibling, 2 replies; 8+ messages in thread From: Jinjie Ruan @ 2023-09-18 5:10 UTC (permalink / raw) To: sj, akpm, brendan.higgins, feng.tang, damon, linux-mm, kunit-dev, linux-kselftest, linux-kernel Cc: ruanjinjie The damon_ctx which is allocated by kzalloc() in damon_new_ctx() in damon_test_ops_registration() and damon_test_set_attrs() are not freed and it causes below memory leak. So use damon_destroy_ctx() to free it. unreferenced object 0xffff2b49c6968800 (size 512): comm "kunit_try_catch", pid 350, jiffies 4294895294 (age 557.028s) hex dump (first 32 bytes): 88 13 00 00 00 00 00 00 a0 86 01 00 00 00 00 00 ................ 00 87 93 03 00 00 00 00 0a 00 00 00 00 00 00 00 ................ backtrace: [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 [<0000000073acab3b>] __kmem_cache_alloc_node+0x174/0x290 [<00000000b5f89cef>] kmalloc_trace+0x40/0x164 [<00000000eb19e83f>] damon_new_ctx+0x28/0xb4 [<00000000daf6227b>] damon_test_ops_registration+0x34/0x328 [<00000000559c4801>] kunit_try_run_case+0x50/0xac [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c [<000000003c3e9211>] kthread+0x124/0x130 [<0000000028f85bdd>] ret_from_fork+0x10/0x20 unreferenced object 0xffff2b49c1a9cc00 (size 512): comm "kunit_try_catch", pid 356, jiffies 4294895306 (age 557.000s) hex dump (first 32 bytes): 88 13 00 00 00 00 00 00 a0 86 01 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 ................ backtrace: [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 [<0000000073acab3b>] __kmem_cache_alloc_node+0x174/0x290 [<00000000b5f89cef>] kmalloc_trace+0x40/0x164 [<00000000eb19e83f>] damon_new_ctx+0x28/0xb4 [<00000000058495c4>] damon_test_set_attrs+0x30/0x1a8 [<00000000559c4801>] kunit_try_run_case+0x50/0xac [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c [<000000003c3e9211>] kthread+0x124/0x130 [<0000000028f85bdd>] ret_from_fork+0x10/0x20 Fixes: d1836a3b2a9a ("mm/damon/core-test: initialise context before test in damon_test_set_attrs()") Fixes: 4f540f5ab4f2 ("mm/damon/core-test: add a kunit test case for ops registration") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> --- mm/damon/core-test.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h index 255f8c925c00..ce86ed30fb47 100644 --- a/mm/damon/core-test.h +++ b/mm/damon/core-test.h @@ -266,6 +266,8 @@ static void damon_test_ops_registration(struct kunit *test) /* Check double-registration failure again */ KUNIT_EXPECT_EQ(test, damon_register_ops(&ops), -EINVAL); + + damon_destroy_ctx(c); } static void damon_test_set_regions(struct kunit *test) @@ -342,6 +344,8 @@ static void damon_test_set_attrs(struct kunit *test) invalid_attrs = valid_attrs; invalid_attrs.aggr_interval = 4999; KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); + + damon_destroy_ctx(c); } static void damos_test_new_filter(struct kunit *test) -- 2.34.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() 2023-09-18 5:10 ` [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() Jinjie Ruan @ 2023-09-18 5:38 ` Feng Tang 2023-09-18 5:43 ` SeongJae Park 1 sibling, 0 replies; 8+ messages in thread From: Feng Tang @ 2023-09-18 5:38 UTC (permalink / raw) To: Jinjie Ruan Cc: sj, akpm, brendan.higgins, damon, linux-mm, kunit-dev, linux-kselftest, linux-kernel On Mon, Sep 18, 2023 at 01:10:44PM +0800, Jinjie Ruan wrote: > The damon_ctx which is allocated by kzalloc() in damon_new_ctx() in > damon_test_ops_registration() and damon_test_set_attrs() are not freed and > it causes below memory leak. So use damon_destroy_ctx() to free it. > > unreferenced object 0xffff2b49c6968800 (size 512): > comm "kunit_try_catch", pid 350, jiffies 4294895294 (age 557.028s) > hex dump (first 32 bytes): > 88 13 00 00 00 00 00 00 a0 86 01 00 00 00 00 00 ................ > 00 87 93 03 00 00 00 00 0a 00 00 00 00 00 00 00 ................ > backtrace: > [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 > [<0000000073acab3b>] __kmem_cache_alloc_node+0x174/0x290 > [<00000000b5f89cef>] kmalloc_trace+0x40/0x164 > [<00000000eb19e83f>] damon_new_ctx+0x28/0xb4 > [<00000000daf6227b>] damon_test_ops_registration+0x34/0x328 > [<00000000559c4801>] kunit_try_run_case+0x50/0xac > [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c > [<000000003c3e9211>] kthread+0x124/0x130 > [<0000000028f85bdd>] ret_from_fork+0x10/0x20 > unreferenced object 0xffff2b49c1a9cc00 (size 512): > comm "kunit_try_catch", pid 356, jiffies 4294895306 (age 557.000s) > hex dump (first 32 bytes): > 88 13 00 00 00 00 00 00 a0 86 01 00 00 00 00 00 ................ > 00 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 ................ > backtrace: > [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 > [<0000000073acab3b>] __kmem_cache_alloc_node+0x174/0x290 > [<00000000b5f89cef>] kmalloc_trace+0x40/0x164 > [<00000000eb19e83f>] damon_new_ctx+0x28/0xb4 > [<00000000058495c4>] damon_test_set_attrs+0x30/0x1a8 > [<00000000559c4801>] kunit_try_run_case+0x50/0xac > [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c > [<000000003c3e9211>] kthread+0x124/0x130 > [<0000000028f85bdd>] ret_from_fork+0x10/0x20 > > Fixes: d1836a3b2a9a ("mm/damon/core-test: initialise context before test in damon_test_set_attrs()") Thanks for the fix! Reviewed-by: Feng Tang <feng.tang@intel.com> > Fixes: 4f540f5ab4f2 ("mm/damon/core-test: add a kunit test case for ops registration") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > mm/damon/core-test.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h > index 255f8c925c00..ce86ed30fb47 100644 > --- a/mm/damon/core-test.h > +++ b/mm/damon/core-test.h > @@ -266,6 +266,8 @@ static void damon_test_ops_registration(struct kunit *test) > > /* Check double-registration failure again */ > KUNIT_EXPECT_EQ(test, damon_register_ops(&ops), -EINVAL); > + > + damon_destroy_ctx(c); > } > > static void damon_test_set_regions(struct kunit *test) > @@ -342,6 +344,8 @@ static void damon_test_set_attrs(struct kunit *test) > invalid_attrs = valid_attrs; > invalid_attrs.aggr_interval = 4999; > KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); > + > + damon_destroy_ctx(c); > } > > static void damos_test_new_filter(struct kunit *test) > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() 2023-09-18 5:10 ` [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() Jinjie Ruan 2023-09-18 5:38 ` Feng Tang @ 2023-09-18 5:43 ` SeongJae Park 2023-09-18 7:12 ` Ruan Jinjie 1 sibling, 1 reply; 8+ messages in thread From: SeongJae Park @ 2023-09-18 5:43 UTC (permalink / raw) To: Jinjie Ruan Cc: sj, akpm, brendan.higgins, feng.tang, damon, linux-mm, kunit-dev, linux-kselftest, linux-kernel Hi Jinjie, On Mon, 18 Sep 2023 13:10:44 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote: > The damon_ctx which is allocated by kzalloc() in damon_new_ctx() in > damon_test_ops_registration() and damon_test_set_attrs() are not freed and > it causes below memory leak. So use damon_destroy_ctx() to free it. > > unreferenced object 0xffff2b49c6968800 (size 512): > comm "kunit_try_catch", pid 350, jiffies 4294895294 (age 557.028s) > hex dump (first 32 bytes): > 88 13 00 00 00 00 00 00 a0 86 01 00 00 00 00 00 ................ > 00 87 93 03 00 00 00 00 0a 00 00 00 00 00 00 00 ................ > backtrace: > [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 > [<0000000073acab3b>] __kmem_cache_alloc_node+0x174/0x290 > [<00000000b5f89cef>] kmalloc_trace+0x40/0x164 > [<00000000eb19e83f>] damon_new_ctx+0x28/0xb4 > [<00000000daf6227b>] damon_test_ops_registration+0x34/0x328 > [<00000000559c4801>] kunit_try_run_case+0x50/0xac > [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c > [<000000003c3e9211>] kthread+0x124/0x130 > [<0000000028f85bdd>] ret_from_fork+0x10/0x20 > unreferenced object 0xffff2b49c1a9cc00 (size 512): > comm "kunit_try_catch", pid 356, jiffies 4294895306 (age 557.000s) > hex dump (first 32 bytes): > 88 13 00 00 00 00 00 00 a0 86 01 00 00 00 00 00 ................ > 00 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 ................ > backtrace: > [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 > [<0000000073acab3b>] __kmem_cache_alloc_node+0x174/0x290 > [<00000000b5f89cef>] kmalloc_trace+0x40/0x164 > [<00000000eb19e83f>] damon_new_ctx+0x28/0xb4 > [<00000000058495c4>] damon_test_set_attrs+0x30/0x1a8 > [<00000000559c4801>] kunit_try_run_case+0x50/0xac > [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c > [<000000003c3e9211>] kthread+0x124/0x130 > [<0000000028f85bdd>] ret_from_fork+0x10/0x20 Same to the comment on the first patch of this patchset, simply letting others know the name of the tool you used for making this cool output could be helpful. Could you please add it? Also, I prefer adding four spaces indent to quoted command outputs on commit messages. Could you please add that indentation? Also, I cannot 'git am' this patch on top of mm-unstable[1]. Could you please double check and rebase on latest mm-unstable if I din't miss something? [1] https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/log/?h=mm-unstable > > Fixes: d1836a3b2a9a ("mm/damon/core-test: initialise context before test in damon_test_set_attrs()") > Fixes: 4f540f5ab4f2 ("mm/damon/core-test: add a kunit test case for ops registration") > Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> > --- > mm/damon/core-test.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h > index 255f8c925c00..ce86ed30fb47 100644 > --- a/mm/damon/core-test.h > +++ b/mm/damon/core-test.h > @@ -266,6 +266,8 @@ static void damon_test_ops_registration(struct kunit *test) > > /* Check double-registration failure again */ > KUNIT_EXPECT_EQ(test, damon_register_ops(&ops), -EINVAL); > + > + damon_destroy_ctx(c); > } > > static void damon_test_set_regions(struct kunit *test) > @@ -342,6 +344,8 @@ static void damon_test_set_attrs(struct kunit *test) > invalid_attrs = valid_attrs; > invalid_attrs.aggr_interval = 4999; > KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); > + > + damon_destroy_ctx(c); > } > > static void damos_test_new_filter(struct kunit *test) Other than above comments, changes look good to me. > -- > 2.34.1 > Thanks, SJ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() 2023-09-18 5:43 ` SeongJae Park @ 2023-09-18 7:12 ` Ruan Jinjie 0 siblings, 0 replies; 8+ messages in thread From: Ruan Jinjie @ 2023-09-18 7:12 UTC (permalink / raw) To: SeongJae Park Cc: akpm, brendan.higgins, feng.tang, damon, linux-mm, kunit-dev, linux-kselftest, linux-kernel On 2023/9/18 13:43, SeongJae Park wrote: > Hi Jinjie, > > On Mon, 18 Sep 2023 13:10:44 +0800 Jinjie Ruan <ruanjinjie@huawei.com> wrote: > >> The damon_ctx which is allocated by kzalloc() in damon_new_ctx() in >> damon_test_ops_registration() and damon_test_set_attrs() are not freed and >> it causes below memory leak. So use damon_destroy_ctx() to free it. >> >> unreferenced object 0xffff2b49c6968800 (size 512): >> comm "kunit_try_catch", pid 350, jiffies 4294895294 (age 557.028s) >> hex dump (first 32 bytes): >> 88 13 00 00 00 00 00 00 a0 86 01 00 00 00 00 00 ................ >> 00 87 93 03 00 00 00 00 0a 00 00 00 00 00 00 00 ................ >> backtrace: >> [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 >> [<0000000073acab3b>] __kmem_cache_alloc_node+0x174/0x290 >> [<00000000b5f89cef>] kmalloc_trace+0x40/0x164 >> [<00000000eb19e83f>] damon_new_ctx+0x28/0xb4 >> [<00000000daf6227b>] damon_test_ops_registration+0x34/0x328 >> [<00000000559c4801>] kunit_try_run_case+0x50/0xac >> [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c >> [<000000003c3e9211>] kthread+0x124/0x130 >> [<0000000028f85bdd>] ret_from_fork+0x10/0x20 >> unreferenced object 0xffff2b49c1a9cc00 (size 512): >> comm "kunit_try_catch", pid 356, jiffies 4294895306 (age 557.000s) >> hex dump (first 32 bytes): >> 88 13 00 00 00 00 00 00 a0 86 01 00 00 00 00 00 ................ >> 00 00 00 00 00 00 00 00 0a 00 00 00 00 00 00 00 ................ >> backtrace: >> [<0000000088e71769>] slab_post_alloc_hook+0xb8/0x368 >> [<0000000073acab3b>] __kmem_cache_alloc_node+0x174/0x290 >> [<00000000b5f89cef>] kmalloc_trace+0x40/0x164 >> [<00000000eb19e83f>] damon_new_ctx+0x28/0xb4 >> [<00000000058495c4>] damon_test_set_attrs+0x30/0x1a8 >> [<00000000559c4801>] kunit_try_run_case+0x50/0xac >> [<000000003932ed49>] kunit_generic_run_threadfn_adapter+0x20/0x2c >> [<000000003c3e9211>] kthread+0x124/0x130 >> [<0000000028f85bdd>] ret_from_fork+0x10/0x20 > > Same to the comment on the first patch of this patchset, simply letting others > know the name of the tool you used for making this cool output could be > helpful. Could you please add it? > > Also, I prefer adding four spaces indent to quoted command outputs on commit > messages. Could you please add that indentation? > > Also, I cannot 'git am' this patch on top of mm-unstable[1]. Could you please > double check and rebase on latest mm-unstable if I din't miss something? > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/log/?h=mm-unstable Sure. Thank you very much! > >> >> Fixes: d1836a3b2a9a ("mm/damon/core-test: initialise context before test in damon_test_set_attrs()") >> Fixes: 4f540f5ab4f2 ("mm/damon/core-test: add a kunit test case for ops registration") >> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> >> --- >> mm/damon/core-test.h | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/mm/damon/core-test.h b/mm/damon/core-test.h >> index 255f8c925c00..ce86ed30fb47 100644 >> --- a/mm/damon/core-test.h >> +++ b/mm/damon/core-test.h >> @@ -266,6 +266,8 @@ static void damon_test_ops_registration(struct kunit *test) >> >> /* Check double-registration failure again */ >> KUNIT_EXPECT_EQ(test, damon_register_ops(&ops), -EINVAL); >> + >> + damon_destroy_ctx(c); >> } >> >> static void damon_test_set_regions(struct kunit *test) >> @@ -342,6 +344,8 @@ static void damon_test_set_attrs(struct kunit *test) >> invalid_attrs = valid_attrs; >> invalid_attrs.aggr_interval = 4999; >> KUNIT_EXPECT_EQ(test, damon_set_attrs(c, &invalid_attrs), -EINVAL); >> + >> + damon_destroy_ctx(c); >> } >> >> static void damos_test_new_filter(struct kunit *test) > > Other than above comments, changes look good to me. > >> -- >> 2.34.1 >> > > Thanks, > SJ ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-09-18 7:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-09-18 5:10 [PATCH RESEND 0/2] mm/damon/core-test: Fix memory leaks in core-test Jinjie Ruan 2023-09-18 5:10 ` [PATCH RESEND 1/2] mm/damon/core-test: Fix memory leak in damon_new_region() Jinjie Ruan 2023-09-18 5:33 ` SeongJae Park 2023-09-18 7:13 ` Ruan Jinjie 2023-09-18 5:10 ` [PATCH RESEND 2/2] mm/damon/core-test: Fix memory leak in damon_new_ctx() Jinjie Ruan 2023-09-18 5:38 ` Feng Tang 2023-09-18 5:43 ` SeongJae Park 2023-09-18 7:12 ` Ruan Jinjie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox