public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
* [PATCH 0/5] mm/damon: support addr_unit on default monitoring targets for modules
@ 2026-03-11  5:29 SeongJae Park
  2026-03-11  5:29 ` [PATCH 1/5] mm/damon/core: fix wrong end address assignment on walk_system_ram() SeongJae Park
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-11  5:29 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SeongJae Park, Yang Yingliang, damon, linux-kernel, linux-mm

DAMON_RECLAIM and DAMON_LRU_SORT support 'addr_unit' parameters only
when the monitoring target address range is explicitly set.  This was
intentional for making the initial 'addr_unit' support change small.
Now 'addr_unit' support is being quite stabilized.  Having the corner
case of the support is only making the code inconsistent with implicit
rules.  The inconsistency makes it easy to confuse [1] readers.  After
all, there is no real reason to keep 'addr_unit' support incomplete.
Add the support for the case to improve the readability and more
completely support 'addr_unit'.

This series is constructed with five patches.  The first one (patch 1)
fixes a small bug that mistakenly assigns inclusive end address to open
end address, which was found from this work.  The second and third ones
(patches 2 and 3) extend the default monitoring target setting functions
in the core layer one by one, to support the 'addr_unit' while making no
visible changes.  The final two patches (patches 4 and 5) update
DAMON_RECLAIM and DAMON_LRU_SORT to support 'addr_unit' for the default
monitoring target address ranges, by passing the user input to the core
functions.

[1] https://lore.kernel.org/20260131015643.79158-1-sj@kernel.org

Changes from RFC
(https://lore.kernel.org/20260305053918.83786-1-sj@kernel.org)
- Rebase to latest mm-new.
- Wordsmith commit messages.

SeongJae Park (5):
  mm/damon/core: fix wrong end address assignment on walk_system_ram()
  mm/damon/core: support addr_unit on damon_find_biggest_system_ram()
  mm/damon/core: receive addr_unit on
    damon_set_region_biggest_system_ram_default()
  mm/damon/reclaim: respect addr_unit on default monitoring region setup
  mm/damon/lru_sort: respect addr_unit on default monitoring region
    setup

 include/linux/damon.h |  1 +
 mm/damon/core.c       | 36 +++++++++++++++++++++++++-----------
 mm/damon/lru_sort.c   |  7 +------
 mm/damon/reclaim.c    |  7 +------
 mm/damon/stat.c       |  2 +-
 5 files changed, 29 insertions(+), 24 deletions(-)


base-commit: 31ae3f4d2741372ab511aeefc5e6898d12971fcd
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/5] mm/damon/core: fix wrong end address assignment on walk_system_ram()
  2026-03-11  5:29 [PATCH 0/5] mm/damon: support addr_unit on default monitoring targets for modules SeongJae Park
@ 2026-03-11  5:29 ` SeongJae Park
  2026-03-11  5:29 ` [PATCH 2/5] mm/damon/core: support addr_unit on damon_find_biggest_system_ram() SeongJae Park
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-11  5:29 UTC (permalink / raw)
  To: Andrew Morton
  Cc: SeongJae Park, Yang Yingliang, damon, linux-kernel, linux-mm

'struct damon_addr_range' and 'struct resource' represent different
types of address ranges.  'damon_addr_range' is for end-open ranges
([start, end)).  'resource' is for fully-closed ranges ([start, end]).
But walk_system_ram() is assigning resource->end to
damon_addr_range->end without the inclusiveness adjustment.  As a
result, the function returns an address range that is missing the last
one byte.

The function is being used to find and set the biggest system ram as the
default monitoring target for DAMON_RECLAIM and DAMON_LRU_SORT.  Missing
the last byte of the big range shouldn't be a real problem for the real
use cases.  That said, the loss is definitely an unintended behavior.
Do the correct adjustment.

Fixes: 43b0536cb471 ("mm/damon: introduce DAMON-based Reclamation (DAMON_RECLAIM)")
Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 2e36024d99506..3925720a172a6 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3060,7 +3060,7 @@ static int walk_system_ram(struct resource *res, void *arg)
 
 	if (a->end - a->start < resource_size(res)) {
 		a->start = res->start;
-		a->end = res->end;
+		a->end = res->end + 1;
 	}
 	return 0;
 }
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/5] mm/damon/core: support addr_unit on damon_find_biggest_system_ram()
  2026-03-11  5:29 [PATCH 0/5] mm/damon: support addr_unit on default monitoring targets for modules SeongJae Park
  2026-03-11  5:29 ` [PATCH 1/5] mm/damon/core: fix wrong end address assignment on walk_system_ram() SeongJae Park
@ 2026-03-11  5:29 ` SeongJae Park
  2026-03-17 14:47   ` SeongJae Park
  2026-03-11  5:29 ` [PATCH 3/5] mm/damon/core: receive addr_unit on damon_set_region_biggest_system_ram_default() SeongJae Park
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-03-11  5:29 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SeongJae Park, damon, linux-kernel, linux-mm

damon_find_biggest_system_ram() sets an 'unsigned long' variable with
'resource_size_t' value.  This is fundamentally wrong.  On environments
such as ARM 32 bit machines having LPAE (Large Physical Address
Extensions), which DAMON supports, the size of 'unsigned long' may be
smaller than that of 'resource_size_t'.  It is safe, though, since we
restrict the walk to be done only up to ULONG_MAX.

DAMON supports the address size gap using 'addr_unit'.  We didn't add
the support to the function, just to make the initial support change
small.  Now the support is reasonably settled.  This kind of gap is only
making the code inconsistent and easy to be confused.  Add the support
of 'addr_unit' to the function, by letting callers pass the 'addr_unit'
and handling it in the function.  All callers are passing 'addr_unit' 1,
though, to keep the old behavior.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/core.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 3925720a172a6..aee61bf991baa 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3056,31 +3056,44 @@ static int kdamond_fn(void *data)
 
 static int walk_system_ram(struct resource *res, void *arg)
 {
-	struct damon_addr_range *a = arg;
+	struct resource *a = arg;
 
-	if (a->end - a->start < resource_size(res)) {
+	if (resource_size(a) < resource_size(res)) {
 		a->start = res->start;
-		a->end = res->end + 1;
+		a->end = res->end;
 	}
 	return 0;
 }
 
+static unsigned long damon_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;
+}
+
 /*
  * Find biggest 'System RAM' resource and store its start and end address in
  * @start and @end, respectively.  If no System RAM is found, returns false.
  */
 static bool damon_find_biggest_system_ram(unsigned long *start,
-						unsigned long *end)
+		unsigned long *end, unsigned long addr_unit)
 
 {
-	struct damon_addr_range arg = {};
+	struct resource res = {};
 
-	walk_system_ram_res(0, ULONG_MAX, &arg, walk_system_ram);
-	if (arg.end <= arg.start)
+	walk_system_ram_res(0, -1, &res, walk_system_ram);
+	if (res.end < res.start)
 		return false;
 
-	*start = arg.start;
-	*end = arg.end;
+	*start = damon_res_to_core_addr(res.start, addr_unit);
+	*end = damon_res_to_core_addr(res.end + 1, addr_unit);
 	return true;
 }
 
@@ -3110,7 +3123,7 @@ int damon_set_region_biggest_system_ram_default(struct damon_target *t,
 		return -EINVAL;
 
 	if (!*start && !*end &&
-		!damon_find_biggest_system_ram(start, end))
+		!damon_find_biggest_system_ram(start, end, 1))
 		return -EINVAL;
 
 	addr_range.start = *start;
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/5] mm/damon/core: receive addr_unit on damon_set_region_biggest_system_ram_default()
  2026-03-11  5:29 [PATCH 0/5] mm/damon: support addr_unit on default monitoring targets for modules SeongJae Park
  2026-03-11  5:29 ` [PATCH 1/5] mm/damon/core: fix wrong end address assignment on walk_system_ram() SeongJae Park
  2026-03-11  5:29 ` [PATCH 2/5] mm/damon/core: support addr_unit on damon_find_biggest_system_ram() SeongJae Park
@ 2026-03-11  5:29 ` SeongJae Park
  2026-03-14  0:18   ` SeongJae Park
  2026-03-11  5:29 ` [PATCH 4/5] mm/damon/reclaim: respect addr_unit on default monitoring region setup SeongJae Park
  2026-03-11  5:29 ` [PATCH 5/5] mm/damon/lru_sort: " SeongJae Park
  4 siblings, 1 reply; 8+ messages in thread
From: SeongJae Park @ 2026-03-11  5:29 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SeongJae Park, damon, linux-kernel, linux-mm

damon_find_biggest_system_ram() was not supporting addr_unit in the
past.  Hence, its caller, damon_set_region_biggest_system_ram_default(),
was also not supporting addr_unit.  The previous commit has updated the
inner function to support addr_unit.  There is no more reason to not
support addr_unit on damon_set_region_biggest_system_ram_default().
Rather, it makes unnecessary inconsistency on support of addr_unit.
Update it to receive addr_unit and handle it inside.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 include/linux/damon.h | 1 +
 mm/damon/core.c       | 7 ++++---
 mm/damon/lru_sort.c   | 1 +
 mm/damon/reclaim.c    | 1 +
 mm/damon/stat.c       | 2 +-
 5 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/linux/damon.h b/include/linux/damon.h
index 1130c2f9a92f4..3a441fbca170d 100644
--- a/include/linux/damon.h
+++ b/include/linux/damon.h
@@ -988,6 +988,7 @@ int damos_walk(struct damon_ctx *ctx, struct damos_walk_control *control);
 
 int damon_set_region_biggest_system_ram_default(struct damon_target *t,
 				unsigned long *start, unsigned long *end,
+				unsigned long addr_unit,
 				unsigned long min_region_sz);
 
 #endif	/* CONFIG_DAMON */
diff --git a/mm/damon/core.c b/mm/damon/core.c
index aee61bf991baa..d4f86c20b4f48 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3103,6 +3103,7 @@ static bool damon_find_biggest_system_ram(unsigned long *start,
  * @t:		The monitoring target to set the region.
  * @start:	The pointer to the start address of the region.
  * @end:	The pointer to the end address of the region.
+ * @addr_unit:	The address unit for the damon_ctx of @t.
  * @min_region_sz:	Minimum region size.
  *
  * This function sets the region of @t as requested by @start and @end.  If the
@@ -3115,7 +3116,7 @@ static bool damon_find_biggest_system_ram(unsigned long *start,
  */
 int damon_set_region_biggest_system_ram_default(struct damon_target *t,
 			unsigned long *start, unsigned long *end,
-			unsigned long min_region_sz)
+			unsigned long addr_unit, unsigned long min_region_sz)
 {
 	struct damon_addr_range addr_range;
 
@@ -3123,12 +3124,12 @@ int damon_set_region_biggest_system_ram_default(struct damon_target *t,
 		return -EINVAL;
 
 	if (!*start && !*end &&
-		!damon_find_biggest_system_ram(start, end, 1))
+			!damon_find_biggest_system_ram(start, end, addr_unit))
 		return -EINVAL;
 
 	addr_range.start = *start;
 	addr_range.end = *end;
-	return damon_set_regions(t, &addr_range, 1, min_region_sz);
+	return damon_set_regions(t, &addr_range, addr_unit, min_region_sz);
 }
 
 /*
diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 7bc5c0b2aea3e..133ea17e258df 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -345,6 +345,7 @@ static int damon_lru_sort_apply_parameters(void)
 	err = damon_set_region_biggest_system_ram_default(param_target,
 					&monitor_region_start,
 					&monitor_region_end,
+					param_ctx->addr_unit,
 					param_ctx->min_region_sz);
 	if (err)
 		goto out;
diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 43d76f5bed449..01f2f6cdbcdfe 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -251,6 +251,7 @@ static int damon_reclaim_apply_parameters(void)
 	err = damon_set_region_biggest_system_ram_default(param_target,
 					&monitor_region_start,
 					&monitor_region_end,
+					param_ctx->addr_unit,
 					param_ctx->min_region_sz);
 	if (err)
 		goto out;
diff --git a/mm/damon/stat.c b/mm/damon/stat.c
index 25fb44ccf99d0..f9a2028483b05 100644
--- a/mm/damon/stat.c
+++ b/mm/damon/stat.c
@@ -181,7 +181,7 @@ static struct damon_ctx *damon_stat_build_ctx(void)
 		goto free_out;
 	damon_add_target(ctx, target);
 	if (damon_set_region_biggest_system_ram_default(target, &start, &end,
-							ctx->min_region_sz))
+				ctx->addr_unit, ctx->min_region_sz))
 		goto free_out;
 	return ctx;
 free_out:
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/5] mm/damon/reclaim: respect addr_unit on default monitoring region setup
  2026-03-11  5:29 [PATCH 0/5] mm/damon: support addr_unit on default monitoring targets for modules SeongJae Park
                   ` (2 preceding siblings ...)
  2026-03-11  5:29 ` [PATCH 3/5] mm/damon/core: receive addr_unit on damon_set_region_biggest_system_ram_default() SeongJae Park
@ 2026-03-11  5:29 ` SeongJae Park
  2026-03-11  5:29 ` [PATCH 5/5] mm/damon/lru_sort: " SeongJae Park
  4 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-11  5:29 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SeongJae Park, damon, linux-kernel, linux-mm

In the past, damon_set_region_biggest_system_ram_default(), which is the
core function for setting the default monitoring target region of
DAMON_RECLAIM, didn't support addr_unit.  Hence DAMON_RECLAIM was
silently ignoring the user input for addr_unit when the user doesn't
explicitly set the monitoring target regions, and therefore the default
target region is being used.  No real problem from that ignorance was
reported so far.  But, the implicit rule is only making things
confusing.  Also, the default target region setup function is updated to
support addr_unit.  Hence there is no reason to keep ignoring it.
Respect the user-passed addr_unit for the default target monitoring
region use case.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/reclaim.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
index 01f2f6cdbcdfe..86da147786583 100644
--- a/mm/damon/reclaim.c
+++ b/mm/damon/reclaim.c
@@ -201,12 +201,6 @@ static int damon_reclaim_apply_parameters(void)
 	if (err)
 		return err;
 
-	/*
-	 * If monitor_region_start/end are unset, always silently
-	 * reset addr_unit to 1.
-	 */
-	if (!monitor_region_start && !monitor_region_end)
-		addr_unit = 1;
 	param_ctx->addr_unit = addr_unit;
 	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/5] mm/damon/lru_sort: respect addr_unit on default monitoring region setup
  2026-03-11  5:29 [PATCH 0/5] mm/damon: support addr_unit on default monitoring targets for modules SeongJae Park
                   ` (3 preceding siblings ...)
  2026-03-11  5:29 ` [PATCH 4/5] mm/damon/reclaim: respect addr_unit on default monitoring region setup SeongJae Park
@ 2026-03-11  5:29 ` SeongJae Park
  4 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-11  5:29 UTC (permalink / raw)
  To: Andrew Morton; +Cc: SeongJae Park, damon, linux-kernel, linux-mm

In the past, damon_set_region_biggest_system_ram_default(), which is the
core function for setting the default monitoring target region of
DAMON_LRU_SORT, didn't support addr_unit.  Hence DAMON_LRU_SORT was
silently ignoring the user input for addr_unit when the user doesn't
explicitly set the monitoring target regions, and therefore the default
target region is being used.  No real problem from that ignorance was
reported so far.  But, the implicit rule is only making things
confusing.  Also, the default target region setup function is updated to
support addr_unit.  Hence there is no reason to keep ignoring it.
Respect the user-passed addr_unit for the default target monitoring
region use case.

Signed-off-by: SeongJae Park <sj@kernel.org>
---
 mm/damon/lru_sort.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/mm/damon/lru_sort.c b/mm/damon/lru_sort.c
index 133ea17e258df..554559d729760 100644
--- a/mm/damon/lru_sort.c
+++ b/mm/damon/lru_sort.c
@@ -291,12 +291,6 @@ static int damon_lru_sort_apply_parameters(void)
 	if (err)
 		return err;
 
-	/*
-	 * If monitor_region_start/end are unset, always silently
-	 * reset addr_unit to 1.
-	 */
-	if (!monitor_region_start && !monitor_region_end)
-		addr_unit = 1;
 	param_ctx->addr_unit = addr_unit;
 	param_ctx->min_region_sz = max(DAMON_MIN_REGION_SZ / addr_unit, 1);
 
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/5] mm/damon/core: receive addr_unit on damon_set_region_biggest_system_ram_default()
  2026-03-11  5:29 ` [PATCH 3/5] mm/damon/core: receive addr_unit on damon_set_region_biggest_system_ram_default() SeongJae Park
@ 2026-03-14  0:18   ` SeongJae Park
  0 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-14  0:18 UTC (permalink / raw)
  To: SeongJae Park; +Cc: Andrew Morton, damon, linux-kernel, linux-mm

On Tue, 10 Mar 2026 22:29:24 -0700 SeongJae Park <sj@kernel.org> wrote:

> damon_find_biggest_system_ram() was not supporting addr_unit in the
> past.  Hence, its caller, damon_set_region_biggest_system_ram_default(),
> was also not supporting addr_unit.  The previous commit has updated the
> inner function to support addr_unit.  There is no more reason to not
> support addr_unit on damon_set_region_biggest_system_ram_default().
> Rather, it makes unnecessary inconsistency on support of addr_unit.
> Update it to receive addr_unit and handle it inside.
> 
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
>  include/linux/damon.h | 1 +
>  mm/damon/core.c       | 7 ++++---
>  mm/damon/lru_sort.c   | 1 +
>  mm/damon/reclaim.c    | 1 +
>  mm/damon/stat.c       | 2 +-
>  5 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index 1130c2f9a92f4..3a441fbca170d 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -988,6 +988,7 @@ int damos_walk(struct damon_ctx *ctx, struct damos_walk_control *control);
>  
>  int damon_set_region_biggest_system_ram_default(struct damon_target *t,
>  				unsigned long *start, unsigned long *end,
> +				unsigned long addr_unit,
>  				unsigned long min_region_sz);
>  
>  #endif	/* CONFIG_DAMON */
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index aee61bf991baa..d4f86c20b4f48 100644
[...]
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[...]
> @@ -3123,12 +3124,12 @@ int damon_set_region_biggest_system_ram_default(struct damon_target *t,
>  		return -EINVAL;
>  
>  	if (!*start && !*end &&
> -		!damon_find_biggest_system_ram(start, end, 1))
> +			!damon_find_biggest_system_ram(start, end, addr_unit))
>  		return -EINVAL;
>  
>  	addr_range.start = *start;
>  	addr_range.end = *end;
> -	return damon_set_regions(t, &addr_range, 1, min_region_sz);
> +	return damon_set_regions(t, &addr_range, addr_unit, min_region_sz);

The above line is unnecessarily and wrongly changing the third argument.  It is
the number of ranges that passed by the second argument, never meant to be the
addr_unit.

A similar bug [1] was found by an AI review, and I found this patch also has a
similar bug.

Andrew, could you please add below attaching fixup?

[1] https://lore.kernel.org/20260313234026.48872-1-sj@kernel.org


Thanks,
SJ

[...]
=== >8 ===
From 0d00bb5ca76502aef4a6a97b0855e33435147269 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Fri, 13 Mar 2026 16:49:08 -0700
Subject: [PATCH] mm/damon/core: fix wrong damon_set_regions() argument

The third argument is the length of the second parameter.  But addr_unit
is wrongly being passed.  Fix it.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index d4f86c20b4f48..f9854aedc42d1 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3129,7 +3129,7 @@ int damon_set_region_biggest_system_ram_default(struct damon_target *t,
 
 	addr_range.start = *start;
 	addr_range.end = *end;
-	return damon_set_regions(t, &addr_range, addr_unit, min_region_sz);
+	return damon_set_regions(t, &addr_range, 1, min_region_sz);
 }
 
 /*
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/5] mm/damon/core: support addr_unit on damon_find_biggest_system_ram()
  2026-03-11  5:29 ` [PATCH 2/5] mm/damon/core: support addr_unit on damon_find_biggest_system_ram() SeongJae Park
@ 2026-03-17 14:47   ` SeongJae Park
  0 siblings, 0 replies; 8+ messages in thread
From: SeongJae Park @ 2026-03-17 14:47 UTC (permalink / raw)
  To: SeongJae Park; +Cc: Andrew Morton, damon, linux-kernel, linux-mm

On Tue, 10 Mar 2026 22:29:23 -0700 SeongJae Park <sj@kernel.org> wrote:

> damon_find_biggest_system_ram() sets an 'unsigned long' variable with
> 'resource_size_t' value.  This is fundamentally wrong.  On environments
> such as ARM 32 bit machines having LPAE (Large Physical Address
> Extensions), which DAMON supports, the size of 'unsigned long' may be
> smaller than that of 'resource_size_t'.  It is safe, though, since we
> restrict the walk to be done only up to ULONG_MAX.
> 
> DAMON supports the address size gap using 'addr_unit'.  We didn't add
> the support to the function, just to make the initial support change
> small.  Now the support is reasonably settled.  This kind of gap is only
> making the code inconsistent and easy to be confused.  Add the support
> of 'addr_unit' to the function, by letting callers pass the 'addr_unit'
> and handling it in the function.  All callers are passing 'addr_unit' 1,
> though, to keep the old behavior.
> 
> Signed-off-by: SeongJae Park <sj@kernel.org>
> ---
>  mm/damon/core.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 3925720a172a6..aee61bf991baa 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -3056,31 +3056,44 @@ static int kdamond_fn(void *data)
>  
>  static int walk_system_ram(struct resource *res, void *arg)
>  {
> -	struct damon_addr_range *a = arg;
> +	struct resource *a = arg;
>  
> -	if (a->end - a->start < resource_size(res)) {
> +	if (resource_size(a) < resource_size(res)) {
>  		a->start = res->start;
> -		a->end = res->end + 1;
> +		a->end = res->end;
>  	}
>  	return 0;
>  }
>  
> +static unsigned long damon_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;
> +}
> +
>  /*
>   * Find biggest 'System RAM' resource and store its start and end address in
>   * @start and @end, respectively.  If no System RAM is found, returns false.
>   */
>  static bool damon_find_biggest_system_ram(unsigned long *start,
> -						unsigned long *end)
> +		unsigned long *end, unsigned long addr_unit)
>  
>  {
> -	struct damon_addr_range arg = {};
> +	struct resource res = {};
>  
> -	walk_system_ram_res(0, ULONG_MAX, &arg, walk_system_ram);
> -	if (arg.end <= arg.start)
> +	walk_system_ram_res(0, -1, &res, walk_system_ram);
> +	if (res.end < res.start)
>  		return false;
>  
> -	*start = arg.start;
> -	*end = arg.end;
> +	*start = damon_res_to_core_addr(res.start, addr_unit);
> +	*end = damon_res_to_core_addr(res.end + 1, addr_unit);
>  	return true;

On 32 bit systems having PAE (>4 GiB physical memory address space), above
start and end address could be overflowed, resulting in making an invalid
address range (end <= start).  The range validity should be tested here, like
below attaching fixup patch.

Andrew, could you please add the fixup patch?


Thanks,
SJ

[...]
=== >8 ===
From d5654a6cce8a21ae100625ed54c0885556f89645 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Mon, 16 Mar 2026 23:32:48 -0700
Subject: [PATCH] mm/damon/core: verify found biggest system ram

On 32 bit systems having PAE (>4 GiB physical memory address sapce), the
final start and end address could overflow, resulting in returning an
invalid address range.  Verify the returning region.  Also remove the
resource validation after walk_system_ram_res(), since the validation
means not a lot.

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

diff --git a/mm/damon/core.c b/mm/damon/core.c
index f9854aedc42d1..339325e1096bc 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -3089,11 +3089,10 @@ static bool damon_find_biggest_system_ram(unsigned long *start,
 	struct resource res = {};
 
 	walk_system_ram_res(0, -1, &res, walk_system_ram);
-	if (res.end < res.start)
-		return false;
-
 	*start = damon_res_to_core_addr(res.start, addr_unit);
 	*end = damon_res_to_core_addr(res.end + 1, addr_unit);
+	if (*end <= *start)
+		return false;
 	return true;
 }
 
-- 
2.47.3



^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-03-17 14:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11  5:29 [PATCH 0/5] mm/damon: support addr_unit on default monitoring targets for modules SeongJae Park
2026-03-11  5:29 ` [PATCH 1/5] mm/damon/core: fix wrong end address assignment on walk_system_ram() SeongJae Park
2026-03-11  5:29 ` [PATCH 2/5] mm/damon/core: support addr_unit on damon_find_biggest_system_ram() SeongJae Park
2026-03-17 14:47   ` SeongJae Park
2026-03-11  5:29 ` [PATCH 3/5] mm/damon/core: receive addr_unit on damon_set_region_biggest_system_ram_default() SeongJae Park
2026-03-14  0:18   ` SeongJae Park
2026-03-11  5:29 ` [PATCH 4/5] mm/damon/reclaim: respect addr_unit on default monitoring region setup SeongJae Park
2026-03-11  5:29 ` [PATCH 5/5] mm/damon/lru_sort: " SeongJae Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox