[RFC PATCH v2 0/3] Add NUMA-aware DAMOS watermarks

[RFC PATCH v2 0/3] Add NUMA-aware DAMOS watermarks

[Jonghyeon Kim]

Current DAMOS schemes are not considered with multiple NUMA memory nodes.
For example, If we want to proactively reclaim memory of a one NUMA node,
DAMON_RECLAIM has to wake up kdamond before kswapd does reclaim memory.
However, since the DAMON watermarks are based on not a one NUMA memory
node but total system free memory, kdamond is not waked up before invoking
memory reclamation from kswapd of the target node.

These patches allow for DAMON to select monitoring target either total
memory or a specific NUMA memory node.

---
Changes from RFC PATCH v1
(https://lore.kernel.org/all/20220218102611.31895-1-tome01@ajou.ac.kr)
- Add new metric type for NUMA node, DAMOS_WMARK_NODE_FREE_MEM_RATE
- Drop commit about damon_start()
- Support DAMON_LRU_SORT

Jonghyeon Kim (3):
  mm/damon: Add new metric type and target node for watermark
  mm/damon: add module parameters for NUMA system
  mm/damon: add NUMA-awareness to DAMON modules

 include/linux/damon.h     | 11 +++++++++--
 mm/damon/core.c           | 11 ++++++++---
 mm/damon/lru_sort.c       | 14 ++++++++++++++
 mm/damon/modules-common.h |  4 +++-
 mm/damon/reclaim.c        | 14 ++++++++++++++
 mm/damon/sysfs-schemes.c  | 35 +++++++++++++++++++++++++++++++++--
 6 files changed, 81 insertions(+), 8 deletions(-)

-- 
2.34.1

[RFC PATCH v2 1/3] mm/damon: Add new metric type and target node for watermark

[Jonghyeon Kim]

For NUMA systems, there is a need to allow damos to select watermark
options for monitoring a target NUMA node or total system memory.

This patch adds new watermark metric type, DAMOS_WMARK_NODE_FREE_MEM_RATE,
and its target node. By doing so, DAMON can avoid that kdamond is not
waked up although the free memory of the target node is between mid and
low watermark.

Signed-off-by: Jonghyeon Kim <tome01@ajou.ac.kr>
---
 include/linux/damon.h    | 11 +++++++++--
 mm/damon/core.c          | 11 ++++++++---
 mm/damon/sysfs-schemes.c | 35 +++++++++++++++++++++++++++++++++--
 3 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 5881e4ac3..d8daa30d7 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -206,11 +206,14 @@ struct damos_quota {
  *
  * @DAMOS_WMARK_NONE:		Ignore the watermarks of the given scheme.
  * @DAMOS_WMARK_FREE_MEM_RATE:	Free memory rate of the system in [0,1000].
+ * @DAMOS_WMARK_NODE_FREE_MEM_RATE: Free memory rate of the system in [0,1000]
+ *				    for specific NUMA node.
  * @NR_DAMOS_WMARK_METRICS:	Total number of DAMOS watermark metrics
  */
 enum damos_wmark_metric {
 	DAMOS_WMARK_NONE,
 	DAMOS_WMARK_FREE_MEM_RATE,
+	DAMOS_WMARK_NODE_FREE_MEM_RATE,
 	NR_DAMOS_WMARK_METRICS,
 };
 
@@ -221,11 +224,14 @@ enum damos_wmark_metric {
  * @high:	High watermark.
  * @mid:	Middle watermark.
  * @low:	Low watermark.
+ * @target_nid:	Target NUMA node id for the watermarks.
  *
  * If &metric is &DAMOS_WMARK_NONE, the scheme is always active.  Being active
  * means DAMON does monitoring and applying the action of the scheme to
- * appropriate memory regions.  Else, DAMON checks &metric of the system for at
- * least every &interval microseconds and works as below.
+ * appropriate memory regions.  Else if $metric is $DAMOS_WMARK_FREE_MEM_RATE,
+ * DAMON checks &metric of the system for at least every &interval microseconds
+ * and works as below. If $metric is $DAMOS_WMARK_NODE_FREE_MEM_RATE, DAMON
+ * only chceks $metric of the $target_nid.
  *
  * If &metric is higher than &high, the scheme is inactivated.  If &metric is
  * between &mid and &low, the scheme is activated.  If &metric is lower than
@@ -237,6 +243,7 @@ struct damos_watermarks {
 	unsigned long high;
 	unsigned long mid;
 	unsigned long low;
+	int target_nid;
 
 /* private: */
 	bool activated;
diff --git a/mm/damon/core.c b/mm/damon/core.c
index 5b325749f..b1c8ed5fd 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1380,12 +1380,17 @@ static bool kdamond_need_stop(struct damon_ctx *ctx)
 	return true;
 }
 
-static unsigned long damos_wmark_metric_value(enum damos_wmark_metric metric)
+static unsigned long damos_wmark_metric_value(struct damos_watermarks wmarks)
 {
-	switch (metric) {
+	switch (wmarks.metric) {
 	case DAMOS_WMARK_FREE_MEM_RATE:
 		return global_zone_page_state(NR_FREE_PAGES) * 1000 /
 		       totalram_pages();
+	case DAMOS_WMARK_NODE_FREE_MEM_RATE:
+		if (wmarks.target_nid == NUMA_NO_NODE)
+			return 0;
+		return sum_zone_node_page_state(wmarks.target_nid, NR_FREE_PAGES) *
+			1000 / node_present_pages(wmarks.target_nid);
 	default:
 		break;
 	}
@@ -1403,7 +1408,7 @@ static unsigned long damos_wmark_wait_us(struct damos *scheme)
 	if (scheme->wmarks.metric == DAMOS_WMARK_NONE)
 		return 0;
 
-	metric = damos_wmark_metric_value(scheme->wmarks.metric);
+	metric = damos_wmark_metric_value(scheme->wmarks);
 	/* higher than high watermark or lower than low watermark */
 	if (metric > scheme->wmarks.high || scheme->wmarks.low > metric) {
 		if (scheme->wmarks.activated)
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index ae0f0b314..b65291e4a 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -656,11 +656,13 @@ struct damon_sysfs_watermarks {
 	unsigned long high;
 	unsigned long mid;
 	unsigned long low;
+	int target_nid;
 };
 
 static struct damon_sysfs_watermarks *damon_sysfs_watermarks_alloc(
 		enum damos_wmark_metric metric, unsigned long interval_us,
-		unsigned long high, unsigned long mid, unsigned long low)
+		unsigned long high, unsigned long mid, unsigned long low,
+		int target_nid)
 {
 	struct damon_sysfs_watermarks *watermarks = kmalloc(
 			sizeof(*watermarks), GFP_KERNEL);
@@ -673,6 +675,7 @@ static struct damon_sysfs_watermarks *damon_sysfs_watermarks_alloc(
 	watermarks->high = high;
 	watermarks->mid = mid;
 	watermarks->low = low;
+	watermarks->target_nid = target_nid;
 	return watermarks;
 }
 
@@ -680,6 +683,7 @@ static struct damon_sysfs_watermarks *damon_sysfs_watermarks_alloc(
 static const char * const damon_sysfs_wmark_metric_strs[] = {
 	"none",
 	"free_mem_rate",
+	"node_free_mem_rate",
 };
 
 static ssize_t metric_show(struct kobject *kobj, struct kobj_attribute *attr,
@@ -784,6 +788,27 @@ static ssize_t low_store(struct kobject *kobj,
 	return err ? err : count;
 }
 
+static ssize_t target_nid_show(struct kobject *kobj,
+		struct kobj_attribute *attr, char *buf)
+{
+	struct damon_sysfs_watermarks *watermarks = container_of(kobj,
+			struct damon_sysfs_watermarks, kobj);
+
+	return sysfs_emit(buf, "%d\n", watermarks->target_nid);
+}
+
+static ssize_t target_nid_store(struct kobject *kobj,
+		struct kobj_attribute *attr, const char *buf, size_t count)
+{
+	struct damon_sysfs_watermarks *watermarks = container_of(kobj,
+			struct damon_sysfs_watermarks, kobj);
+	int err = kstrtoint(buf, 0, &watermarks->target_nid);
+
+	if (err)
+		return -EINVAL;
+	return count;
+}
+
 static void damon_sysfs_watermarks_release(struct kobject *kobj)
 {
 	kfree(container_of(kobj, struct damon_sysfs_watermarks, kobj));
@@ -804,12 +829,16 @@ static struct kobj_attribute damon_sysfs_watermarks_mid_attr =
 static struct kobj_attribute damon_sysfs_watermarks_low_attr =
 		__ATTR_RW_MODE(low, 0600);
 
+static struct kobj_attribute damon_sysfs_watermarks_target_nid_attr =
+		__ATTR_RW_MODE(target_nid, 0600);
+
 static struct attribute *damon_sysfs_watermarks_attrs[] = {
 	&damon_sysfs_watermarks_metric_attr.attr,
 	&damon_sysfs_watermarks_interval_us_attr.attr,
 	&damon_sysfs_watermarks_high_attr.attr,
 	&damon_sysfs_watermarks_mid_attr.attr,
 	&damon_sysfs_watermarks_low_attr.attr,
+	&damon_sysfs_watermarks_target_nid_attr.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(damon_sysfs_watermarks);
@@ -1471,7 +1500,7 @@ static int damon_sysfs_scheme_set_quotas(struct damon_sysfs_scheme *scheme)
 static int damon_sysfs_scheme_set_watermarks(struct damon_sysfs_scheme *scheme)
 {
 	struct damon_sysfs_watermarks *watermarks =
-		damon_sysfs_watermarks_alloc(DAMOS_WMARK_NONE, 0, 0, 0, 0);
+		damon_sysfs_watermarks_alloc(DAMOS_WMARK_NONE, 0, 0, 0, 0, NUMA_NO_NODE);
 	int err;
 
 	if (!watermarks)
@@ -1951,6 +1980,7 @@ static struct damos *damon_sysfs_mk_scheme(
 		.high = sysfs_wmarks->high,
 		.mid = sysfs_wmarks->mid,
 		.low = sysfs_wmarks->low,
+		.target_nid = sysfs_wmarks->target_nid,
 	};
 
 	damos_sysfs_set_quota_score(sysfs_quotas->goals, &quota);
@@ -2002,6 +2032,7 @@ static void damon_sysfs_update_scheme(struct damos *scheme,
 	scheme->wmarks.high = sysfs_wmarks->high;
 	scheme->wmarks.mid = sysfs_wmarks->mid;
 	scheme->wmarks.low = sysfs_wmarks->low;
+	scheme->wmarks.target_nid = sysfs_wmarks->target_nid;
 
 	err = damon_sysfs_set_scheme_filters(scheme, sysfs_scheme->filters);
 	if (err)
-- 
2.34.1

[RFC PATCH v2 2/3] mm/damon: add module parameters for NUMA system

[Jonghyeon Kim]

This patch implements support NUMA-aware DAMON modules by adding kernel
module parameters. With this patch, DAMON_RECLAIM and DAMON_LRU_SORT can
use this feature.

Signed-off-by: Jonghyeon Kim <tome01@ajou.ac.kr>
---
 mm/damon/lru_sort.c       | 1 +
 mm/damon/modules-common.h | 4 +++-
 mm/damon/reclaim.c        | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 3de2916a6..3e25a52de 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -81,6 +81,7 @@ static struct damos_watermarks damon_lru_sort_wmarks = {
 	.high = 200,		/* 20 percent */
 	.mid = 150,		/* 15 percent */
 	.low = 50,		/* 5 percent */
+	.target_nid = NUMA_NO_NODE,
 };
 DEFINE_DAMON_MODULES_WMARKS_PARAMS(damon_lru_sort_wmarks);
 
diff --git a/mm/damon/modules-common.h b/mm/damon/modules-common.h
index f49cdb417..b167bb24a 100644
--- a/mm/damon/modules-common.h
+++ b/mm/damon/modules-common.h
@@ -27,11 +27,13 @@
 	module_param_named(quota_sz, quota.sz, ulong, 0600);
 
 #define DEFINE_DAMON_MODULES_WMARKS_PARAMS(wmarks)			\
+	module_param_named(metric, wmarks.metric, uint, 0600);	\
 	module_param_named(wmarks_interval, wmarks.interval, ulong,	\
 			0600);						\
 	module_param_named(wmarks_high, wmarks.high, ulong, 0600);	\
 	module_param_named(wmarks_mid, wmarks.mid, ulong, 0600);	\
-	module_param_named(wmarks_low, wmarks.low, ulong, 0600);
+	module_param_named(wmarks_low, wmarks.low, ulong, 0600);	\
+	module_param_named(target_nid, wmarks.target_nid, int, 0600);
 
 #define DEFINE_DAMON_MODULES_DAMOS_STATS_PARAMS(stat, try_name,		\
 		succ_name, qt_exceed_name)				\
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 66e190f03..31b8830a8 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -68,6 +68,7 @@ static struct damos_watermarks damon_reclaim_wmarks = {
 	.high = 500,		/* 50 percent */
 	.mid = 400,		/* 40 percent */
 	.low = 200,		/* 20 percent */
+	.target_nid = NUMA_NO_NODE,
 };
 DEFINE_DAMON_MODULES_WMARKS_PARAMS(damon_reclaim_wmarks);
 
-- 
2.34.1

[RFC PATCH v2 3/3] mm/damon: add NUMA-awareness to DAMON modules

[Jonghyeon Kim]

Using the 'target_nid' parameter of DAMOS watermark, both DAMON_RECLAIM and
DAMON_LRU_SORT modules can set the monitoring range to a specific NUMA
memory node.

Signed-off-by: Jonghyeon Kim <tome01@ajou.ac.kr>
---
 mm/damon/lru_sort.c | 13 +++++++++++++
 mm/damon/reclaim.c  | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 3e25a52de..ed136ccba 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -238,6 +238,19 @@ static int damon_lru_sort_apply_parameters(void)
 	damon_set_schemes(ctx, &hot_scheme, 1);
 	damon_add_scheme(ctx, cold_scheme);
 
+	/* Set monitoring range from target node */
+	if (hot_scheme->wmarks.metric == DAMOS_WMARK_NODE_FREE_MEM_RATE) {
+		int target_nid = hot_scheme->wmarks.target_nid;
+
+		if (target_nid == NUMA_NO_NODE)
+			return -EINVAL;
+		monitor_region_start = PFN_PHYS(node_start_pfn(target_nid));
+		monitor_region_end = PFN_PHYS(node_start_pfn(target_nid) +
+				node_present_pages(target_nid) - 1);
+		if (monitor_region_end < monitor_region_start)
+			return -EINVAL;
+	}
+
 	return damon_set_region_biggest_system_ram_default(target,
 					&monitor_region_start,
 					&monitor_region_end);
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 31b8830a8..9c0c0096b 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -192,6 +192,19 @@ static int damon_reclaim_apply_parameters(void)
 	}
 	damon_set_schemes(ctx, &scheme, 1);
 
+	/* Set monitoring range from target node */
+	if (scheme->wmarks.metric == DAMOS_WMARK_NODE_FREE_MEM_RATE) {
+		int target_nid = scheme->wmarks.target_nid;
+
+		if (target_nid == NUMA_NO_NODE)
+			return -EINVAL;
+		monitor_region_start = PFN_PHYS(node_start_pfn(target_nid));
+		monitor_region_end = PFN_PHYS(node_start_pfn(target_nid) +
+					node_present_pages(target_nid) - 1);
+		if (monitor_region_end < monitor_region_start)
+			return -EINVAL;
+	}
+
 	return damon_set_region_biggest_system_ram_default(target,
 					&monitor_region_start,
 					&monitor_region_end);
-- 
2.34.1

Re: [RFC PATCH v2 0/3] Add NUMA-aware DAMOS watermarks

[SeongJae Park]

Hi Jonghyeon,

On Mon, 20 May 2024 14:30:35 +0000 Jonghyeon Kim <tome01@ajou.ac.kr> wrote:

> Current DAMOS schemes are not considered with multiple NUMA memory nodes.
> For example, If we want to proactively reclaim memory of a one NUMA node,
> DAMON_RECLAIM has to wake up kdamond before kswapd does reclaim memory.
> However, since the DAMON watermarks are based on not a one NUMA memory
> node but total system free memory, kdamond is not waked up before invoking
> memory reclamation from kswapd of the target node.
> 
> These patches allow for DAMON to select monitoring target either total
> memory or a specific NUMA memory node.

I feel such usage could exist, but my humble brain is not clearly imagining
such realistic usage.  If you could further clarify the exampected usage, it
would be very helpful for me to better understand the intention and pros/cons
of this patchset.  Especially, I'm wondering why they would want to use the
watermark feature, rather than manually checking the metric and turning DAMON
on/off, or feeding the metric as a quota tuning goal.

> 
> ---
> Changes from RFC PATCH v1
> (https://lore.kernel.org/all/20220218102611.31895-1-tome01@ajou.ac.kr)
> - Add new metric type for NUMA node, DAMOS_WMARK_NODE_FREE_MEM_RATE
> - Drop commit about damon_start()
> - Support DAMON_LRU_SORT
> 
> Jonghyeon Kim (3):
>   mm/damon: Add new metric type and target node for watermark
>   mm/damon: add module parameters for NUMA system
>   mm/damon: add NUMA-awareness to DAMON modules

Following up to the above question, why they would want to use DAMON modules
rather than manually controlling DAMON via DAMON sysfs interface?


Thanks,
SJ

> 
>  include/linux/damon.h     | 11 +++++++++--
>  mm/damon/core.c           | 11 ++++++++---
>  mm/damon/lru_sort.c       | 14 ++++++++++++++
>  mm/damon/modules-common.h |  4 +++-
>  mm/damon/reclaim.c        | 14 ++++++++++++++
>  mm/damon/sysfs-schemes.c  | 35 +++++++++++++++++++++++++++++++++--
>  6 files changed, 81 insertions(+), 8 deletions(-)
> 
> -- 
> 2.34.1

Re: [RFC PATCH v2 0/3] Add NUMA-aware DAMOS watermarks

[Jonghyeon Kim]

On Tue, May 21, 2024 at 06:00:34PM -0700, SeongJae Park wrote:
> Hi Jonghyeon,
> 

Hi, SeongJae

> On Mon, 20 May 2024 14:30:35 +0000 Jonghyeon Kim <tome01@ajou.ac.kr> wrote:
> 
> > Current DAMOS schemes are not considered with multiple NUMA memory nodes.
> > For example, If we want to proactively reclaim memory of a one NUMA node,
> > DAMON_RECLAIM has to wake up kdamond before kswapd does reclaim memory.
> > However, since the DAMON watermarks are based on not a one NUMA memory
> > node but total system free memory, kdamond is not waked up before invoking
> > memory reclamation from kswapd of the target node.
> > 
> > These patches allow for DAMON to select monitoring target either total
> > memory or a specific NUMA memory node.
> 
> I feel such usage could exist, but my humble brain is not clearly imagining
> such realistic usage.  If you could further clarify the exampected usage, it
> would be very helpful for me to better understand the intention and pros/cons
> of this patchset.  Especially, I'm wondering why they would want to use the
> watermark feature, rather than manually checking the metric and turning DAMON
> on/off, or feeding the metric as a quota tuning goal.
> 

The goal of this patchset is to manage each NUMA memory node
individually through DAMON. Also, the main target scheme is memory
reclaim (or demotion in tiered memory). By allowing DAMON to be managed
by each NUMA node, I expect that users can easily set up memory reclaim
for each node.

Additionally, I think that a watermark for each node is an appropriate
metric for activating DAMON_RECLAIM, because the kswapd reclaim logic
also follows a watermark of free memory for each node.

There are two use cases. Let's assume two NUMA nodes are constructed of
32GB (node0) and 16GB (node1), respectively.

The first case is when using DAMON module. If users do not specify a
monitoring region, DAMON's module finds the biggest size of the two NUMA
memory nodes and designates it as the monitoring region (node0, 32GB).
Even if we want to enable DAMON_RECLAIM to node0, it does not work
proactively because the watermark works based on the total system memory
(48GB).

Similarly, if the users want to enable DAMON_RECLAIM to node1, users have
to manually designate the monitoring region as the address of node1.
Nonetheless, since DAMON still follows the default watermark
(total memory, 48GB), proactive reclaim will not work properly.

Below is an example.

# echo Y > /sys/module/damon_reclaim/parameters/enabled
# cat /sys/module/damon_reclaim/parameters/monitor_region_start
4294967296 # 0x100000000
# cat /sys/module/damon_reclaim/parameters/monitor_region_end
36507222015 # 0x87fffffff

# dmesg | grep node
...
[0.012812] Early memory node ranges
[0.012813]   node   0: [mem 0x0000000000001000-0x000000000009ffff]
[0.012815]   node   0: [mem 0x0000000000100000-0x000000005e22dfff]
[0.012817]   node   0: [mem 0x000000005e62c000-0x0000000069835fff]
[0.012818]   node   0: [mem 0x000000006f2d3000-0x000000006f7fffff] 
[0.012819]   node   0: [mem 0x0000000100000000-0x000000087fffffff] < target 
[0.012825]   node   1: [mem 0x0000002800000000-0x0000002bffffffff] 
...

When we use DAMON_RECLAIM by default, DAMON_RECLAIM targets node0
memory (32GB). However, DAMON runs differently from the initial goal
because the watermark works based on the combined node0 and node1(48GB).
DAMON_LRU_SORT also faces the same situation.

The second case is when we apply DAMON to a process. If a process
allocates memory that exceeds a single NUMA node(node0), some users
could want to reclaim the cold memory of the process in that node. In my
humble opinion, the reclaim scheme(DAMOS_PAGEOUT) is effective in this
case.  Unlike the DAMON module, since DAMON monitors process memory
using a virtual address, it is hard to decide whether to enable a
DAMOS_PAGEOUT due to a lack of node memory stats. Even though we use
watermarks for DAMOS_PAGEOUT, it works the same with the above module
case (thresholds based on total memory, 48GB). To overcome this problem,
I think the dedicated watermark (for node0) can be an answer. 

> > 
> > ---
> > Changes from RFC PATCH v1
> > (https://lore.kernel.org/all/20220218102611.31895-1-tome01@ajou.ac.kr)
> > - Add new metric type for NUMA node, DAMOS_WMARK_NODE_FREE_MEM_RATE
> > - Drop commit about damon_start()
> > - Support DAMON_LRU_SORT
> > 
> > Jonghyeon Kim (3):
> >   mm/damon: Add new metric type and target node for watermark
> >   mm/damon: add module parameters for NUMA system
> >   mm/damon: add NUMA-awareness to DAMON modules
> 
> Following up to the above question, why they would want to use DAMON modules
> rather than manually controlling DAMON via DAMON sysfs interface?

IMHO, some users want to use DAMON feature without mannualy
configurating via DAMON sysfs due to complexity.  Since this patchset
can be adopted to sysfs interface, I will update supporting NUMA-aware
watermarks for sysfs interface in the next version.

Best Regards,
Jonghyeon

> 
> 
> Thanks,
> SJ
> 
> > 
> >  include/linux/damon.h     | 11 +++++++++--
> >  mm/damon/core.c           | 11 ++++++++---
> >  mm/damon/lru_sort.c       | 14 ++++++++++++++
> >  mm/damon/modules-common.h |  4 +++-
> >  mm/damon/reclaim.c        | 14 ++++++++++++++
> >  mm/damon/sysfs-schemes.c  | 35 +++++++++++++++++++++++++++++++++--
> >  6 files changed, 81 insertions(+), 8 deletions(-)
> > 
> > -- 
> > 2.34.1

Re: [RFC PATCH v2 0/3] Add NUMA-aware DAMOS watermarks

[SeongJae Park]

Hi Jonghyeon,


Thank you for the detailed explanations!  Please read my comments below.

On Fri, 24 May 2024 14:24:28 +0900 Jonghyeon Kim <tome01@ajou.ac.kr> wrote:

> On Tue, May 21, 2024 at 06:00:34PM -0700, SeongJae Park wrote:
> > Hi Jonghyeon,
> > 
> 
> Hi, SeongJae
> 
> > On Mon, 20 May 2024 14:30:35 +0000 Jonghyeon Kim <tome01@ajou.ac.kr> wrote:
> > 
> > > Current DAMOS schemes are not considered with multiple NUMA memory nodes.
> > > For example, If we want to proactively reclaim memory of a one NUMA node,
> > > DAMON_RECLAIM has to wake up kdamond before kswapd does reclaim memory.
> > > However, since the DAMON watermarks are based on not a one NUMA memory
> > > node but total system free memory, kdamond is not waked up before invoking
> > > memory reclamation from kswapd of the target node.
> > > 
> > > These patches allow for DAMON to select monitoring target either total
> > > memory or a specific NUMA memory node.
> > 
> > I feel such usage could exist, but my humble brain is not clearly imagining
> > such realistic usage.  If you could further clarify the exampected usage, it
> > would be very helpful for me to better understand the intention and pros/cons
> > of this patchset.  Especially, I'm wondering why they would want to use the
> > watermark feature, rather than manually checking the metric and turning DAMON
> > on/off, or feeding the metric as a quota tuning goal.
> > 
> 
> The goal of this patchset is to manage each NUMA memory node
> individually through DAMON. Also, the main target scheme is memory
> reclaim (or demotion in tiered memory). By allowing DAMON to be managed
> by each NUMA node, I expect that users can easily set up memory reclaim
> for each node.
> 
> Additionally, I think that a watermark for each node is an appropriate
> metric for activating DAMON_RECLAIM, because the kswapd reclaim logic
> also follows a watermark of free memory for each node.
> 
> There are two use cases. Let's assume two NUMA nodes are constructed of
> 32GB (node0) and 16GB (node1), respectively.
> 
> The first case is when using DAMON module. If users do not specify a
> monitoring region, DAMON's module finds the biggest size of the two NUMA
> memory nodes and designates it as the monitoring region (node0, 32GB).
> Even if we want to enable DAMON_RECLAIM to node0, it does not work
> proactively because the watermark works based on the total system memory
> (48GB).
> 
> Similarly, if the users want to enable DAMON_RECLAIM to node1, users have
> to manually designate the monitoring region as the address of node1.
> Nonetheless, since DAMON still follows the default watermark
> (total memory, 48GB), proactive reclaim will not work properly.
> 
> Below is an example.
> 
> # echo Y > /sys/module/damon_reclaim/parameters/enabled
> # cat /sys/module/damon_reclaim/parameters/monitor_region_start
> 4294967296 # 0x100000000
> # cat /sys/module/damon_reclaim/parameters/monitor_region_end
> 36507222015 # 0x87fffffff
> 
> # dmesg | grep node
> ...
> [0.012812] Early memory node ranges
> [0.012813]   node   0: [mem 0x0000000000001000-0x000000000009ffff]
> [0.012815]   node   0: [mem 0x0000000000100000-0x000000005e22dfff]
> [0.012817]   node   0: [mem 0x000000005e62c000-0x0000000069835fff]
> [0.012818]   node   0: [mem 0x000000006f2d3000-0x000000006f7fffff] 
> [0.012819]   node   0: [mem 0x0000000100000000-0x000000087fffffff] < target 
> [0.012825]   node   1: [mem 0x0000002800000000-0x0000002bffffffff] 
> ...
> 
> When we use DAMON_RECLAIM by default, DAMON_RECLAIM targets node0
> memory (32GB). However, DAMON runs differently from the initial goal
> because the watermark works based on the combined node0 and node1(48GB).
> DAMON_LRU_SORT also faces the same situation.
> 
> The second case is when we apply DAMON to a process. If a process
> allocates memory that exceeds a single NUMA node(node0), some users
> could want to reclaim the cold memory of the process in that node. In my
> humble opinion,áthe reclaim scheme(DAMOS_PAGEOUT) is effective in this
> case.  Unlike the DAMON module, since DAMON monitors process memory
> using a virtual address, it is hard to decide whether to enable a
> DAMOS_PAGEOUT due to a lack of node memory stats. Even though we use
> watermarks for DAMOS_PAGEOUT, it works the same with the above module
> case (thresholds based on total memory, 48GB). To overcome this problem,
> I think the dedicated watermark (for node0) can be an answer. 

Thank you very much for the detailed explanation.  I think quotas might be the
better appraoch, though, for below reasons.

We initially designed quotas to control DAMOS (schemes-execution part)
overhead, and watermarks to further eliminate DAMON (monitoring part) overhead.
That is, when every scheme is deactivated by watermarks, DAMON does only
watermarks check every user-specified time interval (5 seconds by default for
DAMON_RECLAIM), so the overhead is effectively zero.

However, this causes some problems on monitoring results quality.  For example,
let's say DAMON_RECLAIM was deactivated by watermarks, and reactivated after
long time, say, one hour.  DAMON_RECLAIM will start work with the monitoring
results that made one hour ago, which may not really reflecting current access
pattern.

We also found some users are not actively using the watermarks feature.  They
set mid watermark pretty high so that DAMON_RECLAIM almost always be active.  I
haven't met DAMON_RECLAIM users who relying on watermarks so far.  Your use
case sounds reasonable, but I guess that's still not production use case?
Please let me know if you are!

For the reason, nowadays I think it might better to consider deprecating
watermarks, and let users use quotas instead.  Quotas is more flexible, and it
recently got feedback-driven auto-tuning feature that allows user feeding it
with arbitrary metrics.  When it receives negative feedback only, it reduces
quota down to zero, so it can effectively deactivate DAMOS.

Of course, quotas cannot eliminate the monitoring part overhead.  However,
DAMON allows users set upperbound monmitoring overhead.  And in many use cases,
I feel users are setting the overhead unnecessarily high.  For example, we
usually show 2-3% single CPU time with DAMON's default setups (5ms sampling
interval, 10-1000 number of regions).  In many use cases, I think 5ms sampling
interval is too small.  Especially for DAMON_RECLAIM, because the 'min_age'
parameter is two minutes by default (I found some users who using DAMON_RECLAIM
on production is setting it much larger), increasing sampling interval wouldn't
make big degradation.  How much we can increase?  Depends on the use case, but
I think 20x (100ms) or even 200x (1s) is ok.  In the case, DAMON's
monitoring-part overhead would be only about 0.1-0.15% or 0.01-0.015% single
CPU time, so it wouldn't be real problem.  We are planning to add more
enhancement on the overhead/accuracy control, though.

So, what do you think about handling the NUMA use case via quotas?
Specifically, users could provide NUMA-specific free memory rate as arbitrary
user feedback for DAMON_RECLAIM (quota_autotune_feedback parameter).  If such
manual feeding is not desirable, we could add NUMA-specific free memory rate as
another self-feed metric, like quota_mem_pressure_us parameter.

[...]
> > Following up to the above question, why they would want to use DAMON modules
> > rather than manually controlling DAMON via DAMON sysfs interface?
> 
> IMHO, some users want to use DAMON feature without mannualy
> configurating via DAMON sysfs due to complexity.

I agree.  But also for same reason, I try to be careful at adding more
parameters for the modules.

> Since this patchset can be adopted to sysfs interface, I will update
> supporting NUMA-aware watermarks for sysfs interface in the next version.

DAMON sysfs interface aims to provide full features for users who prefer having
control plane on the user-space.  So, yes, please add that.  Further, I
wouldn't mind having the knob on only DAMON sysfs interface.


Thanks,
SJ

[...]