Re: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions()

On Sat, 18 Jul 2026 17:00:57 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete bug fix in damon_apply_min_nr_regions() leads to O(N^2) allocator thrashing on allocation failure.
> - [High] Silent failure of damon_split_region_at() bypasses DAMON quotas and bounds checks.
> --
> 
> --- Patch [1]: [PATCH] mm/damon/core: handle region split failure in apply_min_nr_regions() ---
> commit 51e7e9012c52dde83bac4be44f0bbfbdbe01572d
> Author: SJ Park <[email protected]>
> Subject: mm/damon/core: handle region split failure in apply_min_nr_regions()
> 
> damon_apply_min_nr_regions() repeatedly split each region until its size
> becomes small enough to meet the user-defined low limit of the number of
> regions. The loop assumes the split operation (damon_split_region_at())
> will always succeed and create the new region.
> 
> Link: https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 20d267c615faf..3d829e0ad63b5 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -1913,6 +1913,9 @@ static unsigned long damon_apply_min_nr_regions(struct damon_ctx *ctx)
> >  		damon_for_each_region_safe(r, next, t) {
> >  			while (damon_sz_region(r) > max_region_sz) {
> >  				damon_split_region_at(t, r, max_region_sz);
> > +				/* split might failed */
> > +				if (r == damon_last_region(t))
> > +					break;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does this fix leave an
> O(N^2) allocator thrashing loop when an allocation fails on a non-last
> region?
> 
> If a region fails to split due to memory pressure and is not the last
> region, the inner loop unconditionally advances r = damon_next_region(r).
> This moves to the next pre-existing region instead of a newly created
> one, and the inner loop will attempt to split this next region, continuing
> to the end of the list.
> 
> Then, the outer damon_for_each_region_safe() loop advances to its
> pre-saved next pointer and repeats the entire process.
> 
> Could this severely thrash the page allocator with redundant
> kmem_cache_alloc() calls and cause RCU stalls or lockups during an
> OOM condition?

The allocation is arguably too small to fail.  And the while loop is supposed
to run only occasionally.  Most regions in most case would not exceed
max_region_sz.  But good call.  I will separately work on making this more
safe.

> 
> [Severity: High]
> This is also a pre-existing issue and was not introduced by this patch, but
> since we are handling split failures here, could the silent failure of
> damon_split_region_at() bypass DAMON quotas in other callers?
> 
> Because damon_split_region_at() returns void, callers like
> damos_apply_scheme() cannot detect if the split failed:
> 
> mm/damon/core.c:damos_apply_scheme() {
> ...
> 			damon_split_region_at(t, r, sz);
> 		}
> 		if (damos_core_filter_out(c, t, r, s))
> 			return;
> ...
> 		sz_applied = c->ops.apply_scheme(c, t, r, s,
> 				&sz_ops_filter_passed);
> ...
> 		damos_charge_quota(quota, sz, sz_applied);
> }
> 
> If the split fails, the region remains oversized and the scheme is
> erroneously applied to the entire original region.
> 
> The quota charging logic then only charges the intended small size (sz),
> which permanently hides the limit bypass.
> 
> Could this cause DAMON schemes to violate their strict resource quota
> limits and apply actions to much larger memory areas than permitted?

Good finding.  Seems not urgent and not related to this patch.  I will
separately work on this.

> 
> >  				r = damon_next_region(r);
> >  			}
> >  		}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1


Thanks,
SJ