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

[SJ Park <sj@kernel.org>]

On Fri, 09 Oct 2026 14:56:56 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] mm/damon: double processing and skipped regions due to broken backport
> --
> 
> commit 18797502eb0b5cf3a96dbf53e3e238e6ea4158ce
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/core: do non-safe region walk on kdamond_apply_schemes()
> 
> This commit replaces the damon_for_each_region_safe iterator with
> damon_for_each_region in kdamond_apply_schemes. This change ensures that
> newly split regions from address filters are properly processed during the
> iteration.
> 
> 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 7afbc20833ea4..c65d2cbeea6b5 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -912,7 +912,7 @@ static void kdamond_apply_schemes(struct damon_ctx *c)
> >  	}
> >  
> >  	damon_for_each_target(t, c) {
> > -		damon_for_each_region_safe(r, next_r, t)
> > +		damon_for_each_region(r, t)
> 
> [Severity: High]
> Does this change result in regions being skipped and double-processed?
> 
> The commit message mentions dropping the workaround in
> damos_skip_charged_region(), but in this stable kernel version, that helper
> does not exist yet and the workaround is still inlined in
> damon_do_apply_schemes(). Since the inline workaround wasn't removed in this
> backport, it appears to conflict with the new loop iterator.

Good catch.  I will fix this in the next spin.

> 
> If a scheme's quota->charge_addr_from check triggers a region split in
> damon_do_apply_schemes(), the local pointer is advanced:
> 
> mm/damon/core.c:damon_do_apply_schemes() {
>     ...
> 			if (quota->charge_addr_from && r->ar.start <
> 					quota->charge_addr_from) {
> 				sz = ALIGN_DOWN(quota->charge_addr_from -
> 						r->ar.start, DAMON_MIN_REGION);
> 				if (!sz) {
> 					if (damon_sz_region(r) <=
> 					    DAMON_MIN_REGION)
> 						continue;
> 					sz = DAMON_MIN_REGION;
> 				}
> 				damon_split_region_at(t, r, sz);
> 				r = damon_next_region(r);
> 				sz = damon_sz_region(r);
> 			}
>     ...
> }
> 
> Because r is reassigned locally within the inner scheme loop, subsequent
> schemes will apply their actions to the newly created second-half region,
> completely skipping the first half. 
> 
> Then, when damon_do_apply_schemes() returns to the outer loop in
> kdamond_apply_schemes():
> 
> 		damon_for_each_region(r, t)
> 			damon_do_apply_schemes(c, t, r);
> 
> The iterator will advance naturally to the newly split second-half region.
> This causes damon_do_apply_schemes() to be called a second time on the exact
> same region, leading to DAMON actions being applied multiple times to the
> same memory, double-charged quotas, and corrupted statistics.

This is an issue that exist in the mainline, too.  I will separately work on
it.

> 
> >  			damon_do_apply_schemes(c, t, r);
> >  	}
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261009144808.18317-1-sj@kernel.org?part=1


Thanks,
SJ