From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 96253CE7A96 for ; Mon, 25 Sep 2023 16:03:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 19E068D002F; Mon, 25 Sep 2023 12:03:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 14EA38D0001; Mon, 25 Sep 2023 12:03:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 017138D002F; Mon, 25 Sep 2023 12:03:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id E5A3E8D0001 for ; Mon, 25 Sep 2023 12:03:34 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 41F0C140C47 for ; Mon, 25 Sep 2023 16:03:34 +0000 (UTC) X-FDA: 81275589948.22.BB405AB Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf24.hostedemail.com (Postfix) with ESMTP id 25AAB180026 for ; Mon, 25 Sep 2023 16:03:31 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=sXFobBiO; spf=pass (imf24.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695657812; a=rsa-sha256; cv=none; b=R99cdar9g1aJbZjsw6hY2UabpdW8HVeBEdJXi7SuN711F/KxF46Bz6c4qwMQgCbBSLZHqL U2XAEJGu0YWPOpmapUeVCQGdAVDmmDRDVNdiHA+3rvF+V8D+FCRKVSjeiNBugrWRILR+Ia 3XyekXdD9TIIbW/SbtMglynSWnyk1tQ= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=sXFobBiO; spf=pass (imf24.hostedemail.com: domain of sj@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=sj@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695657812; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Iv9lJ2lvgoHlofnKsXBim043+FtBPi3bJ6l7t8mdETs=; b=M9EEpPss1xkazQlOfdnR6ZyMnmN8Op97ci4+7v4s2illwk15n0UwbQuS61J5PpdHf7xL2f S47nNiOSbBfO2vGEhBZW7HfQfj+OPgUlPqrQleJ4yCFlY9j+7Tw9wAjd2mNdToRqR3/a45 nFBkuKeMWfrqhlx4qXCTwwyEwVMy7fE= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 4EDDA61086; Mon, 25 Sep 2023 16:03:31 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7CA1BC433C9; Mon, 25 Sep 2023 16:03:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1695657811; bh=vUI3Cd6cxiEQtqgzvldxnhQBLz4dRr7iFUWkLjyMIKA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sXFobBiOAqRJVBCc/rZGI8jiPns7GflU9VYy8zcUUF5FW1nwxVlgLLHHvf3v8Fmkh gjwpeOpGjfLCmGT20WFKmyIVmLb+rr5psSETivv5slQOFCCsFlkkFiuqprAKzfarr5 iP6A4E+nkI9o+pJgewHdg+O7ZmNQbcB0vBv+37B2DkrBUPvm/tRLcMd4T6jlOpvLBG BoX1rOIGSm2w0AwU/s78EBU4vd9WPVfxOeaZvkBB9FJqx+X7XZs4ymJ03y7Rp6mJxi zdXr66eHlIwAuPwMK/XkeRkwvlwB3uGfudBxSSBrIsJW/gvYvQGUUUq8nS8diW9Hg8 77CrmGGsmrimQ== From: SeongJae Park To: Jinjie Ruan Cc: linux-mm@kvack.org, damon@lists.linux.dev, SeongJae Park , Andrew Morton 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 Message-Id: <20230925160329.83109-1-sj@kernel.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230925072100.3725620-1-ruanjinjie@huawei.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 25AAB180026 X-Stat-Signature: 5ioerdo46t6jimdo5qdftdjed5jbp6jq X-Rspam-User: X-HE-Tag: 1695657811-892621 X-HE-Meta: U2FsdGVkX1+1GwqSseTnAfcTMnBclLXVOqmIzUVBsKEKwOV4pgt/N/i3pCz+KnujTQF/cpAlncT1AnLRbdyx3CWGtaFhK9m6WdYGbJcrBfEpLnWkKD/9/RgtQUx6Vr8QL5x8ZYOKAwNpARiqsRtjHhf0ppBpUbzaI2U6svyPjLykEgI/3yiqmtegRI3AMQBeUu0ORa852RM2PLTeG01NuEX57J3588uh+kPBSGMiXy0eBifD+nIIbeK8GQ+2GywKbMrTiBEnEc+cGXKcF6HvbTj8CymwQNC4SgyD0HM74WchNXzkdKPshLKg4ULjZ8ArvthIJEu8bG3zs9awIbOnHv8utbI9J2f3mcG2zfWp95MjQ49fWwS1/KswbaigDjB0A8KLGjzNpWP/y7sODPBaakVp7wPuUfgkIC+QJKUOuUSJTV0kgBR+MEyf3KThJuW8NjB2zD0U/o/XU9dgxvYwDdyeD/A+MqK8u9ll6qR7ogLvf1hx/pK08SAZdCzy1DFySi0MpiYdvw9Mar+K9mjHAJ9FRgnrPjv7IXJyaLSD5wMCA7MSmWscd6R8TvQ44yuXK/jld5zhhWnKw0FREfgKwIW6rUczZbR3h776uOYA4SzBJUD4B1QxZ9iyBo8XxnaRFX5q1JnJQ/xJgIrytgOmd97Uju6tcc0aPnBu8d9LVSm+MTxiEnV7M3DdllM0TA+38a7FHJnq4skHGsJIxANsK+rjGwVu9MRRdUcanoG4+NhqGga8SoYjfE5vnXOgwFVfLrUw7IfzQ0t1GEcGalSpykBtQgyXR6GoHhOdADOtJrqh6I4BNnaK/iQVKGeYo15PmyDhcBc6sbcgNS+JD+++4nTzwdzrkRvkAXdw1Nozvt50a5fs/Os1QcLCpaLO9ELcFDZXYZCygcJuxhBIc8t1y2ObToC4huI1bwM3SnpTnj2tAeGBnrdbxXFPAdfHBRoc22smwWgUJeIMlPfTqaU MZwNOhUn bOKOoCqhRPY768LMcNpy43kuNxJlYOrk0CB/6asQIZ3PCTBEvnD6Ttc0u2nQcnxWApNMZ0V61DyGnzsaf4jNJBdT4ZFT4eyhYz9Q1ctpGI+6wm3A0w2dbUG895Q8ZcGwjwCxv86YBkwCznkViAZ20l3o9NH/YZ/9WhJjRuMxOpY5yP2E/5sLxpQwbfSZKzU95CrhDBVzHhbqrO+tPcCcdzLwjEDiWRGX7ca+ofkvWe+SJdnPI/8M7A74m4qUPu8akKjWnERJYMTqlGqnRTmt+rUdi+R4e6gZqOnwBb86P6Z/UR3jjbdSz02+++lClwNulaQsBzoM0/8F3l/k+uj8Bz70BGYEpYeLrsypH72Ihqkdq1RB4WEwjMkdJFg== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Jinjie, On Mon, 25 Sep 2023 15:20:59 +0800 Jinjie Ruan 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: > [] kmalloc_trace+0x27/0xa0 > [] damon_new_target+0x3f/0x1b0 > [] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0 > [] damon_test_apply_three_regions1+0x21e/0x260 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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: > [] damon_new_region+0x22/0x1c0 > [] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0 > [] damon_test_apply_three_regions1+0x21e/0x260 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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: > [] kmalloc_trace+0x27/0xa0 > [] damon_new_target+0x3f/0x1b0 > [] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0 > [] damon_test_apply_three_regions2+0x21e/0x260 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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: > [] damon_new_region+0x22/0x1c0 > [] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0 > [] damon_test_apply_three_regions2+0x21e/0x260 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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: > [] kmalloc_trace+0x27/0xa0 > [] damon_new_target+0x3f/0x1b0 > [] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0 > [] damon_test_apply_three_regions3+0x21e/0x260 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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: > [] damon_new_region+0x22/0x1c0 > [] damon_do_test_apply_three_regions.constprop.0+0xd1/0x3e0 > [] damon_test_apply_three_regions3+0x21e/0x260 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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: > [] damon_new_region+0x22/0x1c0 > [] damon_set_regions+0x4c2/0x8e0 > [] damon_do_test_apply_three_regions.constprop.0+0xfb/0x3e0 > [] damon_test_apply_three_regions3+0x21e/0x260 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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: > [] kmalloc_trace+0x27/0xa0 > [] damon_new_target+0x3f/0x1b0 > [] damon_do_test_apply_three_regions.constprop.0+0x95/0x3e0 > [] damon_test_apply_three_regions4+0x1cd/0x210 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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: > [] damon_new_region+0x22/0x1c0 > [] damon_set_regions+0x4c2/0x8e0 > [] damon_do_test_apply_three_regions.constprop.0+0xfb/0x3e0 > [] damon_test_apply_three_regions4+0x1cd/0x210 > [] kunit_generic_run_threadfn_adapter+0x4a/0x90 > [] kthread+0x2b6/0x380 > [] ret_from_fork+0x2d/0x70 > [] 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 Other than above trivial nit, Reviewed-by: SeongJae Park [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