[PATCH] mm/damon: unify address range representation with damon_addr_range

[PATCH] mm/damon: unify address range representation with damon_addr_range

[Enze Li]

Currently, DAMON defines two identical structures for representing
address ranges: damon_system_ram_region and damon_addr_range.  Both
structures share the same semantic interpretation of a half-open
interval [start, end), where the start address is inclusive and the end
address is exclusive.

This duplication adds unnecessary redundancy and increases maintenance
overhead.  This patch replaces all uses of damon_system_ram_region with
the more generic damon_addr_range structure, ensuring a unified type
representation for address ranges within the DAMON subsystem.  The
change simplifies the codebase, improves readability, and avoids
potential inconsistencies in future modifications.

Signed-off-by: Enze Li <lienze@kylinos.cn>
---
 mm/damon/core.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 70efbf22a2b4..5e2724a4f285 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -2856,20 +2856,9 @@ static int kdamond_fn(void *data)
 	return 0;
 }
 
-/*
- * struct damon_system_ram_region - System RAM resource address region of
- *				    [@start, @end).
- * @start:	Start address of the region (inclusive).
- * @end:	End address of the region (exclusive).
- */
-struct damon_system_ram_region {
-	unsigned long start;
-	unsigned long end;
-};
-
 static int walk_system_ram(struct resource *res, void *arg)
 {
-	struct damon_system_ram_region *a = arg;
+	struct damon_addr_range *a = arg;
 
 	if (a->end - a->start < resource_size(res)) {
 		a->start = res->start;
@@ -2886,7 +2875,7 @@ static bool damon_find_biggest_system_ram(unsigned long *start,
 						unsigned long *end)
 
 {
-	struct damon_system_ram_region arg = {};
+	struct damon_addr_range arg = {};
 
 	walk_system_ram_res(0, ULONG_MAX, &arg, walk_system_ram);
 	if (arg.end <= arg.start)

base-commit: 0241748f8b68fc2bf637f4901b9d7ca660d177ca
-- 
2.52.0

Re: [PATCH] mm/damon: unify address range representation with damon_addr_range

[SeongJae Park]

On Thu, 29 Jan 2026 18:08:45 +0800 Enze Li <lienze@kylinos.cn> wrote:

> Currently, DAMON defines two identical structures for representing
> address ranges: damon_system_ram_region and damon_addr_range.  Both
> structures share the same semantic interpretation of a half-open
> interval [start, end), where the start address is inclusive and the end
> address is exclusive.

Nice finding!

> 
> This duplication adds unnecessary redundancy and increases maintenance
> overhead.  This patch replaces all uses of damon_system_ram_region with
> the more generic damon_addr_range structure, ensuring a unified type
> representation for address ranges within the DAMON subsystem.  The
> change simplifies the codebase, improves readability, and avoids
> potential inconsistencies in future modifications.

Actually, there is another bug that need to be fixed.  Nonetheless, that's
orthogonal to this patch, so it is not a blocker of this patch in my opinion.

> 
> Signed-off-by: Enze Li <lienze@kylinos.cn>

Reviewed-by: SeongJae Park <sj@kernel.org>

> ---
>  mm/damon/core.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 70efbf22a2b4..5e2724a4f285 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2856,20 +2856,9 @@ static int kdamond_fn(void *data)
>  	return 0;
>  }
>  
> -/*
> - * struct damon_system_ram_region - System RAM resource address region of
> - *				    [@start, @end).
> - * @start:	Start address of the region (inclusive).
> - * @end:	End address of the region (exclusive).
> - */
> -struct damon_system_ram_region {
> -	unsigned long start;
> -	unsigned long end;
> -};
> -
>  static int walk_system_ram(struct resource *res, void *arg)
>  {
> -	struct damon_system_ram_region *a = arg;
> +	struct damon_addr_range *a = arg;
>  
>  	if (a->end - a->start < resource_size(res)) {
>  		a->start = res->start;

The above line is storing 'resource_size_t' value to 'unsigned long' field.
That shouldn't be a problem on 64 bit environments.  With introduction of
damon_ctx->addr_unit [1], however, DAMON started supporting environments that
'unsigned long' is not enough to represent the whole physical address range,
such as 32 bit environments with large phyiscal address extension.  On such
environments, this could be a problem.

Nonetheless, this is orthogonal to this patch, as I mentioned above.  So it
shouldn't block this patch.  I will fix the problem unless someone steps up
faster than I.

[1] commit 09a616cbb371 ("mm/damon/core: add damon_ctx->addr_unit") 


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: unify address range representation with damon_addr_range

[Quanmin Yan]

Hi SJ,

[...]

> Actually, there is another bug that need to be fixed.  Nonetheless, that's
> orthogonal to this patch, so it is not a blocker of this patch in my opinion.

Are you referring to the scenario where, on a 32-bit system with LPAE 
support, the biggest System RAM address found exceeds the range of 
unsigned long? Actually, DAMON only actively searches for the biggest 
System RAM address when the user does not set a monitoring region. In 
this case, we always reset addr_unit to 1, which essentially restricts 
DAMON’s monitorable address range to 0–UL. Therefore, the described 
situation does not occur. For details, please refer to commit [1]. 
However, we could perhaps further optimize this by finding any biggest 
System RAM address, setting damon_addr_range, and dynamically adjusting 
addr_unit. However, there are currently no specific requirements for 
this, so this work has been temporarily put on hold. [1] commit 
dfc02531f413 ("mm/damon/reclaim: use min_sz_region for core address 
alignment when setting regions") Thanks, Quanmin Yan

[...]

Re: [PATCH] mm/damon: unify address range representation with damon_addr_range

[Quanmin Yan]

在 2026/1/30 10:26, Quanmin Yan 写道:
> Hi SJ,
>
> [...]
>
>> Actually, there is another bug that need to be fixed.  Nonetheless, 
>> that's
>> orthogonal to this patch, so it is not a blocker of this patch in my 
>> opinion.
>
> Are you referring to the scenario where, on a 32-bit system with LPAE 
> support, the biggest System RAM address found exceeds the range of 
> unsigned long? Actually, DAMON only actively searches for the biggest 
> System RAM address when the user does not set a monitoring region. In 
> this case, we always reset addr_unit to 1, which essentially restricts 
> DAMON’s monitorable address range to 0–UL. Therefore, the described 
> situation does not occur. For details, please refer to commit [1]. 
> However, we could perhaps further optimize this by finding any biggest 
> System RAM address, setting damon_addr_range, and dynamically 
> adjusting addr_unit. However, there are currently no specific 
> requirements for this, so this work has been temporarily put on hold. 
> [1] commit dfc02531f413 ("mm/damon/reclaim: use min_sz_region for core 
> address alignment when setting regions") Thanks, Quanmin Yan
>
> [...] 

(Sorry there was an issue with my email client. I am resending now.)

Are you referring to the scenario where, on a 32-bit system with LPAE
support, the biggest System RAM address found exceeds the range of
unsigned long?

Actually, DAMON only actively searches for the biggest System RAM
address when the user does not set a monitoring region. In this case,
we always reset addr_unit to 1, which essentially restricts DAMON’s
monitorable address range to 0–UL. Therefore, the described situation
does not occur. For details, please refer to commit [1].

However, we could perhaps further optimize this by finding any biggest
System RAM address, setting damon_addr_range, and dynamically adjusting
addr_unit. However, there are currently no specific requirements for
this, so this work has been temporarily put on hold.

[1] commit dfc02531f413 ("mm/damon/reclaim: use min_sz_region for core
address alignment when setting regions")

Thanks,
Quanmin Yan

Re: [PATCH] mm/damon: unify address range representation with damon_addr_range

[SeongJae Park]

Hello Quanmin,

On Fri, 30 Jan 2026 11:07:08 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:

Thank you for jumping in and giving great comments!

> 在 2026/1/30 10:26, Quanmin Yan 写道:
> > Hi SJ,
> >
> > [...]
> >
> >> Actually, there is another bug that need to be fixed.  Nonetheless, 
> >> that's
> >> orthogonal to this patch, so it is not a blocker of this patch in my 
> >> opinion.
> >
> > Are you referring to the scenario where, on a 32-bit system with LPAE 
> > support, the biggest System RAM address found exceeds the range of 
> > unsigned long? Actually, DAMON only actively searches for the biggest 
> > System RAM address when the user does not set a monitoring region. In 
> > this case, we always reset addr_unit to 1, which essentially restricts 
> > DAMON’s monitorable address range to 0–UL. Therefore, the described 
> > situation does not occur. For details, please refer to commit [1]. 
> > However, we could perhaps further optimize this by finding any biggest 
> > System RAM address, setting damon_addr_range, and dynamically 
> > adjusting addr_unit. However, there are currently no specific 
> > requirements for this, so this work has been temporarily put on hold. 
> > [1] commit dfc02531f413 ("mm/damon/reclaim: use min_sz_region for core 
> > address alignment when setting regions") Thanks, Quanmin Yan
> >
> > [...] 
> 
> (Sorry there was an issue with my email client. I am resending now.)

No worry!

> 
> Are you referring to the scenario where, on a 32-bit system with LPAE
> support, the biggest System RAM address found exceeds the range of
> unsigned long?

You are right.

> 
> Actually, DAMON only actively searches for the biggest System RAM
> address when the user does not set a monitoring region. In this case,
> we always reset addr_unit to 1, which essentially restricts DAMON’s
> monitorable address range to 0–UL. Therefore, the described situation
> does not occur. For details, please refer to commit [1].

Again, you are correct.  There is no user scenario that can trigger the
problematic situation in the real world.  Nevertheless, assigning
'resource_size_t' value to 'unsigned long' storage makes no sense, so I'm
calling such things a bug.  Not impacting user world, but still a bug is a bug.
In my opinion, all bugs are better to be fixed unless it makes maintenance
overhead increased too much.  Also, who knows if I will introduce another code
calling it with >1 add_unit.

> 
> However, we could perhaps further optimize this by finding any biggest
> System RAM address, setting damon_addr_range, and dynamically adjusting
> addr_unit.

I agree.  I think we can fix the function first, and later consider further
allowing users to use its full power.

> However, there are currently no specific requirements for
> this, so this work has been temporarily put on hold.

You are right.  As long as there is no requirements, there is no reason to
expose the full power to users at the moment.

By the way, one thing that may better to be clear is that I requested you to
make the implementation in this way.  Nobody should be blamed, but if someone
has to be blamed for the current shape of the code, it is only me. :)

> 
> [1] commit dfc02531f413 ("mm/damon/reclaim: use min_sz_region for core
> address alignment when setting regions")


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: unify address range representation with damon_addr_range

[SeongJae Park]

On Thu, 29 Jan 2026 19:38:45 -0800 SeongJae Park <sj@kernel.org> wrote:

> Hello Quanmin,
> 
> On Fri, 30 Jan 2026 11:07:08 +0800 Quanmin Yan <yanquanmin1@huawei.com> wrote:
[...]
> > Actually, DAMON only actively searches for the biggest System RAM
> > address when the user does not set a monitoring region. In this case,
> > we always reset addr_unit to 1, which essentially restricts DAMON’s
> > monitorable address range to 0–UL. Therefore, the described situation
> > does not occur. For details, please refer to commit [1].
> 
> Again, you are correct.  There is no user scenario that can trigger the
> problematic situation in the real world.  Nevertheless, assigning
> 'resource_size_t' value to 'unsigned long' storage makes no sense, so I'm
> calling such things a bug.  Not impacting user world, but still a bug is a bug.

I was arguing it as a bug even though ABI users cannot trigger the overflow,
since damon_set_region_biggest_system_ram_default() is exposed to DAMON API
callers, and I was thinking a future caller of the function could trigger the
problem.  But I now realize the API function is safe from any call, since
damon_find_biggest_system_ram() is avoiding the case, by setting 'end'
parameter of walk_system_ram_res() as 'ULONG_MAX'.  Maybe it is not really
appropriate to call it a bug.

That said, I now think this as a room to improve for the readability of the
code.


Thanks,
SJ

[...]