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 05D2ACD37B0 for ; Mon, 18 Sep 2023 05:33:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2EC9C6B020C; Mon, 18 Sep 2023 01:33:32 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 29C456B020D; Mon, 18 Sep 2023 01:33:32 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 18BE76B020E; Mon, 18 Sep 2023 01:33:32 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 057706B020C for ; Mon, 18 Sep 2023 01:33:32 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id B527D40AFA for ; Mon, 18 Sep 2023 05:33:31 +0000 (UTC) X-FDA: 81248600622.06.2807F33 Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf23.hostedemail.com (Postfix) with ESMTP id 4327414000E for ; Mon, 18 Sep 2023 05:33:28 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=TANfFm6f; spf=pass (imf23.hostedemail.com: domain of sj@kernel.org designates 145.40.73.55 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=1695015210; a=rsa-sha256; cv=none; b=O8Q7xHiRFuq4qwunzuCz5fXJOIZ1QuRIb11kP0sGQRyrjjPOo18z4S6wUYTQGzOnTUE94A foUeenC4/GLA/D6fXF35BCE1DTSktXVeAUiI2i70Tr+qO33wwl4CxoB4OtxQFWZs896/qC EztqcQUfjPItlkp347AfqlZbdUoMfs8= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=TANfFm6f; spf=pass (imf23.hostedemail.com: domain of sj@kernel.org designates 145.40.73.55 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=1695015210; 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=g0MqUGlyMr1jmippxkUTdaEAgs+P965wOxDfujoDKBU=; b=p5n3XH5pHYN+fw1rM8Jvnst4T63FYRHgajbaVyvgB8c/Se9Nzeuq9M+E06hA4+5ngbuR6K sIf6mSFlM59qkxvNq4o1FEi5T5J4MaNrOClhqrEQX/X9ck/P216sWz4n7Kmdsg9DWeM7f6 71Cya73yEobyKOBen0VmxYnKM3Wmox8= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by sin.source.kernel.org (Postfix) with ESMTPS id BA180CE0AF5; Mon, 18 Sep 2023 05:33:24 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 431A9C433C7; Mon, 18 Sep 2023 05:33:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1695015203; bh=ZSre9xUXaczWj7SY0m81+m7rnmYcDRo0030/fgV5cdI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TANfFm6fmw4upz/D15/HMmKIdtp3e7oAQqqcn8BfeWoarFhn/LkGWZVe+7snKEyFU cOWWysL8c5SUUGGBnUIv6wmbUULwwtOE9dbAhECEdk8AgmsmnZpToArMXwUdcU8DRc 9gVFthnkt2IsWTRE+5XfZH07iVZwe00q3psZPuZC90fCEuabqwzaoTbk5ZxSPlnmQD cyV1Sg+jseVykrqA0XgXx81JSvKC4wGScf93+qIjP1sFgYQtvQlz7+NPbpwLaZ/qrL mjsCOeFWu1w4nnP5tNhQg3LKe801Xg0lJE/BSCrgLPBhx2MBDiJKx7MFSR+jUj2hhG s2g4KjcNWRBVw== From: SeongJae Park To: Jinjie Ruan Cc: sj@kernel.org, akpm@linux-foundation.org, brendan.higgins@linux.dev, feng.tang@intel.com, damon@lists.linux.dev, linux-mm@kvack.org, kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH RESEND 1/2] mm/damon/core-test: Fix memory leak in damon_new_region() Date: Mon, 18 Sep 2023 05:33:20 +0000 Message-Id: <20230918053320.61408-1-sj@kernel.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230918051044.3814018-2-ruanjinjie@huawei.com> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 4327414000E X-Stat-Signature: 343run4b8e6a89ji7sdzr5qsa4poputw X-Rspam-User: X-HE-Tag: 1695015208-829108 X-HE-Meta: U2FsdGVkX19dFP4Se0EBbm2qn6adSDNgmyk9Pk7yTp70PZcQdjMD5r5l5UritH3uZ9MIyt96TiWd10O+x6eqGjYBTVmIB93bFHTRLCacK1dOsjTPUw64tQK1nVtxoEv4JTU2tjIN9yiukbPh7+8NzjUaQPocZol0+jqYK2xbyLP5m88Nj8UIzqc3JhN+1OFMlwBsHJFpbtGgtL1mw2+O5tzqn0aM+Yehm9bUlZtaMPfjtENl4xOGmP4xACEdxpkDCGwcKe7ySUx3lsVyAIf+JNv+WZ6BHT4eyVPy6uP3kMF7wYY8F0wIW1bHya/urUDeibj2zWJDph/NjISo1qtcKos/ccU5aifn31TGr8gAMyH8loYYK36SKqAULUgxSiEF648Vtge90o1/sn0n6k7J9Y9szNag7Ng8t/oFB5b306igpJeh6in8oi/A4z4D8VVAzknih1kia96t9EJ02YQ/ph15MtlNyuOLptBTRC52MhpzZBvbHepMraVl3JdTMyaNhS1eQc5YRIS+t+ePZEIRCWJ6LFy9qjyR1VcQknuGicO4kBjfpHaGYfKaq4vzFgppRwvR5JyPoCvjCLKh7c54obC+4+eO6CiT8SAjHNeOqhvLEfYM66kgqlB17P+H/IJXqHuUpi7cQX56V9Q3gjXjs+NnjUERH6QQvYfKexlrBktKWaXlvdIld7XRkfHIZskxMVNUP5lNQTno/yRMUXRD2rlYhbxExEcPwH0GZq4Z0n1CeDFwzRNV6LsrNYPoqKmt5QATc75oXDR97KIwgSyxQm1Ht5cw6j09h+Nw207IbVKAlqyj9uYjKwgphzrtzfRk0ZTSY6aGB+Egko1rg0TEbj2Xbu8SQyt5Yzxoj+/jHoiqvz/nZDli+x+2LxFeqykfXuV1mRvmnA/wzvn/yVN6BjsQGtxpYYfuGrPVhTGPpi97PK+xz0HN662R6pTeZ8h66VlAKZ485kbWt5Aq//I uiI5Xw+A vWqqwMac83qK2I7mOSuZIzuafSVIOB1XBuAo4Mue/+fTD64vX/82ojVGZOSWIe2ilLw/z9fKVz54pGKQDF/2/NDLXNQobWXqfhWFK/Nkyd40uf+tyF7b6/MDrUjdWN2T3KKhM076BED/izbSOmpnAoTGLUteXn4ylQNORzbBbJ2SpYD5z3ubOA9Rb7cY+u07JQ4QY3TEhjC5Fy7EuecFZXk85QBHWR3mVOJj04BBzyYjYO6i0nkazjfQatmrHL8dWR5XJPZ3ntkqd2cfarlB+sATEs2JG1mC2OUn9cOBhvLK4+Qm0sHm7DaaTXXDLv3/LHOPLEr8cqG0xSjX+0jukOqK6wP3CSI/je9P1 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, Thank you for this patchset! On Mon, 18 Sep 2023 13:10:43 +0800 Jinjie Ruan 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 > --- > 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