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

[SJ Park <sj@kernel.org>]

On Sat, 18 Jul 2026 10:15:29 -0700 SJ Park <sj@kernel.org> wrote:

> On Sat, 18 Jul 2026 17:00:57 +0000 sashiko-bot@kernel.org 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 <sj@kernel.org>
> > 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/20260717011834.120715-1-sj@kernel.org
> > 
> > > 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.

Hmm, but no reason to rush.  I will fix this together in the next revision.


Thanks,
SJ

[...]