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 54D6DCD1284 for ; Tue, 9 Apr 2024 09:54:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B77876B008A; Tue, 9 Apr 2024 05:54:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B25D26B0092; Tue, 9 Apr 2024 05:54:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9EDE46B0095; Tue, 9 Apr 2024 05:54:47 -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 8149D6B008A for ; Tue, 9 Apr 2024 05:54:47 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 37FDF1C041C for ; Tue, 9 Apr 2024 09:54:47 +0000 (UTC) X-FDA: 81989534214.11.514994B Received: from invmail4.hynix.com (exvmail4.skhynix.com [166.125.252.92]) by imf30.hostedemail.com (Postfix) with ESMTP id 18F2480011 for ; Tue, 9 Apr 2024 09:54:42 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf30.hostedemail.com: domain of honggyu.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=honggyu.kim@sk.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1712656485; 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=iUS/h9oHY3Otx6pMewH2WvNF7wvsEUBSjW83cNTKr2w=; b=tni515LBeXVGFyCTnq84rZ1KauGCkMohkSsEVMpsanZX6ajTDCblgXA6lZrxfCDNvvgVua QPfqfb6Lz7b/R+tJhtsvhuxFLP2AvIusRN7bA2rL3Z3IYU9yyw3LmOUZmx+LrMbzZXr5ih JEnUl+A8NfLW7qZyE92NWPgDehUTITI= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=none; dmarc=none; spf=pass (imf30.hostedemail.com: domain of honggyu.kim@sk.com designates 166.125.252.92 as permitted sender) smtp.mailfrom=honggyu.kim@sk.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1712656485; a=rsa-sha256; cv=none; b=8o9Mufq7HUp6YDrut3n+rdTV4xYBavheXxpIPUlvgMc1ofjP6WxA+ADkcgPOoIyY/ZI9yS VG+yU0of/jBEQnCV4JjtDPxL+2Hwg725lJpbQa0pI0As2BK3XhhrG37WAM1NQZ9YnWU+mv w1TCVPqg49sL3+XuJaZmSVkOizDuJjA= X-AuditID: a67dfc5b-d6dff70000001748-ad-6615105f05cb From: Honggyu Kim To: SeongJae Park Cc: damon@lists.linux.dev, linux-mm@kvack.org, akpm@linux-foundation.org, apopple@nvidia.com, baolin.wang@linux.alibaba.com, dave.jiang@intel.com, hyeongtak.ji@sk.com, kernel_team@skhynix.com, linmiaohe@huawei.com, linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com, mhiramat@kernel.org, rakie.kim@sk.com, rostedt@goodmis.org, surenb@google.com, yangx.jy@fujitsu.com, ying.huang@intel.com, ziy@nvidia.com, 42.hyeyoo@gmail.com, art.jeongseob@gmail.com, Honggyu Kim Subject: Re: [RFC PATCH v3 5/7] mm/damon/paddr: introduce DAMOS_MIGRATE_COLD action for demotion Date: Tue, 9 Apr 2024 18:54:14 +0900 Message-ID: <20240409095418.3051-1-honggyu.kim@sk.com> X-Mailer: git-send-email 2.43.0.windows.1 In-Reply-To: <20240408175228.91414-1-sj@kernel.org> References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrEIsWRmVeSWpSXmKPExsXC9ZZnkW6CgGiawduDzBYTewws5qxfw2ax 60aIxf0Hr9kt/u89xmjx5P9vVosTNxvZLDq/L2WxuLxrDpvFvTX/WS2OrD/LYrH57Blmi8XL 1Sz2dTxgsjj89Q2TxeRLC9gsXkw5w2hxctZkFovZR++xOwh7LD39hs1jQxOQ2DnrLrtHy75b 7B4LNpV6tBx5y+qxeM9LJo9NqzrZPDZ9msTucWLGbxaPnQ8tPV5snsno0dv8js3j8ya5AL4o LpuU1JzMstQifbsEroz2iY8ZC07pVux4/J+9gfGeUhcjB4eEgInEpW6hLkZOMHPOo9csIDab gJrElZeTmEBsEQFFiXOPL7J2MXJxMAvsYZHoOvWGHSQhLJAg0XbjIyvIHBYBVYn7CxxBwrwC ZhJ/1u9kg5ipKfF4+0+wck4BY4lHUxcyg9hCAjwSrzbsZ4SoF5Q4OfMJ2F5mAXmJ5q2zmSF6 T7FLrLvoBGFLShxccYNlAiP/LCQts5C0LGBkWsUolJlXlpuYmWOil1GZl1mhl5yfu4kRGHPL av9E72D8dCH4EKMAB6MSD6/FVeE0IdbEsuLK3EOMEhzMSiK8waaCaUK8KYmVValF+fFFpTmp xYcYpTlYlMR5jb6VpwgJpCeWpGanphakFsFkmTg4pRoY193UT1c90T/j16fYV0kvPO681gpk 4pm8U1SLUTlrh6JHlnvszqylgQwJTDf2ZJU/TG/J/ye52It1r0nhi7ty2XbH/C9LTPY5Erew jFlgU/XRdLfW2b139v7PflXV9t3lasnBV0nCRyceF7/8+MuEpfqJTfce8Ec81Zm92ukBf2zw oicvxcsmKrEUZyQaajEXFScCAM4Xchi1AgAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprHIsWRmVeSWpSXmKPExsXCNUNLTzdeQDTN4MkyPYuJPQYWc9avYbPY dSPE4v6D1+wW//ceY7R48v83q8WJm41sFp+fvWa26HzyndHi8NyTrBad35eyWFzeNYfN4t6a /6wWR9afZbHYfPYMs8Xi5WoWh649Z7XY1/GAyeLw1zdMFpMvLWCzeDHlDKPFyVmTWSxmH73H 7iDusfT0GzaPDU1AYuesu+weLftusXss2FTq0XLkLavH4j0vmTw2repk89j0aRK7x4kZv1k8 dj609HixeSajR2/zOzaPb7c9PBa/+MDk8XmTXIBAFJdNSmpOZllqkb5dAldG+8THjAWndCt2 PP7P3sB4T6mLkZNDQsBEYs6j1ywgNpuAmsSVl5OYQGwRAUWJc48vsnYxcnEwC+xhkeg69YYd JCEskCDRduMjUIKDg0VAVeL+AkeQMK+AmcSf9TvZIGZqSjze/hOsnFPAWOLR1IXMILaQAI/E qw37GSHqBSVOznwCtpdZQF6ieets5gmMPLOQpGYhSS1gZFrFKJKZV5abmJljqlecnVGZl1mh l5yfu4kRGGvLav9M3MH45bL7IUYBDkYlHl6HO8JpQqyJZcWVuYcYJTiYlUR4g00F04R4UxIr q1KL8uOLSnNSiw8xSnOwKInzeoWnJggJpCeWpGanphakFsFkmTg4pRoY7b/XJ9fJXJ6y+eL5 sGA2n38FZwT7xU5O+vbgTtuh3tqd03SOXNE/d7/t8zRBiZ62A/7tM8yOXZG6aDfl8IS2t/kq JQkL9766uPhTmMcES7G1LFNbp4vcc3vnx9n56lh5YW2Go7Dh7aeu5lZ1Z/vupD/wU1eRY039 c9ppm2bgjRX7Ly+z4Sl/psRSnJFoqMVcVJwIAAtzjpaxAgAA X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: 18F2480011 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: kbgnbu1nq7jnbcza8bmk975tpntesrro X-HE-Tag: 1712656482-204711 X-HE-Meta: U2FsdGVkX19WT+5h0dYXn7Q7DC2a8ZizLDB9glHDU1D2BVQF6O8L3n38J4lFMfiW1hzKtkF3v/u9A0UziGr74IYzp8Nzrx9ZffQToCIZuyCPzWbcqY95F6djBzhaXPzyQ2lnXhgt63l5Zr/108T2ieK6Bluc8ZyR7xkr49lSEduwDKN/ZFqcCZ/ozt+Chw/uYh7BGJjLDIk8OruaU4nxcaVSuWlgEryLUgQhBSHf22ymKy+gUUjFovl4IzsuNQOpww2141vOSbZtjG6LRaff5+gHnyPqZiHsRJRKsGPMlkssSXIbFDnmvaa2lkpC64eEPziouDx04E4LDIUYx3M3sa6BXeEfeZoSxUWbMsHq/4cAK1TAZ3h1FXpt2D7yc/YInHnkaddteUdaJrZvhpspNLXwbZMJZ2yuZyJmWh1cPpKdjAVzvHoQkLARGboN94qhVXwTiV2g2aSPFqjPguag1JztqnQdChLk8EDLGFLVse4zWSdvPyqPcxByIIDm1dCKK3GXFmCK/1u4Z88s2ub7TqO0J2unm/bWgqOAdDH/Re/bdcjvlgegQp0/5kmIIqu4Ewk+rMScg5KNWUcc0HQWJ4CQjqohZoXUpbdlBxdEFs+6QMEQnYPT749HFhlKmSrgsLrm7rP2BYyMx8Q9rDtblQ4pa8S5JI+eBbjKsQVhrhQhFCCcd9HuTfZof+Ncav1dV8k4tWmYlDBg9f1lMf44DhuVdBqqxFNbL8boWa3qIEdsnJqptaAH/9tvwAQ6Xfx8VB2pLcxSOuYn2hPU+lKhH418kBwP+1doDSkIV1c3koMUeAUJceenAl930zpjdJ1Dp3SToQEk7ChKPfGN49EDEqdS9czANRbbpQ4e9yrEJZVsFl+47mww/xhiRJcusGtB8N2sxaWfoTcUA6s+PZrFHmFg3Oi9hRmcu9FsZo2hSlgfQRiaH2V27P4IHUNpK4u8yqoUk1CMpFZ2eN5rCrI QUUvxQ70 3b0XEEygfHeNbfy7kfyGv07iLM1NU/GYFJGJBXLgpM5gl2eo7lzEwcbhnBqL38SWRYJOqCV5F/MBknWeOPTdAcmHoN4q5XuidEvF1dugIpZgXv8bfCRp6F/yn5A/BcqGEf8xgkg346FHS0pAegFHvDgPSLtRpUTxGFWWQp/EJ9vhQb+hkQ6Y1fEdWeES2Z3FnzfQoM3k7imyFv+p/9dY+XKHw/zf1v027fSZWJY5AHPJcrIA= 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, On Mon, 8 Apr 2024 10:52:28 -0700 SeongJae Park wrote: > On Mon, 8 Apr 2024 21:06:44 +0900 Honggyu Kim wrote: > > > On Fri, 5 Apr 2024 12:24:30 -0700 SeongJae Park wrote: > > > On Fri, 5 Apr 2024 15:08:54 +0900 Honggyu Kim wrote: > [...] > > > > Here is one of the example usage of this 'migrate_cold' action. > > > > > > > > $ cd /sys/kernel/mm/damon/admin/kdamonds/ > > > > $ cat contexts//schemes//action > > > > migrate_cold > > > > $ echo 2 > contexts//schemes//target_nid > > > > $ echo commit > state > > > > $ numactl -p 0 ./hot_cold 500M 600M & > > > > $ numastat -c -p hot_cold > > > > > > > > Per-node process memory usage (in MBs) > > > > PID Node 0 Node 1 Node 2 Total > > > > -------------- ------ ------ ------ ----- > > > > 701 (hot_cold) 501 0 601 1101 > > > > > > > > Since there are some common routines with pageout, many functions have > > > > similar logics between pageout and migrate cold. > > > > > > > > damon_pa_migrate_folio_list() is a minimized version of > > > > shrink_folio_list(), but it's minified only for demotion. > > > > > > MIGRATE_COLD is not only for demotion, right? I think the last two words are > > > better to be removed for reducing unnecessary confuses. > > > > You mean the last two sentences? I will remove them if you feel it's > > confusing. > > Yes. My real intended suggestion was 's/only for demotion/only for > migration/', but entirely removing the sentences is also ok for me. Ack. > > > > > > > > > > Signed-off-by: Honggyu Kim > > > > Signed-off-by: Hyeongtak Ji > > > > --- > > > > include/linux/damon.h | 2 + > > > > mm/damon/paddr.c | 146 ++++++++++++++++++++++++++++++++++++++- > > > > mm/damon/sysfs-schemes.c | 4 ++ > > > > 3 files changed, 151 insertions(+), 1 deletion(-) > [...] > > > > --- a/mm/damon/paddr.c > > > > +++ b/mm/damon/paddr.c > [...] > > > > +{ > > > > + unsigned int nr_succeeded; > > > > + nodemask_t allowed_mask = NODE_MASK_NONE; > > > > + > > > > > > I personally prefer not having empty lines in the middle of variable > > > declarations/definitions. Could we remove this empty line? > > > > I can remove it, but I would like to have more discussion about this > > issue. The current implementation allows only a single migration > > target with "target_nid", but users might want to provide fall back > > migration target nids. > > > > For example, if more than two CXL nodes exist in the system, users might > > want to migrate cold pages to any CXL nodes. In such cases, we might > > have to make "target_nid" accept comma separated node IDs. nodemask can > > be better but we should provide a way to change the scanning order. > > > > I would like to hear how you think about this. > > Good point. I think we could later extend the sysfs file to receive the > comma-separated numbers, or even mask. For simplicity, adding sysfs files > dedicated for the different format of inputs could also be an option (e.g., > target_nids_list, target_nids_mask). But starting from this single node as is > now looks ok to me. If you think we can start from a single node, then I will keep it as is. But are you okay if I change the same 'target_nid' to accept comma-separated numbers later? Or do you want to introduce another knob such as 'target_nids_list'? What about rename 'target_nid' to 'target_nids' at the first place? > [...] > > > > + /* 'folio_list' is always empty here */ > > > > + > > > > + /* Migrate folios selected for migration */ > > > > + nr_migrated += migrate_folio_list(&migrate_folios, pgdat, target_nid); > > > > + /* Folios that could not be migrated are still in @migrate_folios */ > > > > + if (!list_empty(&migrate_folios)) { > > > > + /* Folios which weren't migrated go back on @folio_list */ > > > > + list_splice_init(&migrate_folios, folio_list); > > > > + } > > > > > > Let's not use braces for single statement > > > (https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces). > > > > Hmm.. I know the convention but left it as is because of the comment. > > If I remove the braces, it would have a weird alignment for the two > > lines for comment and statement lines. > > I don't really hate such alignment. But if you don't like it, how about moving > the comment out of the if statement? Having one comment for one-line if > statement looks not bad to me. Ack. I will manage this in the next revision. > > > > > > + > > > > + try_to_unmap_flush(); > > > > + > > > > + list_splice(&ret_folios, folio_list); > > > > > > Can't we move remaining folios in migrate_folios to ret_folios at once? > > > > I will see if it's possible. > > Thank you. Not a strict request, though. > > [...] > > > > + nid = folio_nid(lru_to_folio(folio_list)); > > > > + do { > > > > + struct folio *folio = lru_to_folio(folio_list); > > > > + > > > > + if (nid == folio_nid(folio)) { > > > > + folio_clear_active(folio); > > > > > > I think this was necessary for demotion, but now this should be removed since > > > this function is no more for demotion but for migrating random pages, right? > > > > Yeah, it can be removed because we do migration instead of demotion, > > but I need to make sure if it doesn't change the performance evaluation > > results. > > Yes, please ensure the test results are valid :) Sure. Thanks for your review in details! Please note that I will be out of office this week so won't be able to answer quickly. Thanks, Honggyu > > Thanks, > SJ > > [...] >