[PATCH v3] mm/damon/stat: monitor all System RAM resources

[PATCH v3] mm/damon/stat: monitor all System RAM resources

[SeongJae Park]

DAMON_STAT usage document (Documentation/admin-guide/mm/damon/stat.rst)
says it monitors the system's entire physical memory.  But, it is
monitoring only the biggest System RAM resource of the system.  When
there are multiple System RAM resources, this results in monitoring only
an unexpectedly small fraction of the physical memory.  For example,
suppose the system has a 500 GiB System RAM, 10 MiB non-System RAM, and
500 GiB System RAM resources in order on the physical address space.
DAMON_STAT will monitor only the first 500 GiB System RAM.  This
situation is particularly common on NUMA systems.

Select a physical address range that covers all System RAM areas of the
system, to fix this issue and make it work as documented.

Fixes: 369c415e6073 ("mm/damon: introduce DAMON_STAT module")
Cc: <stable@vger.kernel.org> # 6.17.x
Signed-off-by: SeongJae Park <sj@kernel.org>
---
Changes from v2
(https://lore.kernel.org/20260315162717.80870-1-sj@kernel.org)
- Rebase to mm-hotfixes-unstable
Changes from v1
(https://lore.kernel.org/20260313044449.4038-1-sj@kernel.org)
- Fix wrong argument for damon_set_regions().

 mm/damon/stat.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index 25fb44ccf99d0..c5af8ad4bcb65 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -145,12 +145,57 @@ static int damon_stat_damon_call_fn(void *data)
 	return 0;
 }
 
+struct damon_stat_system_ram_range_walk_arg {
+	bool walked;
+	struct resource res;
+};
+
+static int damon_stat_system_ram_walk_fn(struct resource *res, void *arg)
+{
+	struct damon_stat_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_stat_res_to_core_addr(resource_size_t ra,
+		unsigned long addr_unit)
+{
+	/*
+	 * Use div_u64() for avoiding linking errors related with __udivdi3,
+	 * __aeabi_uldivmod, or similar problems.  This should also improve the
+	 * performance optimization (read div_u64() comment for the detail).
+	 */
+	if (sizeof(ra) == 8 && sizeof(addr_unit) == 4)
+		return div_u64(ra, addr_unit);
+	return ra / addr_unit;
+}
+
+static int damon_stat_set_monitoring_region(struct damon_target *t,
+		unsigned long addr_unit, unsigned long min_region_sz)
+{
+	struct damon_addr_range addr_range;
+	struct damon_stat_system_ram_range_walk_arg arg = {};
+
+	walk_system_ram_res(0, -1, &arg, damon_stat_system_ram_walk_fn);
+	if (!arg.walked)
+		return -EINVAL;
+	addr_range.start = damon_stat_res_to_core_addr(
+			arg.res.start, addr_unit);
+	addr_range.end = damon_stat_res_to_core_addr(
+			arg.res.end + 1, addr_unit);
+	return damon_set_regions(t, &addr_range, 1, min_region_sz);
+}
+
 static struct damon_ctx *damon_stat_build_ctx(void)
 {
 	struct damon_ctx *ctx;
 	struct damon_attrs attrs;
 	struct damon_target *target;
-	unsigned long start = 0, end = 0;
 
 	ctx = damon_new_ctx();
 	if (!ctx)
@@ -180,8 +225,8 @@ static struct damon_ctx *damon_stat_build_ctx(void)
 	if (!target)
 		goto free_out;
 	damon_add_target(ctx, target);
-	if (damon_set_region_biggest_system_ram_default(target, &start, &end,
-							ctx->min_region_sz))
+	if (damon_stat_set_monitoring_region(target, ctx->addr_unit,
+				ctx->min_region_sz))
 		goto free_out;
 	return ctx;
 free_out:

base-commit: 10125b79cb0e7037fff6cd8cc9fafacfda8113a0
-- 
2.47.3

Re: [PATCH v3] mm/damon/stat: monitor all System RAM resources

[SeongJae Park]

TL; DR: sashiko.dev found a problem in this patch.  Andrew, could you please
add the attached fixup, or let me know if you prefer v4?

sashiko.dev added [1] comments.  Quoting the comments with ': ' line prefix,
and replying to it in line below.

On Mon, 16 Mar 2026 16:51:17 -0700 SeongJae Park <sj@kernel.org> wrote:

> DAMON_STAT usage document (Documentation/admin-guide/mm/damon/stat.rst)
> says it monitors the system's entire physical memory.  But, it is
> monitoring only the biggest System RAM resource of the system.  When
> there are multiple System RAM resources, this results in monitoring only
> an unexpectedly small fraction of the physical memory.  For example,
> suppose the system has a 500 GiB System RAM, 10 MiB non-System RAM, and
> 500 GiB System RAM resources in order on the physical address space.
> DAMON_STAT will monitor only the first 500 GiB System RAM.  This
> situation is particularly common on NUMA systems.
> 
> Select a physical address range that covers all System RAM areas of the
> system, to fix this issue and make it work as documented.
> 
> Fixes: 369c415e6073 ("mm/damon: introduce DAMON_STAT module")
> Cc: <stable@vger.kernel.org> # 6.17.x
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
> Changes from v2
> (https://lore.kernel.org/20260315162717.80870-1-sj@kernel.org)
> - Rebase to mm-hotfixes-unstable
> Changes from v1
> (https://lore.kernel.org/20260313044449.4038-1-sj@kernel.org)
> - Fix wrong argument for damon_set_regions().
> 
>  mm/damon/stat.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 48 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/damon/stat.c b/mm/damon/stat.c
> index 25fb44ccf99d0..c5af8ad4bcb65 100644
> --- a/mm/damon/stat.c
> +++ b/mm/damon/stat.c
> @@ -145,12 +145,57 @@ static int damon_stat_damon_call_fn(void *data)
>  	return 0;
>  }
>  
> +struct damon_stat_system_ram_range_walk_arg {
> +	bool walked;
> +	struct resource res;
> +};
> +
> +static int damon_stat_system_ram_walk_fn(struct resource *res, void *arg)
> +{
> +	struct damon_stat_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;
> +}

: Does creating a single bounding box from the start of the first RAM resource
: to the end of the last one force DAMON to monitor massive unpopulated
: physical memory gaps?
: 
: On NUMA systems, memory nodes are often separated by large holes. Passing
: this single large range to damon_set_regions() might spread max_nr_regions
: over these holes, which could dilute the accuracy of the memory bandwidth
: statistics since sampling non-RAM addresses yields zero accesses.
: 
: Would it be better to dynamically allocate an array of struct
: damon_addr_range covering only valid System RAM boundaries and pass the
: multiple ranges to damon_set_regions() using the nr_ranges parameter?

Yes, it can force DAMON to monitor massive unpopulated phyiscal memory gaps.
But, because DAMON utilizes its adaptive regions adjustment mechanism, the
overhead will be negligible.  This is an intended behavior.

> +
> +static unsigned long damon_stat_res_to_core_addr(resource_size_t ra,
> +		unsigned long addr_unit)
> +{
> +	/*
> +	 * Use div_u64() for avoiding linking errors related with __udivdi3,
> +	 * __aeabi_uldivmod, or similar problems.  This should also improve the
> +	 * performance optimization (read div_u64() comment for the detail).
> +	 */
> +	if (sizeof(ra) == 8 && sizeof(addr_unit) == 4)
> +		return div_u64(ra, addr_unit);
> +	return ra / addr_unit;
> +}

: Could this silently truncate physical addresses above 4GB on 32-bit systems
: with PAE enabled?
: 
: The function walk_system_ram_res() scans the 64-bit physical address space,
: and resource_size_t can be a 64-bit integer, accurately holding addresses
: above 4GB. However, this function returns an unsigned long, and DAMON's
: struct damon_addr_range uses unsigned long for its boundaries.
: 
: Since damon_stat uses an addr_unit of 1, any 64-bit physical address might
: be truncated to 32 bits when returned. This could create an inverted region
: where the truncated end becomes smaller than start, breaking the
: damon_set_regions() initialization and causing out-of-bounds memory accesses.

This is correct, and the consequence is problematic.

> +
> +static int damon_stat_set_monitoring_region(struct damon_target *t,
> +		unsigned long addr_unit, unsigned long min_region_sz)
> +{
> +	struct damon_addr_range addr_range;
> +	struct damon_stat_system_ram_range_walk_arg arg = {};
> +
> +	walk_system_ram_res(0, -1, &arg, damon_stat_system_ram_walk_fn);
> +	if (!arg.walked)
> +		return -EINVAL;
> +	addr_range.start = damon_stat_res_to_core_addr(
> +			arg.res.start, addr_unit);
> +	addr_range.end = damon_stat_res_to_core_addr(
> +			arg.res.end + 1, addr_unit);


: Is it possible for arg.res.end + 1 to overflow here?
: 
: On 32-bit architectures without PAE, resource_size_t is a 32-bit unsigned
: integer. If physical RAM extends to the very top of the 32-bit address
: space, arg.res.end will be 0xFFFFFFFF.
: 
: Adding 1 to this value triggers an integer overflow, resulting in 0, which
: is then assigned to addr_range.end. Passing a region with a start greater
: than 0 and an end of 0 to damon_set_regions() creates a structurally invalid
: inverted region, which might corrupt subsequent DAMON calculations and
: sampling operations.

Again, this is correct, and the consequence is problematic.

We can avoid the problematic consequence by checking if addr_range is valid,
and return an error if not.  Below attaching fixup patch is implementing it.
Andrew, could you please add the fixup patch?  Pleae let me know if you prefer
v4.


Thanks,
SJ

[...]
=== >8 ===
From e1fd7cf6469977cf1a58ee98b977eec53219ffc2 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Mon, 16 Mar 2026 22:11:22 -0700
Subject: [PATCH] mm/damon/stat: return error if monitoring target region is
 invalid

On 32bit systems with LPAE, the end address might be overflowed or
truncated, resulting in invalid range (the end address is equal or
smaller than the start address).  Return an error for the case.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/stat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index c5af8ad4bcb65..cf2c5a541eeea 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -188,6 +188,8 @@ static int damon_stat_set_monitoring_region(struct damon_target *t,
 			arg.res.start, addr_unit);
 	addr_range.end = damon_stat_res_to_core_addr(
 			arg.res.end + 1, addr_unit);
+	if (addr_range.end <= addr_range.start)
+		return -EINVAL;
 	return damon_set_regions(t, &addr_range, 1, min_region_sz);
 }
 
-- 
2.47.3