[PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions

[PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions

[Jiayuan Chen]

From: Jiayuan Chen <jiayuan.chen@shopee.com>

damon_set_region_system_rams_default(), introduced by commit 70d8797c15d6
("mm/damon: introduce damon_set_region_system_rams_default()"), is used by
DAMON_RECLAIM, DAMON_LRU_SORT and DAMON_STAT to set the default monitoring
target address range covering all 'System RAM' when the user does not
specify a range.  It walks the 'System RAM' resources but keeps only the
start of the first resource and the end of the last one, and then sets a
single monitoring region spanning that whole [first_start, last_end] range.

On systems whose RAM is split into discrete areas that are far apart in the
physical address space, that single region also covers the holes between
them.  For example:

  $ sudo cat /proc/iomem | grep RAM
  00001000-0009ffff : System RAM
  00100000-4848c017 : System RAM
  4848c018-48550c57 : System RAM
  48550c58-48551017 : System RAM
  48551018-48615c57 : System RAM
  48615c58-48616017 : System RAM
  48616018-486dac57 : System RAM
  486dac58-4e563017 : System RAM
  4e563018-4e627c57 : System RAM
  4e627c58-4ef39017 : System RAM
  4ef39018-4ef3f057 : System RAM
  4ef3f058-4efe6017 : System RAM
  4efe6018-4efec057 : System RAM
  4efec058-50247fff : System RAM
  50317000-56720fff : System RAM
  56722000-59c19fff : System RAM
  6bbfe000-6bbfefff : System RAM
  6bc00000-777fffff : System RAM
  100000000-1007effffff : System RAM
  67e80000000-77e7fffffff : System RAM

Here the last two areas (about 1TB starting at 4GiB, and about 1.1TB
starting at ~6.5TB) are separated by a ~5.5TB hole, and the single-region
setup makes DAMON treat that entire hole as if it were memory.

This is harmful in a few ways.  The monitoring target regions are limited
by max_nr_regions, so regions that fall into the hole waste that budget and
leave fewer regions for the real RAM, coarsening the adaptive regions and
degrading the monitoring accuracy.  In addition, DAMOS actions on the paddr
operations set walk such a region page by page, so a region that covers the
hole is walked for its entire (empty) span on every application.

Set a separate monitoring region for each discrete System RAM area instead,
coalescing only truly adjacent (no gap in between) resources into one
range, so holes between the areas are excluded.  The reported *start and
*end still carry the overall first-start and last-end, so the user-visible
default range reported via the module parameters is unchanged.

Cc: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
---
 mm/damon/core.c | 116 ++++++++++++++++++++++++++++++++++--------------
 1 file changed, 82 insertions(+), 34 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 760559d495b3..8dd4a0d22064 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -4394,22 +4394,14 @@ static int kdamond_fn(void *data)
 }
 
 struct damon_system_ram_range_walk_arg {
-	bool walked;
-	struct resource res;
+	unsigned long addr_unit;
+	/* NULL in the counting-only pass */
+	struct damon_addr_range *ranges;
+	unsigned int nr_ranges;
+	unsigned long prev_end;
+	bool has_prev;
 };
 
-static int damon_system_ram_walk_fn(struct resource *res, void *arg)
-{
-	struct damon_system_ram_range_walk_arg *a = arg;
-
-	if (!a->walked) {
-		a->walked = true;
-		a->res.start = res->start;
-	}
-	a->res.end = res->end;
-	return 0;
-}
-
 static unsigned long damon_res_to_core_addr(resource_size_t ra,
 		unsigned long addr_unit)
 {
@@ -4423,23 +4415,66 @@ static unsigned long damon_res_to_core_addr(resource_size_t ra,
 	return ra / addr_unit;
 }
 
-static bool damon_find_system_rams_range(unsigned long *start,
-		unsigned long *end, unsigned long addr_unit)
+static int damon_system_ram_walk_fn(struct resource *res, void *arg)
+{
+	struct damon_system_ram_range_walk_arg *a = arg;
+	unsigned long start = damon_res_to_core_addr(res->start, a->addr_unit);
+	unsigned long end = damon_res_to_core_addr(res->end + 1, a->addr_unit);
+
+	if (end <= start)
+		return 0;
+	/*
+	 * 'System RAM' resources are visited in the ascending address order.
+	 * Coalesce only truly adjacent (no gap in between) resources into one
+	 * range, so that any hole between discrete System RAM areas is kept out
+	 * of the resulting ranges.
+	 */
+	if (a->has_prev && a->prev_end == start) {
+		a->prev_end = end;
+		if (a->ranges)
+			a->ranges[a->nr_ranges - 1].end = end;
+		return 0;
+	}
+	if (a->ranges) {
+		a->ranges[a->nr_ranges].start = start;
+		a->ranges[a->nr_ranges].end = end;
+	}
+	a->nr_ranges++;
+	a->prev_end = end;
+	a->has_prev = true;
+	return 0;
+}
+
+/*
+ * Find all 'System RAM' areas and return them as an array of coalesced
+ * damon_addr_range.  On success, *ranges points to a kvmalloc'ed array that the
+ * caller should kvfree(), and the number of ranges is returned.  Returns 0 if
+ * no System RAM is found, or a negative error code on failure.
+ */
+static int damon_find_system_rams(struct damon_addr_range **ranges_out,
+				  unsigned long addr_unit)
 {
-	struct damon_system_ram_range_walk_arg arg = {};
+	struct damon_system_ram_range_walk_arg arg = { .addr_unit = addr_unit };
+	struct damon_addr_range *ranges;
 
+	/* First pass: count the coalesced ranges. */
 	walk_system_ram_res(0, -1, &arg, damon_system_ram_walk_fn);
-	if (!arg.walked)
-		return false;
-	*start = damon_res_to_core_addr(arg.res.start, addr_unit);
-	*end = damon_res_to_core_addr(arg.res.end + 1, addr_unit);
-	if (*end <= *start)
-		return false;
-	return true;
+	if (!arg.nr_ranges)
+		return 0;
+	ranges = kvmalloc_objs(*ranges, arg.nr_ranges, GFP_KERNEL);
+	if (!ranges)
+		return -ENOMEM;
+	/* Second pass: fill in the coalesced ranges. */
+	arg.ranges = ranges;
+	arg.nr_ranges = 0;
+	arg.has_prev = false;
+	walk_system_ram_res(0, -1, &arg, damon_system_ram_walk_fn);
+	*ranges_out = ranges;
+	return arg.nr_ranges;
 }
 
 /**
- * damon_set_region_system_rams_default() - Set the region of the given
+ * damon_set_region_system_rams_default() - Set the regions of the given
  * monitoring target as requested, or to cover all 'System RAM' resources.
  * @t:		The monitoring target to set the region.
  * @start:	The pointer to the start address of the region.
@@ -4449,9 +4484,11 @@ static bool damon_find_system_rams_range(unsigned long *start,
  *
  * This function sets the region of @t as requested by @start and @end.  If the
  * values of @start and @end are zero, however, this function finds 'System
- * RAM' resources and sets the region to cover all the resource.  In the latter
- * case, this function saves the start and the end addresseses of the first and
- * the last resources in @start and @end, respectively.
+ * RAM' resources and sets the monitoring target regions to cover them.  Each
+ * discrete System RAM area becomes a separate region, so holes between them
+ * (e.g., on multi-socket or CXL systems) are excluded from the monitoring.  In
+ * the latter case, this function saves the start and the end addresses of the
+ * first and the last resources in @start and @end, respectively.
  *
  * Return: 0 on success, negative error code otherwise.
  */
@@ -4460,14 +4497,25 @@ int damon_set_region_system_rams_default(struct damon_target *t,
 			unsigned long addr_unit, unsigned long min_region_sz)
 {
 	struct damon_addr_range addr_range;
+	struct damon_addr_range *ranges;
+	int nr_ranges, err;
 
-	if (!*start && !*end &&
-		!damon_find_system_rams_range(start, end, addr_unit))
-		return -EINVAL;
+	if (*start || *end) {
+		addr_range.start = *start;
+		addr_range.end = *end;
+		return damon_set_regions(t, &addr_range, 1, min_region_sz);
+	}
 
-	addr_range.start = *start;
-	addr_range.end = *end;
-	return damon_set_regions(t, &addr_range, 1, min_region_sz);
+	nr_ranges = damon_find_system_rams(&ranges, addr_unit);
+	if (nr_ranges < 0)
+		return nr_ranges;
+	if (!nr_ranges)
+		return -EINVAL;
+	*start = ranges[0].start;
+	*end = ranges[nr_ranges - 1].end;
+	err = damon_set_regions(t, ranges, nr_ranges, min_region_sz);
+	kvfree(ranges);
+	return err;
 }
 
 /**
-- 
2.43.0

[PATCH 2/2] mm/damon/sysfs-schemes: report the number of tried regions

[Jiayuan Chen]

From: Jiayuan Chen <jiayuan.chen@shopee.com>

The 'tried_regions' directory of each DAMON sysfs scheme exposes the memory
regions that the scheme's action has been tried to be applied to, as
per-region subdirectories.  It also has a 'total_bytes' file that reports
the total size of those regions without materializing the per-region
subdirectories, so that users can cheaply retrieve the aggregated result.

The number of the tried regions is another useful aggregated metric.  When
the scheme's access pattern is not restrictive, it approximates the number
of the adaptive monitoring regions of the context, which users may want to
watch, e.g., to see how well the monitoring is refined under a given
max_nr_regions, or to feed fleet wide access pattern dashboards.
Retrieving it currently requires materializing all the per-region
subdirectories (via writing 'update_schemes_tried_regions') and counting
them, which is unnecessarily expensive for users that only need the count.

Add a 'nr_regions' file to the 'tried_regions' directory.  Like
'total_bytes', it is updated by both 'update_schemes_tried_bytes' and
'update_schemes_tried_regions', so it can be read as a lightweight counter
without materializing the per-region subdirectories.

Suggested-by: SeongJae Park <sj@kernel.org>
Cc: Jiayuan Chen <jiayuan.chen@linux.dev>
Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
---
 .../ABI/testing/sysfs-kernel-mm-damon         | 10 +++++++
 Documentation/admin-guide/mm/damon/usage.rst  | 27 ++++++++++---------
 mm/damon/sysfs-schemes.c                      | 23 +++++++++++++---
 3 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-kernel-mm-damon b/Documentation/ABI/testing/sysfs-kernel-mm-damon
index 786be4537da1..a6e8f5036555 100644
--- a/Documentation/ABI/testing/sysfs-kernel-mm-damon
+++ b/Documentation/ABI/testing/sysfs-kernel-mm-damon
@@ -622,6 +622,16 @@ Description:	Writing a number to this file sets the upper limit of
 		nr_snapshots that deactivates the scheme when the limit is
 		reached or exceeded.
 
+What:		/sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/schemes/<S>/tried_regions/nr_regions
+Date:		Aug 2026
+Contact:	SJ Park <sj@kernel.org>
+Description:	Reading this file returns the number of regions that
+		corresponding DAMON-based Operation Scheme's action has tried
+		to be applied.  The number is updated together with
+		'.../tried_regions/total_bytes' by writing 'update_schemes_tried_bytes'
+		or 'update_schemes_tried_regions' to the relevant 'state' file, and
+		hence can be read without materializing the per-region directories.
+
 What:		/sys/kernel/mm/damon/admin/kdamonds/<K>/contexts/<C>/schemes/<S>/tried_regions/total_bytes
 Date:		Jul 2023
 Contact:	SJ Park <sj@kernel.org>
diff --git a/Documentation/admin-guide/mm/damon/usage.rst b/Documentation/admin-guide/mm/damon/usage.rst
index f6048fc04263..9152ce09de9d 100644
--- a/Documentation/admin-guide/mm/damon/usage.rst
+++ b/Documentation/admin-guide/mm/damon/usage.rst
@@ -108,7 +108,7 @@ comma (",").
     │ │ │ │ │ │ │ :ref:`dests <damon_sysfs_dests>`/nr_dests
     │ │ │ │ │ │ │ │ 0/id,weight
     │ │ │ │ │ │ │ :ref:`stats <sysfs_schemes_stats>`/nr_tried,sz_tried,nr_applied,sz_applied,sz_ops_filter_passed,qt_exceeds,nr_snapshots,max_nr_snapshots
-    │ │ │ │ │ │ │ :ref:`tried_regions <sysfs_schemes_tried_regions>`/total_bytes
+    │ │ │ │ │ │ │ :ref:`tried_regions <sysfs_schemes_tried_regions>`/nr_regions,total_bytes
     │ │ │ │ │ │ │ │ 0/start,end,nr_accesses,age,sz_filter_passed
     │ │ │ │ │ │ │ │ │ probes
     │ │ │ │ │ │ │ │ │ │ 0/hits
@@ -671,21 +671,24 @@ relevant ``kdamonds/<N>/state`` file.  Refer to :ref:`kdamond directory
 schemes/<N>/tried_regions/
 --------------------------
 
-This directory initially has one file, ``total_bytes``.
+This directory initially has two files, ``nr_regions`` and ``total_bytes``.
 
 When a special keyword, ``update_schemes_tried_regions``, is written to the
-relevant ``kdamonds/<N>/state`` file, DAMON updates the ``total_bytes`` file so
-that reading it returns the total size of the scheme tried regions, and creates
-directories named integer starting from ``0`` under this directory.  Each
-directory contains files exposing detailed information about each of the memory
-region that the corresponding scheme's ``action`` has tried to be applied under
-this directory, during next :ref:`apply interval <damon_design_damos>` of the
-corresponding scheme.  The information includes address range, ``nr_accesses``,
-and ``age`` of the region.
+relevant ``kdamonds/<N>/state`` file, DAMON updates the ``nr_regions`` and
+``total_bytes`` files so that reading them returns the number and the total size
+of the scheme tried regions, respectively, and creates directories named
+integer starting from ``0`` under this directory.  Each directory contains files
+exposing detailed information about each of the memory region that the
+corresponding scheme's ``action`` has tried to be applied under this directory,
+during next :ref:`apply interval <damon_design_damos>` of the corresponding
+scheme.  The information includes address range, ``nr_accesses``, and ``age`` of
+the region.
 
 Writing ``update_schemes_tried_bytes`` to the relevant ``kdamonds/<N>/state``
-file will only update the ``total_bytes`` file, and will not create the
-subdirectories.
+file will only update the ``nr_regions`` and ``total_bytes`` files, and will not
+create the subdirectories.  Hence ``nr_regions`` can be used as a lightweight way
+to read the number of the scheme tried regions without materializing the
+per-region subdirectories.
 
 The directories will be removed when another special keyword,
 ``clear_schemes_tried_regions``, is written to the relevant
diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
index 54ff196f8e24..b1df9f21c267 100644
--- a/mm/damon/sysfs-schemes.c
+++ b/mm/damon/sysfs-schemes.c
@@ -297,6 +297,7 @@ static const struct kobj_type damon_sysfs_scheme_region_ktype = {
 struct damon_sysfs_scheme_regions {
 	struct kobject kobj;
 	struct list_head regions_list;
+	int nr_region_dirs;
 	int nr_regions;
 	unsigned long total_bytes;
 };
@@ -311,11 +312,21 @@ damon_sysfs_scheme_regions_alloc(void)
 
 	regions->kobj = (struct kobject){};
 	INIT_LIST_HEAD(&regions->regions_list);
+	regions->nr_region_dirs = 0;
 	regions->nr_regions = 0;
 	regions->total_bytes = 0;
 	return regions;
 }
 
+static ssize_t nr_regions_show(struct kobject *kobj,
+			       struct kobj_attribute *attr, char *buf)
+{
+	struct damon_sysfs_scheme_regions *regions = container_of(kobj,
+			struct damon_sysfs_scheme_regions, kobj);
+
+	return sysfs_emit(buf, "%d\n", regions->nr_regions);
+}
+
 static ssize_t total_bytes_show(struct kobject *kobj,
 		struct kobj_attribute *attr, char *buf)
 {
@@ -335,7 +346,7 @@ static void damon_sysfs_scheme_regions_rm_dirs(
 		list_del(&r->list);
 		kobject_del(&r->kobj);
 		kobject_put(&r->kobj);
-		regions->nr_regions--;
+		regions->nr_region_dirs--;
 	}
 }
 
@@ -344,10 +355,14 @@ static void damon_sysfs_scheme_regions_release(struct kobject *kobj)
 	kfree(container_of(kobj, struct damon_sysfs_scheme_regions, kobj));
 }
 
+static struct kobj_attribute damon_sysfs_scheme_regions_nr_regions_attr =
+		__ATTR_RO_MODE(nr_regions, 0400);
+
 static struct kobj_attribute damon_sysfs_scheme_regions_total_bytes_attr =
 		__ATTR_RO_MODE(total_bytes, 0400);
 
 static struct attribute *damon_sysfs_scheme_regions_attrs[] = {
+	&damon_sysfs_scheme_regions_nr_regions_attr.attr,
 	&damon_sysfs_scheme_regions_total_bytes_attr.attr,
 	NULL,
 };
@@ -3133,6 +3148,7 @@ void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes,
 		return;
 
 	sysfs_regions = sysfs_schemes->schemes_arr[schemes_idx]->tried_regions;
+	sysfs_regions->nr_regions++;
 	sysfs_regions->total_bytes += r->ar.end - r->ar.start;
 	if (total_bytes_only)
 		return;
@@ -3144,13 +3160,13 @@ void damos_sysfs_populate_region_dir(struct damon_sysfs_schemes *sysfs_schemes,
 	if (kobject_init_and_add(&region->kobj,
 				&damon_sysfs_scheme_region_ktype,
 				&sysfs_regions->kobj, "%d",
-				sysfs_regions->nr_regions))
+				sysfs_regions->nr_region_dirs))
 		goto out;
 	if (damos_sysfs_region_add_dirs(region, ctx, r))
 		goto del_out;
 
 	list_add_tail(&region->list, &sysfs_regions->regions_list);
-	sysfs_regions->nr_regions++;
+	sysfs_regions->nr_region_dirs++;
 	return;
 
 del_out:
@@ -3170,6 +3186,7 @@ int damon_sysfs_schemes_clear_regions(
 		sysfs_scheme = sysfs_schemes->schemes_arr[i];
 		damon_sysfs_scheme_regions_rm_dirs(
 				sysfs_scheme->tried_regions);
+		sysfs_scheme->tried_regions->nr_regions = 0;
 		sysfs_scheme->tried_regions->total_bytes = 0;
 	}
 	return 0;
-- 
2.43.0

Re: [PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Time-of-check to time-of-use (TOCTOU) race condition in `damon_find_system_rams()` leads to a heap out-of-bounds write during concurrent memory hotplug.
- [High] DAMON initialization fails with `-EINVAL` on systems with sub-page System RAM gaps due to alignment overlap in `damon_set_regions()`.
--

--- Patch [1]: [PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions ---
commit 20fb01bf6e8e4cefc6016dfc073fb77c95ca3511
Author: Jiayuan Chen <jiayuan.chen@shopee.com>

mm/damon/core: cover discrete System RAM areas with per-range regions

This commit changes how DAMON sets default monitoring regions for System RAM.
Instead of a single region covering all RAM (and the holes between), it now
creates separate regions for each discrete System RAM area. This excludes
large memory holes from being monitored.

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 644daf5a16560..365c43271c0c6 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> +static int damon_find_system_rams(struct damon_addr_range **ranges_out,
> +				  unsigned long addr_unit)
> +{
> +	struct damon_system_ram_range_walk_arg arg = { .addr_unit = addr_unit };
> +	struct damon_addr_range *ranges;
>  
> +	/* First pass: count the coalesced ranges. */
>  	walk_system_ram_res(0, -1, &arg, damon_system_ram_walk_fn);
> -	if (!arg.walked)
> -		return false;
> -	*start = damon_res_to_core_addr(arg.res.start, addr_unit);
> -	*end = damon_res_to_core_addr(arg.res.end + 1, addr_unit);
> -	if (*end <= *start)
> -		return false;
> -	return true;
> +	if (!arg.nr_ranges)
> +		return 0;
> +	ranges = kvmalloc_objs(*ranges, arg.nr_ranges, GFP_KERNEL);
> +	if (!ranges)
> +		return -ENOMEM;
> +	/* Second pass: fill in the coalesced ranges. */
> +	arg.ranges = ranges;
> +	arg.nr_ranges = 0;
> +	arg.has_prev = false;
> +	walk_system_ram_res(0, -1, &arg, damon_system_ram_walk_fn);

[Severity: High]
Can this race with memory hotplug events and cause an out-of-bounds write?

walk_system_ram_res() is called twice without holding memory hotplug locks.
If a new System RAM region is hotplugged between or during the second pass,
the number of visited regions could exceed the allocated buffer size.

If that happens, damon_system_ram_walk_fn() would write past the end of the
kvmalloc-allocated buffer:

mm/damon/core.c:damon_system_ram_walk_fn() {
	...
	if (a->ranges) {
		a->ranges[a->nr_ranges].start = start;
		a->ranges[a->nr_ranges].end = end;
	}
	a->nr_ranges++;
	...
}

> +	*ranges_out = ranges;
> +	return arg.nr_ranges;
>  }

[ ... ]

> @@ -3984,14 +4021,25 @@ int damon_set_region_system_rams_default(struct damon_target *t,
>  			unsigned long addr_unit, unsigned long min_region_sz)
>  {
>  	struct damon_addr_range addr_range;
> +	struct damon_addr_range *ranges;
> +	int nr_ranges, err;
>  
> -	if (!*start && !*end &&
> -		!damon_find_system_rams_range(start, end, addr_unit))
> -		return -EINVAL;
> +	if (*start || *end) {
> +		addr_range.start = *start;
> +		addr_range.end = *end;
> +		return damon_set_regions(t, &addr_range, 1, min_region_sz);
> +	}
>  
> -	addr_range.start = *start;
> -	addr_range.end = *end;
> -	return damon_set_regions(t, &addr_range, 1, min_region_sz);
> +	nr_ranges = damon_find_system_rams(&ranges, addr_unit);
> +	if (nr_ranges < 0)
> +		return nr_ranges;
> +	if (!nr_ranges)
> +		return -EINVAL;
> +	*start = ranges[0].start;
> +	*end = ranges[nr_ranges - 1].end;
> +	err = damon_set_regions(t, ranges, nr_ranges, min_region_sz);

[Severity: High]
Could this fail with -EINVAL during initialization on systems with sub-page
memory gaps?

If the system has System RAM fragmented by small non-RAM holes (like sub-page
BIOS or EFI reserved regions), damon_system_ram_walk_fn() will create separate
discrete regions.

When these regions are passed to damon_set_regions(), it aligns the boundaries
to min_region_sz (typically PAGE_SIZE):

mm/damon/core.c:damon_set_regions() {
	for (i = 0; i < nr_ranges; i++) {
		...
		start = ALIGN_DOWN(ranges[i].start, min_region_sz);
		end = ALIGN(ranges[i].end, min_region_sz);
		if (start >= end)
			return -EINVAL;
		if (i > 0 && last_end > start)
			return -EINVAL;
		last_end = end;
	}
	...
}

If the gap between two RAM areas is smaller than PAGE_SIZE, the ALIGN of the
first region's end will evaluate to a higher address than the ALIGN_DOWN of
the second region's start. Does this cause the strictly-ascending overlap
check (last_end > start) to trigger and reject valid configurations?

> +	kvfree(ranges);
> +	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727095429.143527-1-jiayuan.chen@linux.dev?part=1

Re: [PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions

[Jiayuan Chen]

On 7/27/26 6:07 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Time-of-check to time-of-use (TOCTOU) race condition in `damon_find_system_rams()` leads to a heap out-of-bounds write during concurrent memory hotplug.
> - [High] DAMON initialization fails with `-EINVAL` on systems with sub-page System RAM gaps due to alignment overlap in `damon_set_regions()`.
> --
>
> --- Patch [1]: [PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions ---
> commit 20fb01bf6e8e4cefc6016dfc073fb77c95ca3511
> Author: Jiayuan Chen <jiayuan.chen@shopee.com>
>
> mm/damon/core: cover discrete System RAM areas with per-range regions
>
> This commit changes how DAMON sets default monitoring regions for System RAM.
> Instead of a single region covering all RAM (and the holes between), it now
> creates separate regions for each discrete System RAM area. This excludes
> large memory holes from being monitored.
>
>> diff --git a/mm/damon/core.c b/mm/damon/core.c
>> index 644daf5a16560..365c43271c0c6 100644
>> --- a/mm/damon/core.c
>> +++ b/mm/damon/core.c
> [ ... ]
>
>> +static int damon_find_system_rams(struct damon_addr_range **ranges_out,
>> +				  unsigned long addr_unit)
>> +{
>> +	struct damon_system_ram_range_walk_arg arg = { .addr_unit = addr_unit };
>> +	struct damon_addr_range *ranges;
>>   
>> +	/* First pass: count the coalesced ranges. */
>>   	walk_system_ram_res(0, -1, &arg, damon_system_ram_walk_fn);
>> -	if (!arg.walked)
>> -		return false;
>> -	*start = damon_res_to_core_addr(arg.res.start, addr_unit);
>> -	*end = damon_res_to_core_addr(arg.res.end + 1, addr_unit);
>> -	if (*end <= *start)
>> -		return false;
>> -	return true;
>> +	if (!arg.nr_ranges)
>> +		return 0;
>> +	ranges = kvmalloc_objs(*ranges, arg.nr_ranges, GFP_KERNEL);
>> +	if (!ranges)
>> +		return -ENOMEM;
>> +	/* Second pass: fill in the coalesced ranges. */
>> +	arg.ranges = ranges;
>> +	arg.nr_ranges = 0;
>> +	arg.has_prev = false;
>> +	walk_system_ram_res(0, -1, &arg, damon_system_ram_walk_fn);
> [Severity: High]
> Can this race with memory hotplug events and cause an out-of-bounds write?
>
> walk_system_ram_res() is called twice without holding memory hotplug locks.
> If a new System RAM region is hotplugged between or during the second pass,
> the number of visited regions could exceed the allocated buffer size.
>
> If that happens, damon_system_ram_walk_fn() would write past the end of the
> kvmalloc-allocated buffer:
>
> mm/damon/core.c:damon_system_ram_walk_fn() {
> 	...
> 	if (a->ranges) {
> 		a->ranges[a->nr_ranges].start = start;
> 		a->ranges[a->nr_ranges].end = end;
> 	}
> 	a->nr_ranges++;
> 	...
> }

You're right. The two walk_system_ram_res() calls should be wrapped with

get_online_mems()/put_online_mems() so the resource set cannot change

between the counting and filling passes.



>> +	*ranges_out = ranges;
>> +	return arg.nr_ranges;
>>   }
> [ ... ]
>
>> @@ -3984,14 +4021,25 @@ int damon_set_region_system_rams_default(struct damon_target *t,
>>   			unsigned long addr_unit, unsigned long min_region_sz)
>>   {
>>   	struct damon_addr_range addr_range;
>> +	struct damon_addr_range *ranges;
>> +	int nr_ranges, err;
>>   
>> -	if (!*start && !*end &&
>> -		!damon_find_system_rams_range(start, end, addr_unit))
>> -		return -EINVAL;
>> +	if (*start || *end) {
>> +		addr_range.start = *start;
>> +		addr_range.end = *end;
>> +		return damon_set_regions(t, &addr_range, 1, min_region_sz);
>> +	}
>>   
>> -	addr_range.start = *start;
>> -	addr_range.end = *end;
>> -	return damon_set_regions(t, &addr_range, 1, min_region_sz);
>> +	nr_ranges = damon_find_system_rams(&ranges, addr_unit);
>> +	if (nr_ranges < 0)
>> +		return nr_ranges;
>> +	if (!nr_ranges)
>> +		return -EINVAL;
>> +	*start = ranges[0].start;
>> +	*end = ranges[nr_ranges - 1].end;
>> +	err = damon_set_regions(t, ranges, nr_ranges, min_region_sz);
> [Severity: High]
> Could this fail with -EINVAL during initialization on systems with sub-page
> memory gaps?
>
> If the system has System RAM fragmented by small non-RAM holes (like sub-page
> BIOS or EFI reserved regions), damon_system_ram_walk_fn() will create separate
> discrete regions.
>
> When these regions are passed to damon_set_regions(), it aligns the boundaries
> to min_region_sz (typically PAGE_SIZE):
>
> mm/damon/core.c:damon_set_regions() {
> 	for (i = 0; i < nr_ranges; i++) {
> 		...
> 		start = ALIGN_DOWN(ranges[i].start, min_region_sz);
> 		end = ALIGN(ranges[i].end, min_region_sz);
> 		if (start >= end)
> 			return -EINVAL;
> 		if (i > 0 && last_end > start)
> 			return -EINVAL;
> 		last_end = end;
> 	}
> 	...
> }
>
> If the gap between two RAM areas is smaller than PAGE_SIZE, the ALIGN of the
> first region's end will evaluate to a higher address than the ALIGN_DOWN of
> the second region's start. Does this cause the strictly-ascending overlap
> check (last_end > start) to trigger and reject valid configurations?


this is a real issue. I overlooked the case where System RAM boundaries 
are not page-size aligned


>> +	kvfree(ranges);
>> +	return err;
>>   }

Re: [PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions

[SJ Park]

Hello Jiayuan,

On Mon, 27 Jul 2026 17:54:22 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:

> From: Jiayuan Chen <jiayuan.chen@shopee.com>
> 
> damon_set_region_system_rams_default(), introduced by commit 70d8797c15d6
> ("mm/damon: introduce damon_set_region_system_rams_default()"), is used by
> DAMON_RECLAIM, DAMON_LRU_SORT and DAMON_STAT to set the default monitoring
> target address range covering all 'System RAM' when the user does not
> specify a range.  It walks the 'System RAM' resources but keeps only the
> start of the first resource and the end of the last one, and then sets a
> single monitoring region spanning that whole [first_start, last_end] range.
> 
> On systems whose RAM is split into discrete areas that are far apart in the
> physical address space, that single region also covers the holes between
> them.  For example:
> 
>   $ sudo cat /proc/iomem | grep RAM
>   00001000-0009ffff : System RAM
>   00100000-4848c017 : System RAM
>   4848c018-48550c57 : System RAM
>   48550c58-48551017 : System RAM
>   48551018-48615c57 : System RAM
>   48615c58-48616017 : System RAM
>   48616018-486dac57 : System RAM
>   486dac58-4e563017 : System RAM
>   4e563018-4e627c57 : System RAM
>   4e627c58-4ef39017 : System RAM
>   4ef39018-4ef3f057 : System RAM
>   4ef3f058-4efe6017 : System RAM
>   4efe6018-4efec057 : System RAM
>   4efec058-50247fff : System RAM
>   50317000-56720fff : System RAM
>   56722000-59c19fff : System RAM
>   6bbfe000-6bbfefff : System RAM
>   6bc00000-777fffff : System RAM
>   100000000-1007effffff : System RAM
>   67e80000000-77e7fffffff : System RAM
> 
> Here the last two areas (about 1TB starting at 4GiB, and about 1.1TB
> starting at ~6.5TB) are separated by a ~5.5TB hole, and the single-region
> setup makes DAMON treat that entire hole as if it were memory.
> 
> This is harmful in a few ways.  The monitoring target regions are limited
> by max_nr_regions, so regions that fall into the hole waste that budget and
> leave fewer regions for the real RAM, coarsening the adaptive regions and
> degrading the monitoring accuracy.

The hole would look like not accessed.  As a result, the whole region will be a
few regions that very cold.  That wouldn't waste the budget that much.  Do you
have some specific setups that this cannot help?

> In addition, DAMOS actions on the paddr
> operations set walk such a region page by page, so a region that covers the
> hole is walked for its entire (empty) span on every application.

That makes sense.

> 
> Set a separate monitoring region for each discrete System RAM area instead,
> coalescing only truly adjacent (no gap in between) resources into one
> range, so holes between the areas are excluded.  The reported *start and
> *end still carry the overall first-start and last-end, so the user-visible
> default range reported via the module parameters is unchanged.

I'm concerned if this could result in having too many regions.  The gap between
user-visible parameters and internal state is also a concern.

A quick workaround would be adjusting the memory layout in BIOS, using DAMON
sysfs interface instead, or setting the monitor_region_{start,end} to cover
only the single area.  Have you considered such workarounds?

Let's complete this high level discussion first.


Thanks,
SJ

[...]

Re: [PATCH 2/2] mm/damon/sysfs-schemes: report the number of tried regions

[SJ Park]

Hello Jiayuan,

On Mon, 27 Jul 2026 17:54:23 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:

> From: Jiayuan Chen <jiayuan.chen@shopee.com>
> 
> The 'tried_regions' directory of each DAMON sysfs scheme exposes the memory
> regions that the scheme's action has been tried to be applied to, as
> per-region subdirectories.  It also has a 'total_bytes' file that reports
> the total size of those regions without materializing the per-region
> subdirectories, so that users can cheaply retrieve the aggregated result.
> 
> The number of the tried regions is another useful aggregated metric.  When
> the scheme's access pattern is not restrictive, it approximates the number
> of the adaptive monitoring regions of the context, which users may want to
> watch, e.g., to see how well the monitoring is refined under a given
> max_nr_regions, or to feed fleet wide access pattern dashboards.
> Retrieving it currently requires materializing all the per-region
> subdirectories (via writing 'update_schemes_tried_regions') and counting
> them, which is unnecessarily expensive for users that only need the count.

I agree the number can be useful.

> 
> Add a 'nr_regions' file to the 'tried_regions' directory.  Like
> 'total_bytes', it is updated by both 'update_schemes_tried_bytes' and
> 'update_schemes_tried_regions', so it can be read as a lightweight counter
> without materializing the per-region subdirectories.

You could also get a similar information from DAMOS stat, specifically via
nr_tried stat.  Have you considered using that?

I'm holding reviewing detail before this high level discussion is done.


Thanks,
SJ

[...]

Re: [PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions

[Jiayuan Chen]

On 7/27/26 10:26 PM, SJ Park wrote:
> Hello Jiayuan,
>
> On Mon, 27 Jul 2026 17:54:22 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>>
>> damon_set_region_system_rams_default(), introduced by commit 70d8797c15d6
>> ("mm/damon: introduce damon_set_region_system_rams_default()"), is used by
>> DAMON_RECLAIM, DAMON_LRU_SORT and DAMON_STAT to set the default monitoring
>> target address range covering all 'System RAM' when the user does not
>> specify a range.  It walks the 'System RAM' resources but keeps only the
>> start of the first resource and the end of the last one, and then sets a
>> single monitoring region spanning that whole [first_start, last_end] range.
>>
>> On systems whose RAM is split into discrete areas that are far apart in the
>> physical address space, that single region also covers the holes between
>> them.  For example:
>>
>>    $ sudo cat /proc/iomem | grep RAM
>>    00001000-0009ffff : System RAM
>>    00100000-4848c017 : System RAM
>>    4848c018-48550c57 : System RAM
>>    48550c58-48551017 : System RAM
>>    48551018-48615c57 : System RAM
>>    48615c58-48616017 : System RAM
>>    48616018-486dac57 : System RAM
>>    486dac58-4e563017 : System RAM
>>    4e563018-4e627c57 : System RAM
>>    4e627c58-4ef39017 : System RAM
>>    4ef39018-4ef3f057 : System RAM
>>    4ef3f058-4efe6017 : System RAM
>>    4efe6018-4efec057 : System RAM
>>    4efec058-50247fff : System RAM
>>    50317000-56720fff : System RAM
>>    56722000-59c19fff : System RAM
>>    6bbfe000-6bbfefff : System RAM
>>    6bc00000-777fffff : System RAM
>>    100000000-1007effffff : System RAM
>>    67e80000000-77e7fffffff : System RAM
>>
>> Here the last two areas (about 1TB starting at 4GiB, and about 1.1TB
>> starting at ~6.5TB) are separated by a ~5.5TB hole, and the single-region
>> setup makes DAMON treat that entire hole as if it were memory.
>>
>> This is harmful in a few ways.  The monitoring target regions are limited
>> by max_nr_regions, so regions that fall into the hole waste that budget and
>> leave fewer regions for the real RAM, coarsening the adaptive regions and
>> degrading the monitoring accuracy.
> The hole would look like not accessed.  As a result, the whole region will be a
> few regions that very cold.  That wouldn't waste the budget that much.  Do you
> have some specific setups that this cannot help?

It's my mistake.

I first saw the kdamond CPU go up, and I assumed it was the number of
regions. I was wrong.

I re-tested and profiled it with perf. It's not the region count — it's
the page-by-page walk of the hole.

On a VM with a 116GiB hole and a single [first,last] span, I enabled
DAMON_LRU_SORT and ran perf on its kdamond:
   30.15%  damon_get_folio
   17.09%  pfn_to_online_page
    2.24%  __nr_to_section
    ...
    0.07%  damon_split_region_at
    0.06%  damon_merge_two_regions

Almost all of it is the per-page folio lookup. Merge and split are ~0.1%.
The cold scheme targets cold, old regions. The hole is never accessed, so
it's always cold and old, and it matches. Then the action walks its whole
empty span page by page, every apply. RECLAIM and LRU_SORT do this by
default. It scales with the hole size, so on the real 5.5TiB machine the
kdamond can't keep up.


>
>> In addition, DAMOS actions on the paddr
>> operations set walk such a region page by page, so a region that covers the
>> hole is walked for its entire (empty) span on every application.
> That makes sense.
>
>> Set a separate monitoring region for each discrete System RAM area instead,
>> coalescing only truly adjacent (no gap in between) resources into one
>> range, so holes between the areas are excluded.  The reported *start and
>> *end still carry the overall first-start and last-end, so the user-visible
>> default range reported via the module parameters is unchanged.
> I'm concerned if this could result in having too many regions.  The gap between

On the region count: can't we just coalesce any hole smaller than
min_region_sz?  After that, the number of ranges is just the number of 
discrete
System RAM areas

> user-visible parameters and internal state is also a concern.


I think monitor_region_start/end is meant to expose the overall range, and
skipping the holes inside it is an implementation detail. Even if we didn't
skip them, the adaptive region count is never 1 anyway — DAMON already
splits [first,last] into many regions. The holes just add a few more, so
the param never matched the internal state exactly to begin with.


> A quick workaround would be adjusting the memory layout in BIOS, using DAMON
> sysfs interface instead, or setting the monitor_region_{start,end} to cover
> only the single area.  Have you considered such workarounds?
>
> Let's complete this high level discussion first.


Right, this is doable today — the DAMON sysfs interface can set multiple

regions manually. This patch is only about convenience.


>
> Thanks,
> SJ
>
> [...]

Re: [PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions

[SJ Park]

On Tue, 28 Jul 2026 18:06:48 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:

> 
> On 7/27/26 10:26 PM, SJ Park wrote:
> > Hello Jiayuan,
> >
> > On Mon, 27 Jul 2026 17:54:22 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> >
> >> From: Jiayuan Chen <jiayuan.chen@shopee.com>
> >>
> >> damon_set_region_system_rams_default(), introduced by commit 70d8797c15d6
> >> ("mm/damon: introduce damon_set_region_system_rams_default()"), is used by
> >> DAMON_RECLAIM, DAMON_LRU_SORT and DAMON_STAT to set the default monitoring
> >> target address range covering all 'System RAM' when the user does not
> >> specify a range.  It walks the 'System RAM' resources but keeps only the
> >> start of the first resource and the end of the last one, and then sets a
> >> single monitoring region spanning that whole [first_start, last_end] range.
> >>
> >> On systems whose RAM is split into discrete areas that are far apart in the
> >> physical address space, that single region also covers the holes between
> >> them.  For example:
> >>
> >>    $ sudo cat /proc/iomem | grep RAM
> >>    00001000-0009ffff : System RAM
> >>    00100000-4848c017 : System RAM
> >>    4848c018-48550c57 : System RAM
> >>    48550c58-48551017 : System RAM
> >>    48551018-48615c57 : System RAM
> >>    48615c58-48616017 : System RAM
> >>    48616018-486dac57 : System RAM
> >>    486dac58-4e563017 : System RAM
> >>    4e563018-4e627c57 : System RAM
> >>    4e627c58-4ef39017 : System RAM
> >>    4ef39018-4ef3f057 : System RAM
> >>    4ef3f058-4efe6017 : System RAM
> >>    4efe6018-4efec057 : System RAM
> >>    4efec058-50247fff : System RAM
> >>    50317000-56720fff : System RAM
> >>    56722000-59c19fff : System RAM
> >>    6bbfe000-6bbfefff : System RAM
> >>    6bc00000-777fffff : System RAM
> >>    100000000-1007effffff : System RAM
> >>    67e80000000-77e7fffffff : System RAM
> >>
> >> Here the last two areas (about 1TB starting at 4GiB, and about 1.1TB
> >> starting at ~6.5TB) are separated by a ~5.5TB hole, and the single-region
> >> setup makes DAMON treat that entire hole as if it were memory.
> >>
> >> This is harmful in a few ways.  The monitoring target regions are limited
> >> by max_nr_regions, so regions that fall into the hole waste that budget and
> >> leave fewer regions for the real RAM, coarsening the adaptive regions and
> >> degrading the monitoring accuracy.
> > The hole would look like not accessed.  As a result, the whole region will be a
> > few regions that very cold.  That wouldn't waste the budget that much.  Do you
> > have some specific setups that this cannot help?
> 
> It's my mistake.
> 
> I first saw the kdamond CPU go up, and I assumed it was the number of
> regions. I was wrong.
> 
> I re-tested and profiled it with perf. It's not the region count — it's
> the page-by-page walk of the hole.
> 
> On a VM with a 116GiB hole and a single [first,last] span, I enabled
> DAMON_LRU_SORT and ran perf on its kdamond:
>    30.15%  damon_get_folio
>    17.09%  pfn_to_online_page
>     2.24%  __nr_to_section
>     ...
>     0.07%  damon_split_region_at
>     0.06%  damon_merge_two_regions
> 
> Almost all of it is the per-page folio lookup. Merge and split are ~0.1%.
> The cold scheme targets cold, old regions. The hole is never accessed, so
> it's always cold and old, and it matches. Then the action walks its whole
> empty span page by page, every apply. RECLAIM and LRU_SORT do this by
> default. It scales with the hole size, so on the real 5.5TiB machine the
> kdamond can't keep up.

Makes sense, thank you for investigating and sharing this!

> 
> 
> >
> >> In addition, DAMOS actions on the paddr
> >> operations set walk such a region page by page, so a region that covers the
> >> hole is walked for its entire (empty) span on every application.
> > That makes sense.
> >
> >> Set a separate monitoring region for each discrete System RAM area instead,
> >> coalescing only truly adjacent (no gap in between) resources into one
> >> range, so holes between the areas are excluded.  The reported *start and
> >> *end still carry the overall first-start and last-end, so the user-visible
> >> default range reported via the module parameters is unchanged.
> > I'm concerned if this could result in having too many regions.  The gap between
> 
> On the region count: can't we just coalesce any hole smaller than
> min_region_sz?

I'm not fully understanding your point.  min_region_sz is only 4 KiB by
default, and anyway DAMON cannot create regions of size smaller than
min_region_sz.  I cannot get how this helps.

> After that, the number of ranges is just the number of 
> discrete
> System RAM areas

I'm still not convinced with this.  What if the number of discrete system ram
areas is larger than max_nr_regions?

Actually I was also thinking about this problem for in the past.  One of my
idea at that time was, handle only a few largest holes that practically being
problems.  That is, while reading the system ram layout, find the holes, sort
those by size, and do make holes in DAMON regions layout for the biggest N
(say, 2) holes.  This may handle most cases including your 5 TiB hole.
Actually vaddr is doing this, so we may be able to reuse some of the code.

If it makes sense to you, I will try to implement this.

> 
> > user-visible parameters and internal state is also a concern.
> 
> 
> I think monitor_region_start/end is meant to expose the overall range, and
> skipping the holes inside it is an implementation detail. Even if we didn't
> skip them, the adaptive region count is never 1 anyway — DAMON already
> splits [first,last] into many regions. The holes just add a few more, so
> the param never matched the internal state exactly to begin with.

It is arguable, but I believe this also makes sense in my opinion.

> 
> 
> > A quick workaround would be adjusting the memory layout in BIOS, using DAMON
> > sysfs interface instead, or setting the monitor_region_{start,end} to cover
> > only the single area.  Have you considered such workarounds?
> >
> > Let's complete this high level discussion first.
> 
> 
> Right, this is doable today — the DAMON sysfs interface can set multiple
> 
> regions manually. This patch is only about convenience.

Are you actually running DAMON_LRU_SORT or DAMON_RECLAIM in a production system
having 5 TiB hole?  Or, planning to do?  If the above idea makes sense to you,
I could prioritize implementation of it depending on this.


Thanks,
SJ

[...]

Re: [PATCH 1/2] mm/damon/core: cover discrete System RAM areas with per-range regions

[Jiayuan Chen]

On 7/28/26 10:30 PM, SJ Park wrote:
> On Tue, 28 Jul 2026 18:06:48 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>> On 7/27/26 10:26 PM, SJ Park wrote:
>>> Hello Jiayuan,
>>>
>>> On Mon, 27 Jul 2026 17:54:22 +0800 Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>>>
>>>> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>>>>
>>>> damon_set_region_system_rams_default(), introduced by commit 70d8797c15d6
>>>> ("mm/damon: introduce damon_set_region_system_rams_default()"), is used by
>>>> DAMON_RECLAIM, DAMON_LRU_SORT and DAMON_STAT to set the default monitoring
>>>> target address range covering all 'System RAM' when the user does not
>>>> specify a range.  It walks the 'System RAM' resources but keeps only the
>>>> start of the first resource and the end of the last one, and then sets a
>>>> single monitoring region spanning that whole [first_start, last_end] range.
>>>>
>>>> On systems whose RAM is split into discrete areas that are far apart in the
>>>> physical address space, that single region also covers the holes between
>>>> them.  For example:
>>>>
>>>>     $ sudo cat /proc/iomem | grep RAM
>>>>     00001000-0009ffff : System RAM
>>>>     00100000-4848c017 : System RAM
>>>>     4848c018-48550c57 : System RAM
>>>>     48550c58-48551017 : System RAM
>>>>     48551018-48615c57 : System RAM
>>>>     48615c58-48616017 : System RAM
>>>>     48616018-486dac57 : System RAM
>>>>     486dac58-4e563017 : System RAM
>>>>     4e563018-4e627c57 : System RAM
>>>>     4e627c58-4ef39017 : System RAM
>>>>     4ef39018-4ef3f057 : System RAM
>>>>     4ef3f058-4efe6017 : System RAM
>>>>     4efe6018-4efec057 : System RAM
>>>>     4efec058-50247fff : System RAM
>>>>     50317000-56720fff : System RAM
>>>>     56722000-59c19fff : System RAM
>>>>     6bbfe000-6bbfefff : System RAM
>>>>     6bc00000-777fffff : System RAM
>>>>     100000000-1007effffff : System RAM
>>>>     67e80000000-77e7fffffff : System RAM
>>>>
>>>> Here the last two areas (about 1TB starting at 4GiB, and about 1.1TB
>>>> starting at ~6.5TB) are separated by a ~5.5TB hole, and the single-region
>>>> setup makes DAMON treat that entire hole as if it were memory.
>>>>
>>>> This is harmful in a few ways.  The monitoring target regions are limited
>>>> by max_nr_regions, so regions that fall into the hole waste that budget and
>>>> leave fewer regions for the real RAM, coarsening the adaptive regions and
>>>> degrading the monitoring accuracy.
>>> The hole would look like not accessed.  As a result, the whole region will be a
>>> few regions that very cold.  That wouldn't waste the budget that much.  Do you
>>> have some specific setups that this cannot help?
>> It's my mistake.
>>
>> I first saw the kdamond CPU go up, and I assumed it was the number of
>> regions. I was wrong.
>>
>> I re-tested and profiled it with perf. It's not the region count — it's
>> the page-by-page walk of the hole.
>>
>> On a VM with a 116GiB hole and a single [first,last] span, I enabled
>> DAMON_LRU_SORT and ran perf on its kdamond:
>>     30.15%  damon_get_folio
>>     17.09%  pfn_to_online_page
>>      2.24%  __nr_to_section
>>      ...
>>      0.07%  damon_split_region_at
>>      0.06%  damon_merge_two_regions
>>
>> Almost all of it is the per-page folio lookup. Merge and split are ~0.1%.
>> The cold scheme targets cold, old regions. The hole is never accessed, so
>> it's always cold and old, and it matches. Then the action walks its whole
>> empty span page by page, every apply. RECLAIM and LRU_SORT do this by
>> default. It scales with the hole size, so on the real 5.5TiB machine the
>> kdamond can't keep up.
> Makes sense, thank you for investigating and sharing this!
>
>>
>>>> In addition, DAMOS actions on the paddr
>>>> operations set walk such a region page by page, so a region that covers the
>>>> hole is walked for its entire (empty) span on every application.
>>> That makes sense.
>>>
>>>> Set a separate monitoring region for each discrete System RAM area instead,
>>>> coalescing only truly adjacent (no gap in between) resources into one
>>>> range, so holes between the areas are excluded.  The reported *start and
>>>> *end still carry the overall first-start and last-end, so the user-visible
>>>> default range reported via the module parameters is unchanged.
>>> I'm concerned if this could result in having too many regions.  The gap between
>> On the region count: can't we just coalesce any hole smaller than
>> min_region_sz?
> I'm not fully understanding your point.  min_region_sz is only 4 KiB by
> default, and anyway DAMON cannot create regions of size smaller than
> min_region_sz.  I cannot get how this helps.
>
>> After that, the number of ranges is just the number of
>> discrete
>> System RAM areas
> I'm still not convinced with this.  What if the number of discrete system ram
> areas is larger than max_nr_regions?
>
> Actually I was also thinking about this problem for in the past.  One of my
> idea at that time was, handle only a few largest holes that practically being
> problems.  That is, while reading the system ram layout, find the holes, sort
> those by size, and do make holes in DAMON regions layout for the biggest N
> (say, 2) holes.  This may handle most cases including your 5 TiB hole.
> Actually vaddr is doing this, so we may be able to reuse some of the code.
>
> If it makes sense to you, I will try to implement this.

One worry with a fixed N is that it's a magic number. If a machine has more
than N big holes (more sockets / NUMA nodes, or several CXL devices), the
extra ones are silently not excluded and get walked again.

And making N configurable just moves the per-machine tuning back to the
user, which is exactly what I'm trying to avoid.

vaddr over-includes the gaps into its regions too, but it can skip them
cheaply: it walks the VMA tree (find_vma / maple tree), it's cheap.
paddr has no such structure, so an included hole is walked in full.

That's why a fixed N is safe for vaddr but risky for paddr.


>>> user-visible parameters and internal state is also a concern.
>>
>> I think monitor_region_start/end is meant to expose the overall range, and
>> skipping the holes inside it is an implementation detail. Even if we didn't
>> skip them, the adaptive region count is never 1 anyway — DAMON already
>> splits [first,last] into many regions. The holes just add a few more, so
>> the param never matched the internal state exactly to begin with.
> It is arguable, but I believe this also makes sense in my opinion.
>
>>
>>> A quick workaround would be adjusting the memory layout in BIOS, using DAMON
>>> sysfs interface instead, or setting the monitor_region_{start,end} to cover
>>> only the single area.  Have you considered such workarounds?
>>>
>>> Let's complete this high level discussion first.
>>
>> Right, this is doable today — the DAMON sysfs interface can set multiple
>>
>> regions manually. This patch is only about convenience.
> Are you actually running DAMON_LRU_SORT or DAMON_RECLAIM in a production system
> having 5 TiB hole?  Or, planning to do?  If the above idea makes sense to you,
> I could prioritize implementation of it depending on this.


I'm not actually running DAMON_LRU_SORT or DAMON_RECLAIM on such a machine.
I ran into this while working on per-cgroup hot/cold page tracking, where I
was comparing performance and accuracy across setups — that's where these
numbers came from.


>
> Thanks,
> SJ
>
> [...]



And to be clear, I'm not trying to land a patch here — I just noticed this
as a possible optimization. If you have a better idea, I'm happy to go with
it :).