[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 1/2] mm: migrate: restore the nmask after successfully allocating on the target node

[Simon Wang (王传国)]

> + 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.
> 

Under the original logic, the alloc_migrate_folio function would attempt to allocate new memory ​​sequentially across all nodes based on distance​​, even for nodes at the same tier, which is nonsensical. For example, if nodes 0 and 1 are DRAM nodes and nodes 2 and 3 are CXL nodes, attempting to promote a hot page from node 2 to node 0 would ​​erroneously fall back to nodes 2 and 3​​ (the same tier as the source node) if nodes 0 and 1 are out of space. This is a BUG.In ​​Patch 1​​, I fix this BUG. 

In ​​Patch 2​​, I extend the target node range from node 0 to nodes 0 and 1. To accommodate users who require strict migration (e.g., migrating only to node 0 and aborting if it is full), I added a ​​sysfs toggle​​ in Patch 2.
​​Question​​: Should this sysfs toggle default to true (allow fallback to other nodes) or false (strict mode: migrate only to node 0, abort if full)? I would appreciate your advice on the default value, considering backward compatibility and use cases.

> >
> > 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?

Yes, it's a good idea. I will do it.
 

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