[PATCH 0/2] add a knob to control whether to use other nodes at the same tier of the target node in DAMON

[PATCH 0/2] add a knob to control whether to use other nodes at the same tier of the target node in DAMON

[wangchuanguo]

In DAMON's migrate_hot and migrate_cold features, the code was
intended to migrate pages ​​only to the node specified by target_nid​​.
However, during testing, it was observed that memory allocation
and migration could occur on ​​any nodes​​, which is a BUG.
The first patch in this PR fix this issue.

A use_nodes_of_tier file has been added under the directory /sys/kernel/mm/damon/admin/kdamonds/<N>/contexts/<N>/schemes/<N>/
to control whether to ​​use other nodes in the same tier as
the target node​​ for migration.

wangchuanguo (2):
  mm: migrate: restore the nmask after successfully allocating on the 
    target node
  mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

 include/linux/damon.h        |  9 ++++++++-
 include/linux/memory-tiers.h |  5 +++++
 mm/damon/core.c              |  6 ++++--
 mm/damon/lru_sort.c          |  3 ++-
 mm/damon/paddr.c             | 19 ++++++++++++-------
 mm/damon/reclaim.c           |  3 ++-
 mm/damon/sysfs-schemes.c     | 31 ++++++++++++++++++++++++++++++-
 mm/memory-tiers.c            | 13 +++++++++++++
 mm/vmscan.c                  |  2 +-
 samples/damon/mtier.c        |  3 ++-
 samples/damon/prcl.c         |  3 ++-
 11 files changed, 81 insertions(+), 16 deletions(-)

-- 
2.39.3

[PATCH 1/2] mm: migrate: restore the nmask after successfully allocating on the target node

[wangchuanguo]

If memory is successfully allocated on the target node and the
function directly returns without value restore for nmask,
non-first migration operations in migrate_pages() by again label
may ignore the nmask settings, thereby allowing new memory
allocations for migration on any node.

Signed-off-by: wangchuanguo <wangchuanguo@inspur.com>
---
 mm/vmscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index f8dfd2864bbf..e13f17244279 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1035,11 +1035,11 @@ struct folio *alloc_migrate_folio(struct folio *src, unsigned long private)
 	mtc->nmask = NULL;
 	mtc->gfp_mask |= __GFP_THISNODE;
 	dst = alloc_migration_target(src, (unsigned long)mtc);
+	mtc->nmask = allowed_mask;
 	if (dst)
 		return dst;
 
 	mtc->gfp_mask &= ~__GFP_THISNODE;
-	mtc->nmask = allowed_mask;
 
 	return alloc_migration_target(src, (unsigned long)mtc);
 }
-- 
2.39.3

Re: [PATCH 1/2] mm: migrate: restore the nmask after successfully allocating on the target node

[SeongJae Park]

+ Jagdish, since seems the behavior that this patch tries to change is
apparently made by Jagdish's commit 320080272892 ("mm/demotion: demote pages
according to allocation fallback order").

On Wed, 28 May 2025 19:10:37 +0800 wangchuanguo <wangchuanguo@inspur.com> wrote:

> If memory is successfully allocated on the target node and the
> function directly returns without value restore for nmask,
> non-first migration operations in migrate_pages() by again label
> may ignore the nmask settings,

Nice finding!

> thereby allowing new memory
> allocations for migration on any node.

But, isn't the consequence of this behavior is the opposite?  That is, I think
this behavior restricts to use only the specified node (mtc->nid) in the case,
ignoring more allowed fallback nodes (mtc->nmask)?

Anyway, to me, this seems not an intended behavior but a bug.  Cc-ing Jagdish,
who authored the commit 320080272892 ("mm/demotion: demote pages according to
allocation fallback order"), which apparently made this behavior initially,
though, since I may misreading the original author's intention.

> 
> Signed-off-by: wangchuanguo <wangchuanguo@inspur.com>
> ---
>  mm/vmscan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f8dfd2864bbf..e13f17244279 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1035,11 +1035,11 @@ struct folio *alloc_migrate_folio(struct folio *src, unsigned long private)
>  	mtc->nmask = NULL;
>  	mtc->gfp_mask |= __GFP_THISNODE;
>  	dst = alloc_migration_target(src, (unsigned long)mtc);
> +	mtc->nmask = allowed_mask;
>  	if (dst)
>  		return dst;

Restoring ->nmask looks right behavior to me.  But, if so, shouldn't we also
restore ->gfp_mask?

>  
>  	mtc->gfp_mask &= ~__GFP_THISNODE;
> -	mtc->nmask = allowed_mask;
>  
>  	return alloc_migration_target(src, (unsigned long)mtc);
>  }
> -- 
> 2.39.3


Thanks,
SJ

[PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[wangchuanguo]

This patch adds use_nodes_of_tier under
  /sys/kernel/mm/damon/admin/kdamonds/<N>/contexts/<N>/schemes/<N>/

The 'use_nodes_of_tier' can be used to select nodes within the same memory
tier of target_nid for DAMOS actions such as DAMOS_MIGRATE_{HOT,COLD}.

Signed-off-by: wangchuanguo <wangchuanguo@inspur.com>
---
 include/linux/damon.h        |  9 ++++++++-
 include/linux/memory-tiers.h |  5 +++++
 mm/damon/core.c              |  6 ++++--
 mm/damon/lru_sort.c          |  3 ++-
 mm/damon/paddr.c             | 19 ++++++++++++-------
 mm/damon/reclaim.c           |  3 ++-
 mm/damon/sysfs-schemes.c     | 31 ++++++++++++++++++++++++++++++-
 mm/memory-tiers.c            | 13 +++++++++++++
 samples/damon/mtier.c        |  3 ++-
 samples/damon/prcl.c         |  3 ++-
 10 files changed, 80 insertions(+), 15 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index a4011726cb3b..05eae7fd66ad 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -455,6 +455,7 @@ struct damos_access_pattern {
  * @quota:		Control the aggressiveness of this scheme.
  * @wmarks:		Watermarks for automated (in)activation of this scheme.
  * @target_nid:		Destination node if @action is "migrate_{hot,cold}".
+ * @use_nodes_of_tier:	Whether to use nodes of the target tier.
  * @filters:		Additional set of &struct damos_filter for &action.
  * @ops_filters:	ops layer handling &struct damos_filter objects list.
  * @last_applied:	Last @action applied ops-managing entity.
@@ -476,6 +477,10 @@ struct damos_access_pattern {
  * migrate_cold actions, which means it's only meaningful when @action is either
  * "migrate_hot" or "migrate_cold".
  *
+ * @use_nodes_of_tier is used to select nodes of the target tier for migrating
+ * when target_nid is out of memory.Similar to @target_nid, this parameter is
+ * only meaningful when @action is either 'migrate_hot' or 'migrate_cold'.
+ *
  * Before applying the &action to a memory region, &struct damon_operations
  * implementation could check pages of the region and skip &action to respect
  * &filters
@@ -519,6 +524,7 @@ struct damos {
 	union {
 		int target_nid;
 	};
+	bool use_nodes_of_tier;
 	struct list_head filters;
 	struct list_head ops_filters;
 	void *last_applied;
@@ -896,7 +902,8 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
 			unsigned long apply_interval_us,
 			struct damos_quota *quota,
 			struct damos_watermarks *wmarks,
-			int target_nid);
+			int target_nid,
+			bool use_nodes_of_tier);
 void damon_add_scheme(struct damon_ctx *ctx, struct damos *s);
 void damon_destroy_scheme(struct damos *s);
 int damos_commit_quota_goals(struct damos_quota *dst, struct damos_quota *src);
diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h
index 0dc0cf2863e2..4434f95dfde2 100644
--- a/include/linux/memory-tiers.h
+++ b/include/linux/memory-tiers.h
@@ -56,6 +56,7 @@ void mt_put_memory_types(struct list_head *memory_types);
 int next_demotion_node(int node);
 void node_get_allowed_targets(pg_data_t *pgdat, nodemask_t *targets);
 bool node_is_toptier(int node);
+nodemask_t get_tier_nodemask(int node);
 #else
 static inline int next_demotion_node(int node)
 {
@@ -71,6 +72,10 @@ static inline bool node_is_toptier(int node)
 {
 	return true;
 }
+nodemask_t get_tier_nodemask(int node)
+{
+	return NODE_MASK_NONE;
+}
 #endif
 
 #else
diff --git a/mm/damon/core.c b/mm/damon/core.c
index b217e0120e09..2c0feca8f87b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -378,7 +378,8 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
 			unsigned long apply_interval_us,
 			struct damos_quota *quota,
 			struct damos_watermarks *wmarks,
-			int target_nid)
+			int target_nid,
+			bool use_nodes_of_tier)
 {
 	struct damos *scheme;
 
@@ -408,6 +409,7 @@ struct damos *damon_new_scheme(struct damos_access_pattern *pattern,
 	scheme->wmarks.activated = true;
 
 	scheme->target_nid = target_nid;
+	scheme->use_nodes_of_tier = use_nodes_of_tier;
 
 	return scheme;
 }
@@ -1006,7 +1008,7 @@ static int damon_commit_schemes(struct damon_ctx *dst, struct damon_ctx *src)
 				src_scheme->action,
 				src_scheme->apply_interval_us,
 				&src_scheme->quota, &src_scheme->wmarks,
-				NUMA_NO_NODE);
+				NUMA_NO_NODE, false);
 		if (!new_scheme)
 			return -ENOMEM;
 		err = damos_commit(new_scheme, src_scheme);
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 4af8fd4a390b..f663b66550dc 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -164,7 +164,8 @@ static struct damos *damon_lru_sort_new_scheme(
 			&quota,
 			/* (De)activate this according to the watermarks. */
 			&damon_lru_sort_wmarks,
-			NUMA_NO_NODE);
+			NUMA_NO_NODE,
+			false);
 }
 
 /* Create a DAMON-based operation scheme for hot memory regions */
diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
index e8464f7e0014..e13321cff38f 100644
--- a/mm/damon/paddr.c
+++ b/mm/damon/paddr.c
@@ -383,7 +383,7 @@ static unsigned long damon_pa_deactivate_pages(struct damon_region *r,
 
 static unsigned int __damon_pa_migrate_folio_list(
 		struct list_head *migrate_folios, struct pglist_data *pgdat,
-		int target_nid)
+		int target_nid, bool use_nodes_of_tier)
 {
 	unsigned int nr_succeeded = 0;
 	nodemask_t allowed_mask = NODE_MASK_NONE;
@@ -405,6 +405,9 @@ static unsigned int __damon_pa_migrate_folio_list(
 	if (list_empty(migrate_folios))
 		return 0;
 
+	if (use_nodes_of_tier)
+		allowed_mask = get_tier_nodemask(target_nid);
+
 	/* Migration ignores all cpuset and mempolicy settings */
 	migrate_pages(migrate_folios, alloc_migrate_folio, NULL,
 		      (unsigned long)&mtc, MIGRATE_ASYNC, MR_DAMON,
@@ -415,7 +418,7 @@ static unsigned int __damon_pa_migrate_folio_list(
 
 static unsigned int damon_pa_migrate_folio_list(struct list_head *folio_list,
 						struct pglist_data *pgdat,
-						int target_nid)
+						int target_nid, bool use_nodes_of_tier)
 {
 	unsigned int nr_migrated = 0;
 	struct folio *folio;
@@ -444,7 +447,7 @@ static unsigned int damon_pa_migrate_folio_list(struct list_head *folio_list,
 
 	/* Migrate folios selected for migration */
 	nr_migrated += __damon_pa_migrate_folio_list(
-			&migrate_folios, pgdat, target_nid);
+			&migrate_folios, pgdat, target_nid, use_nodes_of_tier);
 	/*
 	 * Folios that could not be migrated are still in @migrate_folios.  Add
 	 * those back on @folio_list
@@ -466,7 +469,7 @@ static unsigned int damon_pa_migrate_folio_list(struct list_head *folio_list,
 }
 
 static unsigned long damon_pa_migrate_pages(struct list_head *folio_list,
-					    int target_nid)
+					    int target_nid, bool use_nodes_of_tier)
 {
 	int nid;
 	unsigned long nr_migrated = 0;
@@ -489,13 +492,15 @@ static unsigned long damon_pa_migrate_pages(struct list_head *folio_list,
 
 		nr_migrated += damon_pa_migrate_folio_list(&node_folio_list,
 							   NODE_DATA(nid),
-							   target_nid);
+							   target_nid,
+							   use_nodes_of_tier);
 		nid = folio_nid(lru_to_folio(folio_list));
 	} while (!list_empty(folio_list));
 
 	nr_migrated += damon_pa_migrate_folio_list(&node_folio_list,
 						   NODE_DATA(nid),
-						   target_nid);
+						   target_nid,
+						   use_nodes_of_tier);
 
 	memalloc_noreclaim_restore(noreclaim_flag);
 
@@ -529,7 +534,7 @@ static unsigned long damon_pa_migrate(struct damon_region *r, struct damos *s,
 		addr += folio_size(folio);
 		folio_put(folio);
 	}
-	applied = damon_pa_migrate_pages(&folio_list, s->target_nid);
+	applied = damon_pa_migrate_pages(&folio_list, s->target_nid, s->use_nodes_of_tier);
 	cond_resched();
 	s->last_applied = folio;
 	return applied * PAGE_SIZE;
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index a675150965e0..1b770553bbd2 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -178,7 +178,8 @@ static struct damos *damon_reclaim_new_scheme(void)
 			&damon_reclaim_quota,
 			/* (De)activate this according to the watermarks. */
 			&damon_reclaim_wmarks,
-			NUMA_NO_NODE);
+			NUMA_NO_NODE,
+			false);
 }
 
 static int damon_reclaim_apply_parameters(void)
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 0f6c9e1fec0b..654e209fd6fd 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -1584,6 +1584,7 @@ struct damon_sysfs_scheme {
 	struct damon_sysfs_stats *stats;
 	struct damon_sysfs_scheme_regions *tried_regions;
 	int target_nid;
+	bool use_nodes_of_tier;
 };
 
 /* This should match with enum damos_action */
@@ -1612,6 +1613,7 @@ static struct damon_sysfs_scheme *damon_sysfs_scheme_alloc(
 	scheme->action = action;
 	scheme->apply_interval_us = apply_interval_us;
 	scheme->target_nid = NUMA_NO_NODE;
+	scheme->use_nodes_of_tier = false;
 	return scheme;
 }
 
@@ -1896,6 +1898,29 @@ static ssize_t target_nid_store(struct kobject *kobj,
 	return err ? err : count;
 }
 
+static ssize_t use_nodes_of_tier_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct damon_sysfs_scheme *scheme = container_of(kobj,
+			struct damon_sysfs_scheme, kobj);
+
+	return sysfs_emit(buf, "%s\n", scheme->use_nodes_of_tier ? "true" : "false");
+}
+
+static ssize_t use_nodes_of_tier_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	struct damon_sysfs_scheme *scheme = container_of(kobj,
+			struct damon_sysfs_scheme, kobj);
+	int err;
+
+	err = kstrtobool(buf, &scheme->use_nodes_of_tier);
+	if (err < 0)
+		return err;
+
+	return err ? err : count;
+}
+
 static void damon_sysfs_scheme_release(struct kobject *kobj)
 {
 	kfree(container_of(kobj, struct damon_sysfs_scheme, kobj));
@@ -1910,10 +1935,14 @@ static struct kobj_attribute damon_sysfs_scheme_apply_interval_us_attr =
 static struct kobj_attribute damon_sysfs_scheme_target_nid_attr =
 		__ATTR_RW_MODE(target_nid, 0600);
 
+static struct kobj_attribute damon_sysfs_scheme_use_nodes_of_tier_attr =
+		__ATTR_RW_MODE(use_nodes_of_tier, 0600);
+
 static struct attribute *damon_sysfs_scheme_attrs[] = {
 	&damon_sysfs_scheme_action_attr.attr,
 	&damon_sysfs_scheme_apply_interval_us_attr.attr,
 	&damon_sysfs_scheme_target_nid_attr.attr,
+	&damon_sysfs_scheme_use_nodes_of_tier_attr.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(damon_sysfs_scheme);
@@ -2258,7 +2287,7 @@ static struct damos *damon_sysfs_mk_scheme(
 
 	scheme = damon_new_scheme(&pattern, sysfs_scheme->action,
 			sysfs_scheme->apply_interval_us, &quota, &wmarks,
-			sysfs_scheme->target_nid);
+			sysfs_scheme->target_nid, sysfs_scheme->use_nodes_of_tier);
 	if (!scheme)
 		return NULL;
 
diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
index fc14fe53e9b7..393f3012a612 100644
--- a/mm/memory-tiers.c
+++ b/mm/memory-tiers.c
@@ -299,6 +299,19 @@ bool node_is_toptier(int node)
 	return toptier;
 }
 
+nodemask_t get_tier_nodemask(int node)
+{
+	struct memory_tier *memtier;
+	nodemask_t tier_nodes = NODE_MASK_NONE;
+
+	memtier = __node_get_memory_tier(node);
+	if (!memtier)
+		return tier_nodes;
+
+	tier_nodes = get_memtier_nodemask(memtier);
+	return tier_nodes;
+}
+
 void node_get_allowed_targets(pg_data_t *pgdat, nodemask_t *targets)
 {
 	struct memory_tier *memtier;
diff --git a/samples/damon/mtier.c b/samples/damon/mtier.c
index 36d2cd933f5a..b5d42086b221 100644
--- a/samples/damon/mtier.c
+++ b/samples/damon/mtier.c
@@ -105,7 +105,8 @@ static struct damon_ctx *damon_sample_mtier_build_ctx(bool promote)
 				.weight_age = 100,
 			},
 			&(struct damos_watermarks){},
-			promote ? 0 : 1);	/* migrate target node id */
+			promote ? 0 : 1,	/* migrate target node id */
+			false);
 	if (!scheme)
 		goto free_out;
 	damon_set_schemes(ctx, &scheme, 1);
diff --git a/samples/damon/prcl.c b/samples/damon/prcl.c
index 056b1b21a0fe..445a9f4e0905 100644
--- a/samples/damon/prcl.c
+++ b/samples/damon/prcl.c
@@ -88,7 +88,8 @@ static int damon_sample_prcl_start(void)
 			0,
 			&(struct damos_quota){},
 			&(struct damos_watermarks){},
-			NUMA_NO_NODE);
+			NUMA_NO_NODE,
+			false);
 	if (!scheme) {
 		damon_destroy_ctx(ctx);
 		return -ENOMEM;
-- 
2.39.3

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[kernel test robot]

Hi wangchuanguo,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on sj/damon/next next-20250528]
[cannot apply to linus/master v6.15]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/wangchuanguo/mm-migrate-restore-the-nmask-after-successfully-allocating-on-the-target-node/20250528-191141
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20250528111038.18378-3-wangchuanguo%40inspur.com
patch subject: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes
config: arc-randconfig-002-20250529 (https://download.01.org/0day-ci/archive/20250529/202505290538.2zlscryI-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250529/202505290538.2zlscryI-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505290538.2zlscryI-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/damon/paddr.c: In function '__damon_pa_migrate_folio_list':
>> mm/damon/paddr.c:409:18: error: implicit declaration of function 'get_tier_nodemask'; did you mean 'set_user_sigmask'? [-Werror=implicit-function-declaration]
      allowed_mask = get_tier_nodemask(target_nid);
                     ^~~~~~~~~~~~~~~~~
                     set_user_sigmask
>> mm/damon/paddr.c:409:16: error: incompatible types when assigning to type 'nodemask_t' {aka 'struct <anonymous>'} from type 'int'
      allowed_mask = get_tier_nodemask(target_nid);
                   ^
   cc1: some warnings being treated as errors


vim +409 mm/damon/paddr.c

   383	
   384	static unsigned int __damon_pa_migrate_folio_list(
   385			struct list_head *migrate_folios, struct pglist_data *pgdat,
   386			int target_nid, bool use_nodes_of_tier)
   387	{
   388		unsigned int nr_succeeded = 0;
   389		nodemask_t allowed_mask = NODE_MASK_NONE;
   390		struct migration_target_control mtc = {
   391			/*
   392			 * Allocate from 'node', or fail quickly and quietly.
   393			 * When this happens, 'page' will likely just be discarded
   394			 * instead of migrated.
   395			 */
   396			.gfp_mask = (GFP_HIGHUSER_MOVABLE & ~__GFP_RECLAIM) |
   397				__GFP_NOWARN | __GFP_NOMEMALLOC | GFP_NOWAIT,
   398			.nid = target_nid,
   399			.nmask = &allowed_mask
   400		};
   401	
   402		if (pgdat->node_id == target_nid || target_nid == NUMA_NO_NODE)
   403			return 0;
   404	
   405		if (list_empty(migrate_folios))
   406			return 0;
   407	
   408		if (use_nodes_of_tier)
 > 409			allowed_mask = get_tier_nodemask(target_nid);
   410	
   411		/* Migration ignores all cpuset and mempolicy settings */
   412		migrate_pages(migrate_folios, alloc_migrate_folio, NULL,
   413			      (unsigned long)&mtc, MIGRATE_ASYNC, MR_DAMON,
   414			      &nr_succeeded);
   415	
   416		return nr_succeeded;
   417	}
   418	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[SeongJae Park]

Hi wangchuanguo,

On Wed, 28 May 2025 19:10:38 +0800 wangchuanguo <wangchuanguo@inspur.com> wrote:

> This patch adds use_nodes_of_tier under
>   /sys/kernel/mm/damon/admin/kdamonds/<N>/contexts/<N>/schemes/<N>/
> 
> The 'use_nodes_of_tier' can be used to select nodes within the same memory
> tier of target_nid for DAMOS actions such as DAMOS_MIGRATE_{HOT,COLD}.

Could you please elaborate in what setup you think this option is useful, and
measurement of the usefulness if you have?

I'm asking the above question because of below reasons.  My anticiapted usage
of DAMOS_MIGRATE_{HOT,COLD} is for not only memory tiering but generic NUMA
node management.  And my proposed usage of these for memory tiering is making
per-node promotion/demotion for gradually promoting and demoting pages step by
step between node.  It could be slow but I anticipate such slow but continued
promotion/demotion is more important for reliable performance on production
systems of large time scale.  And I believe the approach can be applied to
general NUMA nodes management, once DAMON is extended for per-CPU access
monitoring.

I'm not saying this change is not useful, but asking you to give me a chance to
learn your changes, better.

> 
> Signed-off-by: wangchuanguo <wangchuanguo@inspur.com>
> ---
>  include/linux/damon.h        |  9 ++++++++-
>  include/linux/memory-tiers.h |  5 +++++
>  mm/damon/core.c              |  6 ++++--
>  mm/damon/lru_sort.c          |  3 ++-
>  mm/damon/paddr.c             | 19 ++++++++++++-------
>  mm/damon/reclaim.c           |  3 ++-
>  mm/damon/sysfs-schemes.c     | 31 ++++++++++++++++++++++++++++++-
>  mm/memory-tiers.c            | 13 +++++++++++++
>  samples/damon/mtier.c        |  3 ++-
>  samples/damon/prcl.c         |  3 ++-
>  10 files changed, 80 insertions(+), 15 deletions(-)

Can we please make this change more separated?  Maybe we can split the change
for memory-tiers.c, DAMON core layer, and DAMON sysfs interface.  That will
make review much easier.

I'll add more comments for details after above high level discussion is done.


Thanks,
SJ

[...]

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[Honggyu Kim]

Hi Simon and SeongJae,

Sorry for the late response.

On 5/28/2025 8:10 PM, wangchuanguo wrote:
> This patch adds use_nodes_of_tier under
>    /sys/kernel/mm/damon/admin/kdamonds/<N>/contexts/<N>/schemes/<N>/
> 
> The 'use_nodes_of_tier' can be used to select nodes within the same memory
> tier of target_nid for DAMOS actions such as DAMOS_MIGRATE_{HOT,COLD}.
> 
> Signed-off-by: wangchuanguo <wangchuanguo@inspur.com>
> ---
>   include/linux/damon.h        |  9 ++++++++-
>   include/linux/memory-tiers.h |  5 +++++
>   mm/damon/core.c              |  6 ++++--
>   mm/damon/lru_sort.c          |  3 ++-
>   mm/damon/paddr.c             | 19 ++++++++++++-------
>   mm/damon/reclaim.c           |  3 ++-
>   mm/damon/sysfs-schemes.c     | 31 ++++++++++++++++++++++++++++++-
>   mm/memory-tiers.c            | 13 +++++++++++++
>   samples/damon/mtier.c        |  3 ++-
>   samples/damon/prcl.c         |  3 ++-
>   10 files changed, 80 insertions(+), 15 deletions(-)

[...snip...]

> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index e8464f7e0014..e13321cff38f 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -383,7 +383,7 @@ static unsigned long damon_pa_deactivate_pages(struct damon_region *r,
>   
>   static unsigned int __damon_pa_migrate_folio_list(
>   		struct list_head *migrate_folios, struct pglist_data *pgdat,
> -		int target_nid)
> +		int target_nid, bool use_nodes_of_tier)
>   {
>   	unsigned int nr_succeeded = 0;
>   	nodemask_t allowed_mask = NODE_MASK_NONE;
> @@ -405,6 +405,9 @@ static unsigned int __damon_pa_migrate_folio_list(
>   	if (list_empty(migrate_folios))
>   		return 0;
>   
> +	if (use_nodes_of_tier)
> +		allowed_mask = get_tier_nodemask(target_nid);

I have a concern about this part.  This might work but, IMHO, the current memory
tier doesn't provide a concept of cross socket bridge, which is UPI in Intel.

For example, please see the following topology.

        node0           node1
      +-------+  UPI  +-------+
      | CPU 0 |-------| CPU 1 |
      +-------+       +-------+
      | DRAM0 |       | DRAM1 |  <-- memory tier 0
      +---+---+       +---+---+
          |               |
      +---+---+       +---+---+
      | CXL 0 |       | CXL 1 |  <-- memory tier 1
      +---+---+       +---+---+
        node2           node3

Even if some nodes are in the same memory tier, but if those are in the
different socket side, then the migratio makes the situation worse unexpectedly.

For example, if the page at node0 is tried to be demoted to node2, but if node2
is full then the current behavior is to cancel the demotion, but this change
makes it to be demoted to node3, which is on the other side of socket.

Since the cross socket access is a lot worse, I worry about this change.

Please let me know if you have a different thought.

Thanks,
Honggyu

Re: [PATCH 0/2] add a knob to control whether to use other nodes at the same tier of the target node in DAMON

[SeongJae Park]

Hi wangchuanguo,


Thank you for sending this patch series!

On Wed, 28 May 2025 19:10:36 +0800 wangchuanguo <wangchuanguo@inspur.com> wrote:

> In DAMON's migrate_hot and migrate_cold features, the code was
> intended to migrate pages ​​only to the node specified by target_nid​​.
> However, during testing, it was observed that memory allocation
> and migration could occur on ​​any nodes​​, which is a BUG.
> The first patch in this PR fix this issue.
> 
> A use_nodes_of_tier file has been added under the directory /sys/kernel/mm/damon/admin/kdamonds/<N>/contexts/<N>/schemes/<N>/
> to control whether to ​​use other nodes in the same tier as
> the target node​​ for migration.

I left a few comments on the patches.  Looking forward to discussions on each
sub-thread :)

> 
> wangchuanguo (2):

I believe your name is Wang Chuanguo?  Sorry if I read/wrote it wrongly.  But
we disallow[1] anonymous contributions, and prefer more formal Signed-off-by:
if possible.  Could you please use such formal Signed-off-by: identity, say,
"Wang Chuanguo <wangchuanguo@inspur.com>" from next time?

[1] https://docs.kernel.org/process/submitting-patches.html#developer-s-certificate-of-origin-1-1


Thanks,
SJ

[...]

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[Simon Wang (王传国)]

> > This patch adds use_nodes_of_tier under
> >
> /sys/kernel/mm/damon/admin/kdamonds/<N>/contexts/<N>/schemes/<N>/
> >
> > The 'use_nodes_of_tier' can be used to select nodes within the same
> > memory tier of target_nid for DAMOS actions such as
> DAMOS_MIGRATE_{HOT,COLD}.
> 
> Could you please elaborate in what setup you think this option is useful, and
> measurement of the usefulness if you have?
> 
> I'm asking the above question because of below reasons.  My anticiapted
> usage of DAMOS_MIGRATE_{HOT,COLD} is for not only memory tiering but
> generic NUMA node management.  And my proposed usage of these for
> memory tiering is making per-node promotion/demotion for gradually
> promoting and demoting pages step by step between node.  It could be slow
> but I anticipate such slow but continued promotion/demotion is more
> important for reliable performance on production systems of large time scale.
> And I believe the approach can be applied to general NUMA nodes
> management, once DAMON is extended for per-CPU access monitoring.
> 
> I'm not saying this change is not useful, but asking you to give me a chance to
> learn your changes, better.

I believe some users may want to ​​use only the target node's memory​​ and reserve other nodes in the same tier for specific applications. Therefore, I added a switch file use_nodes_of_tier. I think it might be better to set the default value of use_nodes_of_tier to true (i.e., allow using fallback nodes). What do you think

> >
> > Signed-off-by: wangchuanguo <wangchuanguo@inspur.com>
> > ---
> >  include/linux/damon.h        |  9 ++++++++-
> >  include/linux/memory-tiers.h |  5 +++++
> >  mm/damon/core.c              |  6 ++++--
> >  mm/damon/lru_sort.c          |  3 ++-
> >  mm/damon/paddr.c             | 19 ++++++++++++-------
> >  mm/damon/reclaim.c           |  3 ++-
> >  mm/damon/sysfs-schemes.c     | 31
> ++++++++++++++++++++++++++++++-
> >  mm/memory-tiers.c            | 13 +++++++++++++
> >  samples/damon/mtier.c        |  3 ++-
> >  samples/damon/prcl.c         |  3 ++-
> >  10 files changed, 80 insertions(+), 15 deletions(-)
> 
> Can we please make this change more separated?  Maybe we can split the
> change for memory-tiers.c, DAMON core layer, and DAMON sysfs interface.
> That will make review much easier.

Yes,I'll split this patch to be 2 patches.


> I'll add more comments for details after above high level discussion is done.
> 
> 
> Thanks,
> SJ
> 
> [...]

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[SeongJae Park]

On Thu, 29 May 2025 03:12:13 +0000 Simon Wang (王传国) <wangchuanguo@inspur.com> wrote:

> > > This patch adds use_nodes_of_tier under
> > >
> > /sys/kernel/mm/damon/admin/kdamonds/<N>/contexts/<N>/schemes/<N>/
> > >
> > > The 'use_nodes_of_tier' can be used to select nodes within the same
> > > memory tier of target_nid for DAMOS actions such as
> > DAMOS_MIGRATE_{HOT,COLD}.
> > 
> > Could you please elaborate in what setup you think this option is useful, and
> > measurement of the usefulness if you have?
> > 
> > I'm asking the above question because of below reasons.  My anticiapted
> > usage of DAMOS_MIGRATE_{HOT,COLD} is for not only memory tiering but
> > generic NUMA node management.  And my proposed usage of these for
> > memory tiering is making per-node promotion/demotion for gradually
> > promoting and demoting pages step by step between node.  It could be slow
> > but I anticipate such slow but continued promotion/demotion is more
> > important for reliable performance on production systems of large time scale.
> > And I believe the approach can be applied to general NUMA nodes
> > management, once DAMON is extended for per-CPU access monitoring.
> > 
> > I'm not saying this change is not useful, but asking you to give me a chance to
> > learn your changes, better.
> 
> I believe some users may want to ​​use only the target node's
> memory​​ and reserve other nodes in the same tier for specific
> applications. Therefore, I added a switch file use_nodes_of_tier.

Thank you for clarifying, Simon.

Because this is an ABI change that difficult to revert and therefore we may
need to support for long term, I'd like to have more clear theory and/or data
if possible.  In my humble opinion, above clarification doesn't sound like a
strong enough justification for ABI change.

More specifically, it would be better if you could answer below questions.  Who
would be such users, how common the use case would be, and what are the benefit
of doing so?  Is that only theory?  Or, a real existing use case?  Can you
share measurement of the benefit from this change that measured from real
workloads or benchmarks?  Is there an alternative way to do this without ABI
change?

> I think it might be better to set the default value of use_nodes_of_tier to
> true (i.e., allow using fallback nodes). What do you think

In my humble opinion, we can consider setting it true by default, if we agree
the benefit of the change is significant.  With only currently given
information, I cannot easily say if I think this can really be useful.  As
asked abovely, more clear thoery and/or real data would be helpful.

> 
> > >
> > > Signed-off-by: wangchuanguo <wangchuanguo@inspur.com>
> > > ---
> > >  include/linux/damon.h        |  9 ++++++++-
> > >  include/linux/memory-tiers.h |  5 +++++
> > >  mm/damon/core.c              |  6 ++++--
> > >  mm/damon/lru_sort.c          |  3 ++-
> > >  mm/damon/paddr.c             | 19 ++++++++++++-------
> > >  mm/damon/reclaim.c           |  3 ++-
> > >  mm/damon/sysfs-schemes.c     | 31
> > ++++++++++++++++++++++++++++++-
> > >  mm/memory-tiers.c            | 13 +++++++++++++
> > >  samples/damon/mtier.c        |  3 ++-
> > >  samples/damon/prcl.c         |  3 ++-
> > >  10 files changed, 80 insertions(+), 15 deletions(-)
> > 
> > Can we please make this change more separated?  Maybe we can split the
> > change for memory-tiers.c, DAMON core layer, and DAMON sysfs interface.
> > That will make review much easier.
> 
> Yes,I'll split this patch to be 2 patches.

Thank you for accepting my suggestion.  But I think it deserves 3 patches, each
for

- memory-tiers.c,
- DAMON core layer, and
- and DAMON sysfs interface.

But, let's further discuss on the high level topic (if this change is really
beneficial enough to make ABI change).


Thanks,
SJ

[...]

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[Simon Wang (王传国)]

> > > > This patch adds use_nodes_of_tier under
> > > >
> > >
> /sys/kernel/mm/damon/admin/kdamonds/<N>/contexts/<N>/schemes/<N>/
> > > >
> > > > The 'use_nodes_of_tier' can be used to select nodes within the
> > > > same memory tier of target_nid for DAMOS actions such as
> > > DAMOS_MIGRATE_{HOT,COLD}.
> > >
> > > Could you please elaborate in what setup you think this option is
> > > useful, and measurement of the usefulness if you have?
> > >
> > > I'm asking the above question because of below reasons.  My
> > > anticiapted usage of DAMOS_MIGRATE_{HOT,COLD} is for not only memory
> > > tiering but generic NUMA node management.  And my proposed usage of
> > > these for memory tiering is making per-node promotion/demotion for
> > > gradually promoting and demoting pages step by step between node.
> > > It could be slow but I anticipate such slow but continued
> > > promotion/demotion is more important for reliable performance on
> production systems of large time scale.
> > > And I believe the approach can be applied to general NUMA nodes
> > > management, once DAMON is extended for per-CPU access monitoring.
> > >
> > > I'm not saying this change is not useful, but asking you to give me
> > > a chance to learn your changes, better.
> >
> > I believe some users may want to ​​use only the target node's memory​​
> > and reserve other nodes in the same tier for specific applications.
> > Therefore, I added a switch file use_nodes_of_tier.
> 
> Thank you for clarifying, Simon.
> 
> Because this is an ABI change that difficult to revert and therefore we may
> need to support for long term, I'd like to have more clear theory and/or data if
> possible.  In my humble opinion, above clarification doesn't sound like a
> strong enough justification for ABI change.
> 
> More specifically, it would be better if you could answer below questions.
> Who would be such users, how common the use case would be, and what are
> the benefit of doing so?  Is that only theory?  Or, a real existing use case?
> Can you share measurement of the benefit from this change that measured
> from real workloads or benchmarks?  Is there an alternative way to do this
> without ABI change?


Your concern is that adding the bool use_nodes_of_tier variable and introducing 
an additional parameter to multiple functions would cause ABI changes, correct?​​

​​I propose avoiding the creation of the 'use_nodes_of_tier' sysfs file. Instead, 
we can modify the __damon_pa_migrate_folio_list() function to change the allowed_mask 
from NODE_MASK_NONE to the full node mask of the entire tier where the target_nid resides. 
This approach would be similar to the implementation in commit 320080272892
 ('mm/demotion: demote pages according to allocation fallback order').

I'd like to confirm two modification points with you:
​​1.Regarding alloc_migrate_folio()​​:
Restoring the original nodemask and gfp_mask in this function is the correct approach, correct?
​​2.Regarding DAMON's migration logic​​:
The target scope should be expanded from a single specified node to the entire memory tier
 (where the target node resides), correct?
​​Can we confirm these two points are agreed upon?​
 
> > I think it might be better to set the default value of
> > use_nodes_of_tier to true (i.e., allow using fallback nodes). What do
> > you think
> 
> In my humble opinion, we can consider setting it true by default, if we agree
> the benefit of the change is significant.  With only currently given information,
> I cannot easily say if I think this can really be useful.  As asked abovely, more
> clear thoery and/or real data would be helpful.
> 
> >
> > > >
> > > > Signed-off-by: wangchuanguo <wangchuanguo@inspur.com>
> > > > ---
> > > >  include/linux/damon.h        |  9 ++++++++-
> > > >  include/linux/memory-tiers.h |  5 +++++
> > > >  mm/damon/core.c              |  6 ++++--
> > > >  mm/damon/lru_sort.c          |  3 ++-
> > > >  mm/damon/paddr.c             | 19 ++++++++++++-------
> > > >  mm/damon/reclaim.c           |  3 ++-
> > > >  mm/damon/sysfs-schemes.c     | 31
> > > ++++++++++++++++++++++++++++++-
> > > >  mm/memory-tiers.c            | 13 +++++++++++++
> > > >  samples/damon/mtier.c        |  3 ++-
> > > >  samples/damon/prcl.c         |  3 ++-
> > > >  10 files changed, 80 insertions(+), 15 deletions(-)
> > >
> > > Can we please make this change more separated?  Maybe we can split
> > > the change for memory-tiers.c, DAMON core layer, and DAMON sysfs
> interface.
> > > That will make review much easier.
> >
> > Yes,I'll split this patch to be 2 patches.
> 
> Thank you for accepting my suggestion.  But I think it deserves 3 patches,
> each for
> 
> - memory-tiers.c,
> - DAMON core layer, and
> - and DAMON sysfs interface.
> 
> But, let's further discuss on the high level topic (if this change is really
> beneficial enough to make ABI change).
> 
> 
> Thanks,
> SJ
> 
> [...]

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[SeongJae Park]

Hi Simon,


Thank you for continuing this important discussion.

Before starting, though, seems your mail client is not setting 'In-Reply-To'
field of your mails.  For people who uses 'In-Reply-To' field based threads
displaying tools, ths thread could be difficult to read the whole contents.
Please consider using tools that set the field correctly if possible.

You could get more information about available mailing tools from
https://docs.kernel.org/process/email-clients.html

Btw, I use hkml
(https://docs.kernel.org/process/email-clients.html#hackermail-tui) ;)

On Fri, 30 May 2025 08:04:42 +0000 Simon Wang (王传国) <wangchuanguo@inspur.com> wrote:

[...]
> Your concern is that adding the bool use_nodes_of_tier variable and introducing 
> an additional parameter to multiple functions would cause ABI changes, correct?​​

You are correct.

> 
> ​​I propose avoiding the creation of the 'use_nodes_of_tier' sysfs
> file. Instead, we can modify the __damon_pa_migrate_folio_list() function to
> change the allowed_mask from NODE_MASK_NONE to the full node mask of the
> entire tier where the target_nid resides.  This approach would be similar to
> the implementation in commit 320080272892 ('mm/demotion: demote pages
> according to allocation fallback order').

Then, this causes a behavior change, which we should not allow if it can be
considered a regression.  In other words, we could do this if it is a clear
improvement.

So, let's think about if your proposed change is an improvement.  As the commit
320080272892 is nicely explaining, I think that it is an improved behavior for
demotion.  Actually it seems good behavior for promotion, too.  But, the
behavior we are discussing here is not for the demotion but general migration
(specifically, DAMOS_MIGRATE_{HOT,COLD}).

In my opinion, DAMOS_MIGRATE_{HOT,COLD} behavior should be somewhat similar to
that of move_pages() syscall, to make its behavior easy to expect.  So I think
having commit 320080272892's behavior improvement to DAMOS_MIGRATE_{HOT,COLD}
is not a right thing to do.

And this asks me a question.  Is current DAMOS_MIGRATE_{HOT,COLD} behavior
similar to move_pages() syscall?  Not really, since do_move_pages_to_node(),
which is called from move_pages() syscall and calls migrate_pages() is setting
mtc->nmask as NULL, while DAMOS_MIGRATE_{HOT,COLD} set it as NODE_MASK_NONE.
Also, do_move_pages_to_node() uses alloc_migration_target() while
DAMOS_MIGRATE_{HOT,COLD} uses alloc_migrate_folio().

I overlooked this different behavior while reviewing this code, sorry.  And I
don't think this difference is what we need to keep, unless there are good
rasons that well documented.  Thank you for let us find this, Simon.

So I suggest to set mtc->nmask as NULL, and use alloc_migration_target() from
__damon_pa_migrate_folio_list(), same to move_pages() system call.  To use
alloc_migrate_folio() from __damon_pa_migrate_folio_list(), we renamed it from
alloc_demote_folio(), and made it none-static.  If we use
alloc_migration_target() from __damon_pa_migrate_folio_list(), there is no
reason to keep the changes.  Let's revert those too.

Cc-ing Honggyu, who originally implemented the current behavior of
__damon_pa_migrate().  Honggyu, could you please let us know if the above
suggested changes are not ok for you?

If Honggyu has no problem at the suggested change, Simon, would you mind doing
that?  I can also make the patches.  I don't really care who do that.  I just
think someone should do that.  This shouldn't be urgent real issue, in my
opinion, though.

> 
> I'd like to confirm two modification points with you:
> ​​1.Regarding alloc_migrate_folio()​​:
> Restoring the original nodemask and gfp_mask in this function is the correct approach, correct?

I think that's correct, but let's discuss about the patch on the patch's
thread.

> ​​2.Regarding DAMON's migration logic​​:
> The target scope should be expanded from a single specified node to the entire memory tier
>  (where the target node resides), correct?

I don't think so, as abovely explained.

> ​​Can we confirm these two points are agreed upon?​

I believe hope this is answered above.


Thanks,
SJ

[...]

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[wangchuanguo]

On Fri, 30 May 2025 12:40:16 -0700 SeongJae Park <sj@kernel.org> wrote:

> Hi Simon,
> 
> 
> Thank you for continuing this important discussion.
> 
> Before starting, though, seems your mail client is not setting 'In-Reply-To'
> field of your mails.  For people who uses 'In-Reply-To' field based threads
> displaying tools, ths thread could be difficult to read the whole contents.
> Please consider using tools that set the field correctly if possible.
> 
> You could get more information about available mailing tools from
> https://docs.kernel.org/process/email-clients.html
> 
> Btw, I use hkml
> (https://docs.kernel.org/process/email-clients.html#hackermail-tui) ;)
> 
> On Fri, 30 May 2025 08:04:42 +0000 Simon Wang (王传国) <wangchuanguo@inspur.com> wrote:
> 
> [...]
> > Your concern is that adding the bool use_nodes_of_tier variable and introducing 
> > an additional parameter to multiple functions would cause ABI changes, correct?​​
> 
> You are correct.
> 
> > 
> > ​​I propose avoiding the creation of the 'use_nodes_of_tier' sysfs
> > file. Instead, we can modify the __damon_pa_migrate_folio_list() function to
> > change the allowed_mask from NODE_MASK_NONE to the full node mask of the
> > entire tier where the target_nid resides.  This approach would be similar to
> > the implementation in commit 320080272892 ('mm/demotion: demote pages
> > according to allocation fallback order').
> 
> Then, this causes a behavior change, which we should not allow if it can be
> considered a regression.  In other words, we could do this if it is a clear
> improvement.
> 
> So, let's think about if your proposed change is an improvement.  As the commit
> 320080272892 is nicely explaining, I think that it is an improved behavior for
> demotion.  Actually it seems good behavior for promotion, too.  But, the
> behavior we are discussing here is not for the demotion but general migration
> (specifically, DAMOS_MIGRATE_{HOT,COLD}).
> 
> In my opinion, DAMOS_MIGRATE_{HOT,COLD} behavior should be somewhat similar to
> that of move_pages() syscall, to make its behavior easy to expect.  So I think
> having commit 320080272892's behavior improvement to DAMOS_MIGRATE_{HOT,COLD}
> is not a right thing to do.
> 
> And this asks me a question.  Is current DAMOS_MIGRATE_{HOT,COLD} behavior
> similar to move_pages() syscall?  Not really, since do_move_pages_to_node(),
> which is called from move_pages() syscall and calls migrate_pages() is setting
> mtc->nmask as NULL, while DAMOS_MIGRATE_{HOT,COLD} set it as NODE_MASK_NONE.
> Also, do_move_pages_to_node() uses alloc_migration_target() while
> DAMOS_MIGRATE_{HOT,COLD} uses alloc_migrate_folio().
> 
> I overlooked this different behavior while reviewing this code, sorry.  And I
> don't think this difference is what we need to keep, unless there are good
> rasons that well documented.  Thank you for let us find this, Simon.
> 
> So I suggest to set mtc->nmask as NULL, and use alloc_migration_target() from
> __damon_pa_migrate_folio_list(), same to move_pages() system call.  To use
> alloc_migrate_folio() from __damon_pa_migrate_folio_list(), we renamed it from
> alloc_demote_folio(), and made it none-static.  If we use
> alloc_migration_target() from __damon_pa_migrate_folio_list(), there is no
> reason to keep the changes.  Let's revert those too.
> 
> Cc-ing Honggyu, who originally implemented the current behavior of
> __damon_pa_migrate().  Honggyu, could you please let us know if the above
> suggested changes are not ok for you?
> 
> If Honggyu has no problem at the suggested change, Simon, would you mind doing
> that?  I can also make the patches.  I don't really care who do that.  I just
> think someone should do that.  This shouldn't be urgent real issue, in my
> opinion, though.
> 
> > 
> > I'd like to confirm two modification points with you:
> > ​​1.Regarding alloc_migrate_folio()​​:
> > Restoring the original nodemask and gfp_mask in this function is the correct approach, correct?
> 
> I think that's correct, but let's discuss about the patch on the patch's
> thread.
> 
> > ​​2.Regarding DAMON's migration logic​​:
> > The target scope should be expanded from a single specified node to the entire memory tier
> >  (where the target node resides), correct?
> 
> I don't think so, as abovely explained.
> 
> > ​​Can we confirm these two points are agreed upon?​
> 
> I believe hope this is answered above.
> 
> 
> Thanks,
> SJ
> 
> [...]

Sent using hkml (https://github.com/sjp38/hackermail)

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[SeongJae Park]

On Fri, 30 May 2025 12:40:16 -0700 SeongJae Park <sj@kernel.org> wrote:
[...] 
> So I suggest to set mtc->nmask as NULL, and use alloc_migration_target() from
> __damon_pa_migrate_folio_list(), same to move_pages() system call.  To use
> alloc_migrate_folio() from __damon_pa_migrate_folio_list(), we renamed it from
> alloc_demote_folio(), and made it none-static.  If we use
> alloc_migration_target() from __damon_pa_migrate_folio_list(), there is no
> reason to keep the changes.  Let's revert those too.
> 
> Cc-ing Honggyu, who originally implemented the current behavior of
> __damon_pa_migrate().  Honggyu, could you please let us know if the above
> suggested changes are not ok for you?
> 
> If Honggyu has no problem at the suggested change, Simon, would you mind doing
> that?  I can also make the patches.  I don't really care who do that.  I just
> think someone should do that.  This shouldn't be urgent real issue, in my
> opinion, though.

This is a friendly reminder of the above.  Please let me know if you have any
opinion.  I will proceed to make the patches if I don't get other inputs by
this week.


Thanks,
SJ

[...]

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[Honggyu Kim]

Hi SeongJae and Simon,

On 5/31/2025 4:40 AM, SeongJae Park wrote:
> Hi Simon,
> 
> 
> Thank you for continuing this important discussion.
> 
> Before starting, though, seems your mail client is not setting 'In-Reply-To'
> field of your mails.  For people who uses 'In-Reply-To' field based threads
> displaying tools, ths thread could be difficult to read the whole contents.
> Please consider using tools that set the field correctly if possible.

Sorry for the late response, I also had some difficulty to find its original
patch and I just found it and replied at the following links.
https://lore.kernel.org/linux-mm/20250528111038.18378-3-wangchuanguo@inspur.com
https://lore.kernel.org/linux-mm/74a7db85-8fcc-4bd5-8656-0f4d0670f205@sk.com

> 
> You could get more information about available mailing tools from
> https://docs.kernel.org/process/email-clients.html
> 
> Btw, I use hkml
> (https://docs.kernel.org/process/email-clients.html#hackermail-tui) ;)
> 
> On Fri, 30 May 2025 08:04:42 +0000 Simon Wang (王传国) <wangchuanguo@inspur.com> wrote:
> 
> [...]
>> Your concern is that adding the bool use_nodes_of_tier variable and introducing
>> an additional parameter to multiple functions would cause ABI changes, correct?​​
> 
> You are correct.
> 
>>
>> ​​I propose avoiding the creation of the 'use_nodes_of_tier' sysfs
>> file. Instead, we can modify the __damon_pa_migrate_folio_list() function to
>> change the allowed_mask from NODE_MASK_NONE to the full node mask of the
>> entire tier where the target_nid resides.  This approach would be similar to
>> the implementation in commit 320080272892 ('mm/demotion: demote pages
>> according to allocation fallback order').
> 
> Then, this causes a behavior change, which we should not allow if it can be
> considered a regression.  In other words, we could do this if it is a clear
> improvement.

I agree this is a behavior change.

> So, let's think about if your proposed change is an improvement.  As the commit
> 320080272892 is nicely explaining, I think that it is an improved behavior for
> demotion.  Actually it seems good behavior for promotion, too.  But, the
> behavior we are discussing here is not for the demotion but general migration
> (specifically, DAMOS_MIGRATE_{HOT,COLD}).
> 
> In my opinion, DAMOS_MIGRATE_{HOT,COLD} behavior should be somewhat similar to
> that of move_pages() syscall, to make its behavior easy to expect.  So I think
> having commit 320080272892's behavior improvement to DAMOS_MIGRATE_{HOT,COLD}
> is not a right thing to do.
> 
> And this asks me a question.  Is current DAMOS_MIGRATE_{HOT,COLD} behavior
> similar to move_pages() syscall?  Not really, since do_move_pages_to_node(),
> which is called from move_pages() syscall and calls migrate_pages() is setting
> mtc->nmask as NULL, while DAMOS_MIGRATE_{HOT,COLD} set it as NODE_MASK_NONE.
 >
> Also, do_move_pages_to_node() uses alloc_migration_target() while
> DAMOS_MIGRATE_{HOT,COLD} uses alloc_migrate_folio().

I can see alloc_migrate_folio() also calls alloc_migration_target(), but do you
mean alloc_migrate_folio() setting mtc->nmask to NULL is the difference?

> 
> I overlooked this different behavior while reviewing this code, sorry.  And I
> don't think this difference is what we need to keep, unless there are good
> rasons that well documented.  Thank you for let us find this, Simon.
> 
> So I suggest to set mtc->nmask as NULL, and use alloc_migration_target() from
> __damon_pa_migrate_folio_list(), same to move_pages() system call.  To use
> alloc_migrate_folio() from __damon_pa_migrate_folio_list(), we renamed it from
> alloc_demote_folio(), and made it none-static.  If we use
> alloc_migration_target() from __damon_pa_migrate_folio_list(), there is no
> reason to keep the changes.  Let's revert those too.
> 
> Cc-ing Honggyu, who originally implemented the current behavior of
> __damon_pa_migrate().  Honggyu, could you please let us know if the above
> suggested changes are not ok for you?
> 
> If Honggyu has no problem at the suggested change, Simon, would you mind doing
> that?  I can also make the patches.  I don't really care who do that.  I just
> think someone should do that.  This shouldn't be urgent real issue, in my
> opinion, though.
> 
>>
>> I'd like to confirm two modification points with you:
>> ​​1.Regarding alloc_migrate_folio()​​:
>> Restoring the original nodemask and gfp_mask in this function is the correct approach, correct?

I also think restoring the both mtc->nmask and mtc->gfp_mask are needed.

> I think that's correct, but let's discuss about the patch on the patch's
> thread.
> 
>> ​​2.Regarding DAMON's migration logic​​:
>> The target scope should be expanded from a single specified node to the entire memory tier
>>   (where the target node resides), correct?
> 
> I don't think so, as abovely explained.

I also think this makes our use case unexpected and cannot prevent migration is
done beyond other side of socket.

> 
>> ​​Can we confirm these two points are agreed upon?​
> 
> I believe hope this is answered above.
> 
> 
> Thanks,
> SJ
> 
> [...]

Thanks,
Honggyu

Re: [PATCH 2/2] mm/damon/sysfs-schemes: add use_nodes_of_tier on sysfs-schemes

[SeongJae Park]

On Mon, 9 Jun 2025 21:39:50 +0900 Honggyu Kim <honggyu.kim@sk.com> wrote:

> Hi SeongJae and Simon,
> 
> On 5/31/2025 4:40 AM, SeongJae Park wrote:
[...]
> > On Fri, 30 May 2025 08:04:42 +0000 Simon Wang (王传国) <wangchuanguo@inspur.com> wrote:
[...]
> > So, let's think about if your proposed change is an improvement.  As the commit
> > 320080272892 is nicely explaining, I think that it is an improved behavior for
> > demotion.  Actually it seems good behavior for promotion, too.  But, the
> > behavior we are discussing here is not for the demotion but general migration
> > (specifically, DAMOS_MIGRATE_{HOT,COLD}).
> > 
> > In my opinion, DAMOS_MIGRATE_{HOT,COLD} behavior should be somewhat similar to
> > that of move_pages() syscall, to make its behavior easy to expect.  So I think
> > having commit 320080272892's behavior improvement to DAMOS_MIGRATE_{HOT,COLD}
> > is not a right thing to do.
> > 
> > And this asks me a question.  Is current DAMOS_MIGRATE_{HOT,COLD} behavior
> > similar to move_pages() syscall?  Not really, since do_move_pages_to_node(),
> > which is called from move_pages() syscall and calls migrate_pages() is setting
> > mtc->nmask as NULL, while DAMOS_MIGRATE_{HOT,COLD} set it as NODE_MASK_NONE.
>  >
> > Also, do_move_pages_to_node() uses alloc_migration_target() while
> > DAMOS_MIGRATE_{HOT,COLD} uses alloc_migrate_folio().
> 
> I can see alloc_migrate_folio() also calls alloc_migration_target(), but do you
> mean alloc_migrate_folio() setting mtc->nmask to NULL is the difference?

Yes, and also alloc_migration_target()'s internal optimizations for demotion
use case.

Nonetheless, I'm saying about the differences between DAMOS_MIGRATE_{HOT,COLD}
and move_pages() behaviors in the bigger context.

> 
> > 
> > I overlooked this different behavior while reviewing this code, sorry.  And I
> > don't think this difference is what we need to keep, unless there are good
> > rasons that well documented.  Thank you for let us find this, Simon.
> > 
> > So I suggest to set mtc->nmask as NULL, and use alloc_migration_target() from
> > __damon_pa_migrate_folio_list(), same to move_pages() system call.  To use
> > alloc_migrate_folio() from __damon_pa_migrate_folio_list(), we renamed it from
> > alloc_demote_folio(), and made it none-static.  If we use
> > alloc_migration_target() from __damon_pa_migrate_folio_list(), there is no
> > reason to keep the changes.  Let's revert those too.
> > 
> > Cc-ing Honggyu, who originally implemented the current behavior of
> > __damon_pa_migrate().  Honggyu, could you please let us know if the above
> > suggested changes are not ok for you?
> > 
> > If Honggyu has no problem at the suggested change, Simon, would you mind doing
> > that?  I can also make the patches.  I don't really care who do that.  I just
> > think someone should do that.  This shouldn't be urgent real issue, in my
> > opinion, though.

I will send an RFC for this soon, to make discussions easier and unblocked.


Thanks,
SJ

[...]