Re: [PATCH 6.12.y v3 1/2] mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: damon: double application of DAMOS schemes due to non-safe region walk
--

commit 7c60a2f66acad38f06b09b2a9e484519168800bc
Author: SeongJae Park <sj@kernel.org>

mm/damon/core: do non-safe region walk on kdamond_apply_schemes()

This commit changes kdamond_apply_schemes() to use the non-safe
damon_for_each_region() iterator instead of damon_for_each_region_safe().
It also drops the double-pointer workaround in damos_skip_charged_region()
to let newly split regions be seen on the next iteration of the region walk.

Link: https://lkml.kernel.org/r/20260227170623.95384-3-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 32737cc5f877b..76f23c1d2f56b 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -1749,7 +1749,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
>  		return;
>  
>  	damon_for_each_target(t, c) {
> -		damon_for_each_region_safe(r, next_r, t)
> +		damon_for_each_region(r, t)
>  			damon_do_apply_schemes(c, t, r);
>  	}

[Severity: High]
Does this change cause earlier schemes to double-apply their actions when a
subsequent scheme splits the region?

In kdamond_apply_schemes(), changing to the non-safe iterator means the loop
dynamically visits newly created fragments on the very next iteration.

When damon_do_apply_schemes() runs, it evaluates all schemes sequentially on
the region:

mm/damon/core.c:damon_do_apply_schemes() {
	...
	damon_for_each_scheme(s, c) {
		...
		if (damos_skip_charged_region(t, r, s))
			continue;
		...
		damos_apply_scheme(c, t, r, s);
	}
}

If an earlier scheme evaluates the region, it may apply its action and update
its statistics for the full size of the region.

If a subsequent scheme then evaluates the same region and splits it (for
example via damos_skip_charged_region()), it shrinks the original region and
inserts a new region immediately after it.

Because damon_for_each_region() is no longer safe against modifications to the
list, the next iteration of the outer loop will process the newly inserted
region across all schemes.

Since the newly inserted region retains the exact properties (age, access
counts) of the original region, will the earlier scheme re-evaluate it and
apply its action a second time, double-charging its statistics and applying
redundant actions to the memory?

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