Re: [PATCH 2/5] mm/damon/core: support addr_unit on damon_find_biggest_system_ram()

[SeongJae Park]

On Tue, 10 Mar 2026 22:29:23 -0700 SeongJae Park <sj@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  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 ===