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 47724C83030 for ; Thu, 3 Jul 2025 05:18:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7E45A6B00E8; Thu, 3 Jul 2025 01:18:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 76DDC6B00E9; Thu, 3 Jul 2025 01:18:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6AADB6B00EA; Thu, 3 Jul 2025 01:18:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 5C7ED6B00E8 for ; Thu, 3 Jul 2025 01:18:44 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 12C0B107321 for ; Thu, 3 Jul 2025 05:18:44 +0000 (UTC) X-FDA: 83621798568.19.B73C0EA Received: from invmail4.hynix.com (exvmail4.hynix.com [166.125.252.92]) by imf10.hostedemail.com (Postfix) with ESMTP id 6E420C0006 for ; Thu, 3 Jul 2025 05:18:41 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; spf=pass (imf10.hostedemail.com: domain of yunjeong.mun@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=yunjeong.mun@sk.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1751519922; a=rsa-sha256; cv=none; b=s4VtziufYvfzM77cVvvDR7atfq+kG+qyphHsx7MiO+5BfQMC4vPpjXp5U9FUqNS92eAyDo OYsdncgThgC7Mdv6kM32/kFsvtpT5uaVw2/OEtP1oHkz38wYmCuHeR7hQnsYNTzykvev0Y WUrit0UxmO1dAz+p89/VICQR1UfoEtU= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf10.hostedemail.com: domain of yunjeong.mun@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=yunjeong.mun@sk.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1751519922; 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; bh=92rfOcNcYP7tY1UKKkDd6o/ggj1T6iO86gOxJzuXopU=; b=jiXt/jPhUVj1fqQk+PNazaWGYkS2KzqphyMFd8yD4SLH9cRDpNS+OIu46tjuwjdh032iUx FGZqJdXx73IibIzQffhBdquu8NYhSPPcVzFp6OdoAs6BAX01YE4BnCb0xYTRlZXK58Z/9A 339y0JyEv4F34JlTOaTX1V21ac360q4= X-AuditID: a67dfc5b-681ff7000002311f-e9-686612ae7f4d From: Yunjeong Mun To: sj@kernel.org Cc: akpm@linux-foundation.org, damon@lists.linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel_team@skhynix.com, honggyu.kim@sk.com Subject: Re: [RFC PATCH 1/2] samples/damon: convert node id to physical address Date: Thu, 3 Jul 2025 14:18:33 +0900 Message-ID: <20250703051836.1759-1-yunjeong.mun@sk.com> X-Mailer: git-send-email 2.48.1.windows.1 In-Reply-To: <20250701235407.57420-1-sj@kernel.org> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrGLMWRmVeSWpSXmKPExsXC9ZZnke46obQMg91vTS3mrF/DZvHk/29W i8u75rBZ3Fvzn9Xi8Nc3TA6sHptWdbJ5bPo0id3jxIzfLB4vNs9k9Pi8SS6ANYrLJiU1J7Ms tUjfLoErY8OMZ6wFM9QrfrxfydzAOF++i5GTQ0LARGLL2VssMHbDqWeMIDabgIbEwUMnmUFs EQFBif7HM1i7GLk4mAXmMkocOfMerEhYwF/i/aNzYM0sAqoSWz/cBLI5OHgFzCV+v3KFmKkp 0XDpHhOIzSlgLDHzyQuwViEBHolXG/aD2bxA80/OfAI2hllAXqJ562xmkF0SAjPYJI58O8YE MUhS4uCKGywTGPlnIemZhaRnASPTKkahzLyy3MTMHBO9jMq8zAq95PzcTYzA0FxW+yd6B+On C8GHGAU4GJV4eB3kUzOEWBPLiitzDzFKcDArifDyySZnCPGmJFZWpRblxxeV5qQWH2KU5mBR Euc1+laeIiSQnliSmp2aWpBaBJNl4uCUamA069+3/7wkO+PdKd+tZnyQOPT5v+blS33zXijZ 7OJ/OSljmum1JTpTqsWrsxQ/Oz+ZVvkj1u3A89fsjXP+bV2298XqPkcpARWHsI/BkX/qfx+I PVX46+2qNqEzlX5Sc8wuXuPSmnB2395Wzd3Xr/FEi+19Ymq0fFNUnWak1LbZO09ILJpt7xKb ocRSnJFoqMVcVJwIACjIEGdJAgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrGLMWRmVeSWpSXmKPExsXCNUNWR3edUFqGwd6bWhZz1q9hs3jy/zer xednr5ktDs89yWpxedccNot7a/6zWhz++obJgd1j06pONo9Nnyaxe5yY8ZvF48XmmYwe3257 eCx+8YHJ4/MmuQD2KC6blNSczLLUIn27BK6MDTOesRbMUK/48X4lcwPjfPkuRk4OCQETiYZT zxhBbDYBDYmDh04yg9giAoIS/Y9nsHYxcnEwC8xllDhy5j1YkbCAv8T7R+dYQGwWAVWJrR9u AtkcHLwC5hK/X7lCzNSUaLh0jwnE5hQwlpj55AVYq5AAj8SrDfvBbF6g+SdnPgEbwywgL9G8 dTbzBEaeWUhSs5CkFjAyrWIUycwry03MzDHVK87OqMzLrNBLzs/dxAgMvWW1fybuYPxy2f0Q owAHoxIPr4N8aoYQa2JZcWXuIUYJDmYlEV4+2eQMId6UxMqq1KL8+KLSnNTiQ4zSHCxK4rxe 4akJQgLpiSWp2ampBalFMFkmDk6pBkazVB4ZrlC3w16tDlGfMiQyD8dOq8q65WMvadtVVnrk WuJsDv19HssvG4lfjE+M/18jNHfiMk/nd0I7f2cZFzUf285mxm5WecA4p2HOFn035R1HstbK Llz9X5c7xDeDbY4V//mX+/Y124i+15CfP89sW/mtZ8Gruho387DukWbuqzXR0vX3V2Ipzkg0 1GIuKk4EAK+2G/o5AgAA X-CFilter-Loop: Reflected X-Stat-Signature: matktyy9u6a8tmcdeb7w3rpr4jpc9xfb X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: 6E420C0006 X-Rspam-User: X-HE-Tag: 1751519921-765843 X-HE-Meta: U2FsdGVkX18MFlYjOduAYtV2DtJcpK8EEhG1pFzVKOR9vuPJVxS6/OZLV4cRjK1f7lAZouds8mligOTwZFZc2kQvtu7o4xEF6F0vykI4ZqB7AN3SvQQvC95t+TKxHIDqjy7gOEIBJg2QhhwnOwCP2cxFwAHN1OGI7P0VqU22UDmuQS/mtR3KTDr/zxYxmrdEpHFy2QQDYIRyAZRW73tvqcCy3URd8Fe+laK2e7zdYLYeWQVhd/bS9P28bTyogD3QOodqB+75yrGhekBawG+yXmFnAy8EbuMCuO+g8xIDBfTL9Pzv1522Cr1JwXtDCZLBALU4iNR4UMTECB2E1VFOhT9sXiA6XeTfBE6Ts9Y8LMvWjTuRIiFrw4A4/syQItKIevTfLAW2g6BZDmPumBuu5uW7Ge5EtrP482ZRGI4qEu4832uKzzQAqPT7/kwtyuF+l87Vn5d1M/ClQx56OyvXWwCC2H/IlUmtLIqPY9p3BRznJPv2smiTE28acupBVKYOKp2W4JCa/urS4L99Nw9XqIq47qqFoEBTO3XsfEABkFXREt7AEcSegSpvDULrJNRDlHMjL/4i99CYp+Zxxb8vL78yJp+j9e9FpivyHNWeHUr16tF0QXkA7nW5aUvafgQc+ljOwOSutsoMDkTCIx6ZtITfLKdInO8Hiwlwab9Bnf6srVJrh/3RzCksVUzOEuQEc3cGRvzMQG1d+3AHhu9qZiY82zCPYkcrTF6bLT8TTctwtTBJx3UwYiyh8NbJ17/S8ifns3u1IPvpzG93CjyC2IpJjSH4qCHVnkC8VhaJXpmK8xYJAksxtx/osTJ2YZSiAhJsL/bKNasMQKyxRwKOf+1nqyof/PUjoqQ43473JSqSSTQyzXLr1Fu504tb93odwjq5oAJpV33yPJpMdGMAjy38/LWg5fYtHoZDl2JC0+/thOvlR6dxYzY//YEgbscPGoBfIWqzXZn4AWwchkm Yi/DaNzT 5FMxVFzPGnEvV1lHHwuaThEg4XZKETpzpLZMjkTs6NaZ2rtfLBpRa4OaZCMCS5zShHEzS7J4KcbdHbZFIO06vCZ5XPA== 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: List-Subscribe: List-Unsubscribe: Hi Seongjae, thanks for your review :) On Tue, 1 Jul 2025 16:54:07 -0700 SeongJae Park wrote: > Hi Yunjeong, > > On Tue, 1 Jul 2025 17:54:16 +0900 Yunjeong Mun wrote: > > > This patch removes the `node#_start_addr` and `node#_end_addr` knobs, > > and introduces logic for converting numa node id to physical address. > > It only checks whether a numa node is online and calculates the > > start and end addresses of the node. It does not verify whether each > > memory block within the numa node is `usable` or part of `System RAM`, > > as performed by `damo` [1],[2]. > > This is just a sample module, but I'd like to avoid making unnecessary > user-breaking changes. How about keeping the existing knobs but adding yet > another knob for the automatic detection, say, 'detect_node_addresses'? > I agree. From my understanding, 'detect_node_addresses' can be set to either 'Y' or 'N'. If it is set to 'Y', mtier converts node0 and node1 to their physical addresses internally. If it is set to 'N', it uses the existing knobs. Is that correct? > > > > [1] > > https://github.com/damonitor/damo/blob/v2.8.5/src/damo_pa_layout.py#L72-L90 > > [2] > > https://github.com/damonitor/damo/blob/v2.8.5/src/damo_pa_layout.py#L92-L10 > > > > Suggested-by: Honggyu Kim > > Signed-off-by: Yunjeong Mun > > --- > > samples/damon/mtier.c | 44 ++++++++++++++++++++++++++++--------------- > > 1 file changed, 29 insertions(+), 15 deletions(-) > > > > diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c > > index f3220d6e6739..ba6938a89c21 100644 > > --- a/samples/damon/mtier.c > > +++ b/samples/damon/mtier.c > > @@ -12,18 +12,6 @@ > > #include > > #include > > > > -static unsigned long node0_start_addr __read_mostly; > > -module_param(node0_start_addr, ulong, 0600); > > - > > -static unsigned long node0_end_addr __read_mostly; > > -module_param(node0_end_addr, ulong, 0600); > > - > > -static unsigned long node1_start_addr __read_mostly; > > -module_param(node1_start_addr, ulong, 0600); > > - > > -static unsigned long node1_end_addr __read_mostly; > > -module_param(node1_end_addr, ulong, 0600); > > - > > static unsigned long node0_mem_used_bp __read_mostly = 9970; > > module_param(node0_mem_used_bp, ulong, 0600); > > > > @@ -44,6 +32,28 @@ MODULE_PARM_DESC(enable, "Enable of disable DAMON_SAMPLE_MTIER"); > > > > static struct damon_ctx *ctxs[2]; > > > > +struct region_range { > > + phys_addr_t start; > > + phys_addr_t end; > > +}; > > + > > +static int numa_info_init(int target_node, struct region_range *range) { > > + > > checkpatch.pl complaints as below. > > ERROR: open brace '{' following function definitions go on the next line > #82: FILE: samples/damon/mtier.c:40: > +static int numa_info_init(int target_node, struct region_range *range) { > > > + if (!node_online(target_node)) { > > + pr_err("NUMA node %d is not online\n", target_node); > > + return -EINVAL; > > + } > > + > > + /* TODO: Do we need to support more accurate region range? */ > > + unsigned long start_pfn = node_start_pfn(target_node); > > + unsigned long end_pfn = node_end_pfn(target_node); > > + > > + range->start = (phys_addr_t)start_pfn << PAGE_SHIFT; > > + range->end = (phys_addr_t)end_pfn << PAGE_SHIFT; > I'll fix them in the next version. > Let's use PHYS_PFN() instead. Okay, I'm going to change it with PFN_PHYS() > > + > > + return 0; > > +} > > + > > static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote) > > { > > struct damon_ctx *ctx; > > @@ -53,6 +63,7 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote) > > struct damos *scheme; > > struct damos_quota_goal *quota_goal; > > struct damos_filter *filter; > > + struct region_range addr; > > > > ctx = damon_new_ctx(); > > if (!ctx) > > @@ -82,9 +93,12 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote) > > if (!target) > > goto free_out; > > damon_add_target(ctx, target); > > - region = damon_new_region( > > - promote ? node1_start_addr : node0_start_addr, > > - promote ? node1_end_addr : node0_end_addr); > > + > > + int ret = promote ? numa_info_init(1, &addr) : numa_info_init(0, &addr); > > + if (ret) > > + goto free_out; > > Yet another checkpatch.pl complain. > > WARNING: Missing a blank line after declarations > #119: FILE: samples/damon/mtier.c:98: > + int ret = promote ? numa_info_init(1, &addr) : numa_info_init(0, &addr); > + if (ret) > > > + > > + region = damon_new_region(addr.start, addr.end); > > if (!region) > > goto free_out; > > damon_add_region(region, target); > > -- > > 2.34.1 > > Thanks for your detailed reviw! Next time, I will make sure to check checkpatch.pl:) Best Regards, Yunjeong Mun > Thanks, > SJ