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

[email protected]
Newsgroups dev.linux.lists.damon
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `kdamond_merge_regions()` can fall into an infinite loop due to integer overflow of `threshold`.
--

--- Patch [1]: [PATCH] mm/damon/core: skip aging from repeated aggressive merging ---
commit 1fa7392cc3fc19083e90cfb01dcfa4f03ebbc373
Author: SJ Park <[email protected]>

mm/damon/core: skip aging from repeated aggressive merging

This commit modifies the DAMON core to skip aging when repeating region
merges during corner cases where the number of regions exceeds the
user-defined maximum limit. This prevents temporary degradation of
monitoring quality caused by updating region ages multiple times during
repeated merge operations.

Link: https://lore.kernel.org/[email protected]
Link: https://lore.kernel.org/[email protected]

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
[ ... ]
> @@ -3366,15 +3368,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, c);
> +			damon_merge_regions_of(t, threshold, sz_limit, c,
> +					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 kdamond_merge_regions() fall into an
infinite loop under certain configurations?

If a user configures aggr_interval and sample_interval such that their
quotient (max_thres) exceeds 0x40000000, threshold could overflow in the
max() assignment above. 

Since threshold is an unsigned int, doubling it once it reaches 0x80000000
causes an overflow to 0, which max(1, 0) clamps back to 1.

If regions cannot be merged further due to the sz_limit constraint,
nr_regions > c->attrs.max_nr_regions remains true indefinitely.

Because threshold cycles and never exceeds 0xFFFFFFFF, threshold / 2 never
reaches max_thres, causing this loop condition to evaluate to true endlessly.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.