Re: [PATCH 6.1.y] mm/damon/core: skip aging from repeated aggressive merging

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The `kdamond_merge_regions` loop can run infinitely if `max_thres` is extremely large and `sz_limit` restricts further merges, leading to a permanent kernel thread soft lockup due to a 32-bit `unsigned int` overflow.
- [High] Arithmetic integer overflow during weighted average calculations in `damon_merge_two_regions` on 32-bit systems causes severe corruption of `nr_accesses` and `age` metrics.
--

commit 1486ebcac154b450386db76101ad72341a5b5805
Author: SJ Park <sj@kernel.org>
Subject: mm/damon/core: skip aging from repeated aggressive merging

This commit modifies kdamond_merge_regions() to skip updating region
ages during repeated aggressive merges. This prevents temporal degradation
of monitoring quality when the maximum number of regions limit is
temporarily exceeded.

Link: https://lore.kernel.org/20260712165432.87609-1-sj@kernel.org
Link: https://lore.kernel.org/20260621203548.10718-1-sj@kernel.org [1]
Link: https://lore.kernel.org/20260709145425.96247-1-sj@kernel.org [2]

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index dd4eafe8b9611..859a6a636ab0e 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -940,15 +940,17 @@ static void damon_merge_two_regions(struct damon_target *t,

[Severity: High]
This isn't a bug introduced by this patch, but could this cause silent
corruption of the region access counts and ages on 32-bit systems?

In damon_merge_two_regions(), the weighted average calculations are:

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

If a region size is very large (e.g., 100 megabytes) and nr_accesses is 45 or
greater, the product of l->nr_accesses * sz_l and l->age * sz_l will exceed
4.29 billion. This overflows the 32-bit maximum before the division occurs,
truncating the weighted sum and storing incorrect values in the merged region.

>   * sz_limit	size upper limit of each region
>   */
>  static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
> -				   unsigned long sz_limit)
> +		unsigned long sz_limit, bool count_age)
>  {

[ ... ]

> @@ -982,15 +984,18 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
>  	struct damon_target *t;
>  	unsigned int nr_regions;
>  	unsigned int max_thres;
> +	bool count_age = true;
>  
>  	max_thres = c->attrs.aggr_interval /
>  		(c->attrs.sample_interval ?  c->attrs.sample_interval : 1);
>  	do {
>  		nr_regions = 0;
>  		damon_for_each_target(t, c) {
> -			damon_merge_regions_of(t, threshold, sz_limit);
> +			damon_merge_regions_of(t, threshold, sz_limit,
> +					count_age);
>  			nr_regions += damon_nr_regions(t);
>  		}
> +		count_age = false;
>  		threshold = max(1, threshold * 2);
>  	} while (nr_regions > c->attrs.max_nr_regions &&
>  			threshold / 2 < max_thres);

[Severity: High]
This is a pre-existing issue, but can this loop run infinitely if
max_thres is extremely large and merges are blocked?

If a privileged user sets a very large aggr_interval relative to
sample_interval such that max_thres exceeds 2^30 (1,073,741,824), and
merges are blocked by size limits, threshold will continually double.

Because threshold is a 32-bit unsigned int, once it reaches 2^31,
threshold * 2 will overflow to 0, which max(1, 0) will reset to 1.
Then threshold / 2 becomes 0.

This causes the exit condition threshold / 2 < max_thres to remain
perpetually true while nr_regions never drops, resulting in a permanent
kernel thread soft lockup.

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