Received: by 2002:ab2:3350:0:b0:1f4:6588:b3a7 with SMTP id o16csp1533876lqe; Mon, 8 Apr 2024 11:37:33 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCUZR17URBuh+PAO7rFjpfSMMz8BH85wPrgJEgPJvGtBAA/803lGPGH6Ll2MqRtITi2askHE3KKNup1Wn9ezYAOPb9blrukOKyjgCy2hug== X-Google-Smtp-Source: AGHT+IECHETK5pDFXrnHRDfesEzbVWCD16Sxf/Mi43lCSE9fEuko2mwfeiaAOxrz3HkFQ701uzjF X-Received: by 2002:a17:90a:e00c:b0:2a2:414b:ff78 with SMTP id u12-20020a17090ae00c00b002a2414bff78mr6847357pjy.44.1712601453670; Mon, 08 Apr 2024 11:37:33 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1712601453; cv=pass; d=google.com; s=arc-20160816; b=KqclavIAMH8KRXduWj22ufaY3CQU7XgUoBMFJCVa4UvhNStePbDedA6SgEEXJHxKUI dXWgiZQTQRBsS1wSYb73b8S3OhrgI5rziUSHt8sDNdtb1Gsy6LKHPwOZekhMgVzs09UO 2fRjYg92N4EENIZHSccTfuAHL5A/wCXGaxcD+PX0JjViGx9uJNADggn/CsrNtDx1//ST IF9dReh8tKSXZig9oTrdUHPv76gkR+pOITQfq+mVRVGyMIGVVAy+UGKik/pHjx3EdYL9 wCJi4YZ2eHDO73acDocYCdRL0unez3PZaJWzfz0R7fAkCjyVAK3uH4DetCmymMXdQNiS xrrA== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature; bh=VA3WcqXEmFJgqI9n09S3GwkiUE14Z8sZtwntCeAhbvE=; fh=vthu0SYzpky+WzEpp04LHS7M01Tv3cuywMDMy+rfr78=; b=fLNJjYO2f427lj9zqwIL1l2ngVVVZ/r46x0VNgYWAJJaIPLa7w2630ESDHscnNSv3A oVqt0xM6Jt6hWPuswnSAqYMUiVmKBlzOcqlGHqrJlr2Ub3Zmsm/OyjbRzceaGVQi+kZd ZpUyDjgB+ydUutMCWeXSahNtYRG5sOwp0imNac0B3X05dpd1kcAaaqDyUlBe/8/OFDl1 vZEsUsvbCaSrTVuyCid97wcIcOSNv4wN7d5xfTza8faW0EEz2athQ84sWGOY0IhAOcm1 bg/Z61rXbRk4guMeHOD+N6nBg6RfpEBsLXgQ+S9yF7j2lzIwx51qWF24pqs+7bhnXDN3 tV3Q==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=dETVU4z5; arc=pass (i=1 dkim=pass dkdomain=kernel.org); spf=pass (google.com: domain of linux-kernel+bounces-135785-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) smtp.mailfrom="linux-kernel+bounces-135785-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from sy.mirrors.kernel.org (sy.mirrors.kernel.org. [147.75.48.161]) by mx.google.com with ESMTPS id gb21-20020a17090b061500b002a00029047dsi8880137pjb.98.2024.04.08.11.37.33 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Apr 2024 11:37:33 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-135785-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=dETVU4z5; arc=pass (i=1 dkim=pass dkdomain=kernel.org); spf=pass (google.com: domain of linux-kernel+bounces-135785-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) smtp.mailfrom="linux-kernel+bounces-135785-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sy.mirrors.kernel.org (Postfix) with ESMTPS id 22391B2B59A for ; Mon, 8 Apr 2024 17:54:33 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 399A0145B0B; Mon, 8 Apr 2024 17:52:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="dETVU4z5" Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 279EF1E489; Mon, 8 Apr 2024 17:52:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712598752; cv=none; b=KDOa9hUn0IvIqwGTq1H9s4it83fdKd+BbEZJAKXlXAy9xfWD531tuB7ZQeODhyFO4876MfY0cGZyBcpxFwewJoIjJSiRnV1Ns9ojrKR9nC/uWtGJLkEwDapJwlVpFPH2L6053WwTDblutAM4RkL1UccQ+xCf9uvjt62MvDI7gSI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712598752; c=relaxed/simple; bh=lqMYSwhK0qnhw09eJ1+Ailh7I3TSY0ZybkqE6/ufJ1c=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=B7uK9PxtrRUT9Wn28eO99LDoBx7SjBMH6h0NyWpORG/OyatYQpiYUxQcH9MWA7xVamNKvUZdiM5YzPv6344HO75GPeldaEnEbDkHdNpmVv/wDelg6psExKGH9wiQdnt2G1+NuYGrptF2IAhEifW1Fe0ZvYRY0O6QxBAZpYIuiwo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=dETVU4z5; arc=none smtp.client-ip=10.30.226.201 Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0A4BDC433F1; Mon, 8 Apr 2024 17:52:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1712598751; bh=lqMYSwhK0qnhw09eJ1+Ailh7I3TSY0ZybkqE6/ufJ1c=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=dETVU4z5saCmnOtTCAvVfwcmZVyqiTbnvhnbyUHMbHRPFAUROww0oKuWoO6NtGdHS 5f2+06+UOqSYXziqPLu06ZUoMs6i8czPHx+n8SKi1degY9Q7ekGgozVIJs3ohkrJ6u eVb082wPFbA236byh5LRflA46GI8PnahThrnYv9U7l2cgsiVcnAy1LJgzLDiuXHsXB /y1q/lwED31nG8qIWpqvtS3jtO4jwWtmHMTk7RdZ3ieR/Pz/vgfiImOrVtvDHFhP6e j2ZLcku9F1MTc7OtG9myVWttU/0DWkGZAQQle8+TV5q+4FHVQnvvFG0XsVG4MRycMn 0K8vJS4x5hJxg== From: SeongJae Park To: Honggyu Kim Cc: SeongJae Park , 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 Subject: Re: [RFC PATCH v3 5/7] mm/damon/paddr: introduce DAMOS_MIGRATE_COLD action for demotion Date: Mon, 8 Apr 2024 10:52:28 -0700 Message-Id: <20240408175228.91414-1-sj@kernel.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240408120648.2947-1-honggyu.kim@sk.com> References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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. > > > > > > > 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. [...] > > > + /* '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. > > > > + > > > + 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 :) Thanks, SJ [...]