Re: [PATCH 5.15.y] mm/damon/core: avoid infinite kdamond_merge_regions() internal loop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Calculation of merged region access frequency can overflow unsigned long on 32-bit architectures, leading to corrupted access statistics.
--

commit 49c8c958aefed16883d3ce4d2099a0422bae39ec
Author: SJ Park <sj@kernel.org>

mm/damon/core: avoid infinite kdamond_merge_regions() internal loop

This commit fixes a potential infinite loop in kdamond_merge_regions()
by checking the loop break condition before doubling the threshold. It
also prevents the threshold from exceeding the theoretical maximum.

Link: https://lore.kernel.org/20260715031002.108504-1-sj@kernel.org
Link: https://lore.kernel.org/20260715031002.108504-2-sj@kernel.org
Link: https://lore.kernel.org/20260709145425.96247-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 4f031412f65cc..3a50ced2efc0f 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -523,15 +523,20 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
>  
>  	max_thres = c->aggr_interval /
>  		(c->sample_interval ?  c->sample_interval : 1);
> -	do {
> +	while (true) {
>  		nr_regions = 0;
>  		damon_for_each_target(t, c) {
>  			damon_merge_regions_of(t, threshold, sz_limit);

[Severity: High]
This is a pre-existing issue, but does the access frequency calculation in
damon_merge_two_regions() (called via damon_merge_regions_of()) overflow on
32-bit architectures?

In mm/damon/core.c:damon_merge_two_regions(), the new access count is
calculated as:

    l->nr_accesses = (l->nr_accesses * sz_l + r->nr_accesses * sz_r) /
                     (sz_l + sz_r);

Since sz_limit allows regions to grow up to 1/10th of the target's address
space, large regions can cause this to silently overflow a 32-bit unsigned
long. For example, merging a 250 MB region with a typical access frequency
of 20 results in 20 * 262,144,000 = 5,242,880,000, which exceeds the 32-bit
maximum of 4,294,967,295.

Can this lead to severely undercounted access statistics, potentially causing
DAMON_RECLAIM to aggressively swap out falsely "cold" regions and induce
system thrashing?

>  			nr_regions += damon_nr_regions(t);
>  		}
> -		threshold = max(1, threshold * 2);
> -	} while (nr_regions > c->max_nr_regions &&
> -			threshold / 2 < max_thres);
> +		if (nr_regions <= c->max_nr_regions ||
> +				max_thres <= threshold)
> +			break;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909042418.198028-1-sj@kernel.org?part=1